mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-28 05:16:23 -06:00
fix(approvals): bind all clients to request instances
This commit is contained in:
@@ -19,6 +19,7 @@ import java.util.concurrent.atomic.AtomicLong
|
||||
|
||||
data class GatewayExecApprovalSummary(
|
||||
val id: String,
|
||||
val instanceId: String? = null,
|
||||
val commandText: NativeText,
|
||||
val commandPreview: String?,
|
||||
val warningText: String?,
|
||||
@@ -211,9 +212,11 @@ internal fun buildGatewayExecApprovalGetParams(id: String): JsonObject = buildJs
|
||||
internal fun buildGatewayExecApprovalResolveParams(
|
||||
id: String,
|
||||
decision: String,
|
||||
instanceId: String? = null,
|
||||
): JsonObject =
|
||||
buildJsonObject {
|
||||
put("id", id)
|
||||
instanceId?.let { put("instanceId", it) }
|
||||
put("kind", "exec")
|
||||
put("decision", decision)
|
||||
}
|
||||
@@ -387,15 +390,23 @@ internal fun legacyGatewayExecApprovalTerminal(
|
||||
private fun parseGatewayExecApprovalSnapshot(obj: JsonObject): GatewayExecApprovalSnapshot? {
|
||||
val status = obj.strictString("status") ?: return null
|
||||
val expectedKeys = APPROVAL_SNAPSHOT_KEYS_BY_STATUS[status] ?: return null
|
||||
if (!obj.hasExactKeys(expectedKeys)) return null
|
||||
if (
|
||||
status == "pending" &&
|
||||
!obj.hasExactKeys(expectedKeys) &&
|
||||
!obj.hasExactKeys(expectedKeys + "instanceId")
|
||||
) {
|
||||
return null
|
||||
}
|
||||
if (status != "pending" && !obj.hasExactKeys(expectedKeys)) return null
|
||||
val id = obj.strictApprovalId("id") ?: return null
|
||||
val instanceId = obj.optionalString("instanceId", requireNonEmpty = true) ?: return null
|
||||
obj.strictNonEmptyString("urlPath") ?: return null
|
||||
val createdAtMs = obj.strictNonNegativeLong("createdAtMs") ?: return null
|
||||
val expiresAtMs = obj.strictNonNegativeLong("expiresAtMs") ?: return null
|
||||
val presentation = obj["presentation"].asObjectOrNull() ?: return null
|
||||
val summary = parseGatewayExecApprovalPresentation(id, createdAtMs, expiresAtMs, presentation) ?: return null
|
||||
return when (status) {
|
||||
"pending" -> GatewayExecApprovalSnapshot.Pending(summary)
|
||||
"pending" -> GatewayExecApprovalSnapshot.Pending(summary.copy(instanceId = instanceId.value))
|
||||
"allowed" ->
|
||||
parseTerminalApproval(
|
||||
obj = obj,
|
||||
@@ -528,8 +539,8 @@ private fun JsonObject.strictNonNegativeLong(key: String): Long? =
|
||||
?.longOrNull
|
||||
?.takeIf { it >= 0 }
|
||||
|
||||
// Closed-schema contract: the gateway protocol declares approval results with
|
||||
// additionalProperties:false, so additive protocol changes hard-fail old clients by design.
|
||||
// Closed-schema contract: accept only the fields declared by the negotiated
|
||||
// approval projection, including its optional pending instance token.
|
||||
private fun JsonObject.hasExactKeys(expected: Set<String>): Boolean = keys == expected
|
||||
|
||||
private fun JsonObject.hasOnlyKeys(allowed: Set<String>): Boolean = keys.all(allowed::contains)
|
||||
|
||||
@@ -1544,8 +1544,9 @@ class MainViewModel private constructor(
|
||||
fun resolveExecApproval(
|
||||
id: String,
|
||||
decision: String,
|
||||
instanceId: String?,
|
||||
) {
|
||||
ensureRuntime().resolveExecApproval(id = id, decision = decision)
|
||||
ensureRuntime().resolveExecApproval(id = id, decision = decision, instanceId = instanceId)
|
||||
}
|
||||
|
||||
fun dismissExecApprovalsNotice(expected: GatewayExecApprovalNotice) {
|
||||
|
||||
@@ -788,6 +788,7 @@ class NodeRuntime private constructor(
|
||||
val stableId: String,
|
||||
val id: String,
|
||||
val decision: String,
|
||||
val instanceId: String?,
|
||||
// Captured at registration: canonical readback needs it after a refresh has
|
||||
// already replaced the visible rows, or the legacy get parse drops the row.
|
||||
val createdAtMs: Long?,
|
||||
@@ -2731,12 +2732,17 @@ class NodeRuntime private constructor(
|
||||
fun resolveExecApproval(
|
||||
id: String,
|
||||
decision: String,
|
||||
instanceId: String? = null,
|
||||
) {
|
||||
val exactId = id.takeIf(::isWellFormedGatewayApprovalId)
|
||||
val normalizedDecision = normalizeGatewayExecApprovalDecision(decision)
|
||||
if (exactId == null || normalizedDecision == null) return
|
||||
scope.launch {
|
||||
resolveExecApprovalOnGateway(id = exactId, decision = normalizedDecision)
|
||||
resolveExecApprovalOnGateway(
|
||||
id = exactId,
|
||||
decision = normalizedDecision,
|
||||
instanceId = instanceId,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -7102,6 +7108,7 @@ class NodeRuntime private constructor(
|
||||
private suspend fun resolveExecApprovalOnGateway(
|
||||
id: String,
|
||||
decision: String,
|
||||
instanceId: String?,
|
||||
) {
|
||||
val gatewayScope = captureGatewayDataScope() ?: return
|
||||
val methodsSnapshot = captureGatewayMethods()
|
||||
@@ -7111,14 +7118,18 @@ class NodeRuntime private constructor(
|
||||
synchronized(execApprovalsStateLock) {
|
||||
if (!operatorConnected || id in resolvedExecApprovalIds) return@synchronized
|
||||
val currentRows = _execApprovals.value
|
||||
if (currentRows.none { it.id == id && it.resolvingDecision == null }) return@synchronized
|
||||
val selectedRow =
|
||||
currentRows.firstOrNull {
|
||||
it.id == id && it.instanceId == instanceId && it.resolvingDecision == null
|
||||
} ?: return@synchronized
|
||||
if (pendingExecApprovalWrites.containsKey(id)) return@synchronized
|
||||
val pendingWrite =
|
||||
PendingExecApprovalWrite(
|
||||
gatewayScope.stableId,
|
||||
id,
|
||||
decision,
|
||||
currentRows.firstOrNull { it.id == id }?.createdAtMs,
|
||||
selectedRow.instanceId,
|
||||
selectedRow.createdAtMs,
|
||||
)
|
||||
pendingExecApprovalWrites[id] = pendingWrite
|
||||
registeredWrite = pendingWrite
|
||||
@@ -7135,7 +7146,14 @@ class NodeRuntime private constructor(
|
||||
val pendingWrite = registeredWrite
|
||||
if (!scopeCurrent || pendingWrite == null) return
|
||||
try {
|
||||
val resolution = submitExecApprovalResolution(gatewayScope, methodsSnapshot, id, decision)
|
||||
val resolution =
|
||||
submitExecApprovalResolution(
|
||||
gatewayScope,
|
||||
methodsSnapshot,
|
||||
id,
|
||||
decision,
|
||||
pendingWrite.instanceId,
|
||||
)
|
||||
markExecApprovalWriteRequestFinished(pendingWrite)
|
||||
publishGatewayApprovalData(gatewayScope, methodsSnapshot) {
|
||||
synchronized(execApprovalsStateLock) {
|
||||
@@ -7202,10 +7220,11 @@ class NodeRuntime private constructor(
|
||||
methodsSnapshot: GatewayMethodsSnapshot,
|
||||
id: String,
|
||||
decision: String,
|
||||
instanceId: String?,
|
||||
): GatewayExecApprovalResolution =
|
||||
when (methodsSnapshot.approvalRpcFamily) {
|
||||
GatewayApprovalRpcFamily.Canonical -> {
|
||||
val params = buildGatewayExecApprovalResolveParams(id, decision).toString()
|
||||
val params = buildGatewayExecApprovalResolveParams(id, decision, instanceId).toString()
|
||||
val response =
|
||||
requestGatewayApprovalData(
|
||||
gatewayScope = gatewayScope,
|
||||
@@ -7225,6 +7244,7 @@ class NodeRuntime private constructor(
|
||||
val legacyParams =
|
||||
buildJsonObject {
|
||||
put("id", JsonPrimitive(id))
|
||||
instanceId?.let { put("instanceId", JsonPrimitive(it)) }
|
||||
put("decision", JsonPrimitive(decision))
|
||||
}.toString()
|
||||
val legacyResponse =
|
||||
|
||||
@@ -2392,7 +2392,7 @@ internal data class SettingsMetric(
|
||||
@Composable
|
||||
private fun ExecApprovalsPanel(
|
||||
approvals: List<GatewayExecApprovalSummary>,
|
||||
onResolve: (String, String) -> Unit,
|
||||
onResolve: (String, String, String?) -> Unit,
|
||||
) {
|
||||
Column(verticalArrangement = Arrangement.spacedBy(10.dp)) {
|
||||
approvals.forEach { approval ->
|
||||
@@ -2407,7 +2407,7 @@ private fun ExecApprovalsPanel(
|
||||
@Composable
|
||||
private fun ExecApprovalCard(
|
||||
approval: GatewayExecApprovalSummary,
|
||||
onResolve: (String, String) -> Unit,
|
||||
onResolve: (String, String, String?) -> Unit,
|
||||
) {
|
||||
val resolving = approval.resolvingDecision != null
|
||||
ClawPanel {
|
||||
@@ -2434,14 +2434,14 @@ private fun ExecApprovalCard(
|
||||
if (action.decision == "allow-once") {
|
||||
ClawPrimaryButton(
|
||||
text = action.label,
|
||||
onClick = { onResolve(approval.id, action.decision) },
|
||||
onClick = { onResolve(approval.id, action.decision, approval.instanceId) },
|
||||
enabled = !resolving,
|
||||
modifier = Modifier.fillMaxWidth(),
|
||||
)
|
||||
} else {
|
||||
ClawSecondaryButton(
|
||||
text = action.label,
|
||||
onClick = { onResolve(approval.id, action.decision) },
|
||||
onClick = { onResolve(approval.id, action.decision, approval.instanceId) },
|
||||
enabled = !resolving,
|
||||
modifier = Modifier.fillMaxWidth(),
|
||||
)
|
||||
|
||||
@@ -297,6 +297,27 @@ class GatewayExecApprovalParsingTest {
|
||||
"""{"id":"approval-1","kind":"exec","decision":"deny"}""",
|
||||
buildGatewayExecApprovalResolveParams(id = "approval-1", decision = "deny").toString(),
|
||||
)
|
||||
assertEquals(
|
||||
"""{"id":"approval-1","instanceId":"instance:approval-1","kind":"exec","decision":"deny"}""",
|
||||
buildGatewayExecApprovalResolveParams(
|
||||
id = "approval-1",
|
||||
decision = "deny",
|
||||
instanceId = "instance:approval-1",
|
||||
).toString(),
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun parsesOptionalPendingInstanceId() {
|
||||
val payload =
|
||||
pendingGetPayload().replace(
|
||||
"\"status\": \"pending\"",
|
||||
"\"status\": \"pending\", \"instanceId\": \"instance:approval-1\"",
|
||||
)
|
||||
val pending =
|
||||
parseGatewayExecApprovalGetPayload(payload, json, expectedId = "approval-1")
|
||||
as? GatewayExecApprovalSnapshot.Pending
|
||||
assertEquals("instance:approval-1", pending?.summary?.instanceId)
|
||||
}
|
||||
|
||||
@Test
|
||||
|
||||
@@ -43,6 +43,69 @@ class GatewayExecApprovalRuntimeTest {
|
||||
.commit()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun canonicalResolveEchoesPendingInstanceId() =
|
||||
runBlocking {
|
||||
val runtime =
|
||||
approvalRuntime(approvals = listOf(approvalSummary(instanceId = "instance:approval-1")))
|
||||
var sentParams: String? = null
|
||||
runtime.gatewayDataRequestOverrideForTests = { _, method, params ->
|
||||
check(method == "approval.resolve")
|
||||
sentParams = params
|
||||
unifiedResolve(applied = true, status = "denied", decision = "deny")
|
||||
}
|
||||
|
||||
runtime.resolveExecApproval("approval-1", "deny", "instance:approval-1")
|
||||
waitUntil { runtime.execApprovals.value.isEmpty() }
|
||||
|
||||
assertEquals(
|
||||
"""{"id":"approval-1","instanceId":"instance:approval-1","kind":"exec","decision":"deny"}""",
|
||||
sentParams,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun staleInstanceCannotResolveSameIdReplacement() =
|
||||
runBlocking {
|
||||
val runtime =
|
||||
approvalRuntime(approvals = listOf(approvalSummary(instanceId = "instance:replacement")))
|
||||
var requestCount = 0
|
||||
runtime.gatewayDataRequestOverrideForTests = { _, _, _ ->
|
||||
requestCount += 1
|
||||
unifiedResolve(applied = true, status = "denied", decision = "deny")
|
||||
}
|
||||
|
||||
runtime.resolveExecApproval("approval-1", "deny", "instance:stale")
|
||||
delay(50)
|
||||
|
||||
assertEquals(0, requestCount)
|
||||
assertEquals("instance:replacement", runtime.execApprovals.value.single().instanceId)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun legacyResolveEchoesPendingInstanceId() =
|
||||
runBlocking {
|
||||
val runtime =
|
||||
approvalRuntime(
|
||||
methods = legacyMethods,
|
||||
approvals = listOf(approvalSummary(instanceId = "instance:approval-1")),
|
||||
)
|
||||
var sentParams: String? = null
|
||||
runtime.gatewayDataRequestOverrideForTests = { _, method, params ->
|
||||
check(method == "exec.approval.resolve")
|
||||
sentParams = params
|
||||
"""{"ok":true}"""
|
||||
}
|
||||
|
||||
runtime.resolveExecApproval("approval-1", "deny", "instance:approval-1")
|
||||
waitUntil { runtime.execApprovals.value.isEmpty() }
|
||||
|
||||
assertEquals(
|
||||
"""{"id":"approval-1","instanceId":"instance:approval-1","decision":"deny"}""",
|
||||
sentParams,
|
||||
)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun anotherSurfaceWinnerClosesLocalCardFromCanonicalResolveResult() =
|
||||
runBlocking {
|
||||
@@ -1091,9 +1154,11 @@ class GatewayExecApprovalRuntimeTest {
|
||||
private fun approvalSummary(
|
||||
id: String = "approval-1",
|
||||
commandText: String = "echo ok",
|
||||
instanceId: String? = null,
|
||||
): GatewayExecApprovalSummary =
|
||||
GatewayExecApprovalSummary(
|
||||
id = id,
|
||||
instanceId = instanceId,
|
||||
commandText = verbatimText(commandText),
|
||||
commandPreview = "echo",
|
||||
warningText = null,
|
||||
|
||||
@@ -12,6 +12,7 @@ function buildOptionsForAllowedDecisions(allowedDecisions: unknown) {
|
||||
sessionId: "session-1",
|
||||
event: {
|
||||
approvalId: "approval-1",
|
||||
instanceId: "instance:approval-1",
|
||||
command: "echo ok",
|
||||
},
|
||||
details: { allowedDecisions },
|
||||
@@ -34,6 +35,7 @@ describe("ACP permission relay helpers", () => {
|
||||
kind: "exec",
|
||||
status: "pending",
|
||||
approvalId: "approval-1",
|
||||
instanceId: "instance:approval-1",
|
||||
title: "Command approval requested",
|
||||
toolCallId: "tool-1",
|
||||
command: "echo stale",
|
||||
@@ -42,6 +44,7 @@ describe("ACP permission relay helpers", () => {
|
||||
if (!event) {
|
||||
throw new Error("approval event did not parse");
|
||||
}
|
||||
expect(event.instanceId).toBe("instance:approval-1");
|
||||
|
||||
expect(
|
||||
buildAcpPermissionRequest({
|
||||
@@ -95,6 +98,7 @@ describe("ACP permission relay helpers", () => {
|
||||
expect(
|
||||
parseGatewayExecApprovalRequestEventPayload({
|
||||
id: "approval-raw",
|
||||
instanceId: "instance:approval-raw",
|
||||
request: {
|
||||
command: "echo raw",
|
||||
host: "gateway",
|
||||
@@ -104,6 +108,7 @@ describe("ACP permission relay helpers", () => {
|
||||
}),
|
||||
).toEqual({
|
||||
approvalId: "approval-raw",
|
||||
instanceId: "instance:approval-raw",
|
||||
command: "echo raw",
|
||||
host: "gateway",
|
||||
toolCallId: "tool-raw",
|
||||
|
||||
@@ -10,6 +10,7 @@ export type GatewayExecApprovalDecision = "allow-once" | "allow-always" | "deny"
|
||||
|
||||
export type GatewayExecApprovalEvent = {
|
||||
approvalId: string;
|
||||
instanceId?: string;
|
||||
command?: string;
|
||||
host?: string;
|
||||
title?: string;
|
||||
@@ -87,6 +88,7 @@ export function parseGatewayExecApprovalEventData(
|
||||
}
|
||||
return {
|
||||
approvalId,
|
||||
instanceId: readNonEmptyString(data.instanceId),
|
||||
command: readNonEmptyString(data.command),
|
||||
host: readNonEmptyString(data.host),
|
||||
title: readNonEmptyString(data.title),
|
||||
@@ -106,6 +108,7 @@ export function parseGatewayExecApprovalRequestEventPayload(
|
||||
const requestRecord = request as Record<string, unknown>;
|
||||
return {
|
||||
approvalId,
|
||||
instanceId: readNonEmptyString(payload.instanceId),
|
||||
command:
|
||||
readNonEmptyString(requestRecord.command) ?? readNonEmptyString(requestRecord.commandPreview),
|
||||
host: readNonEmptyString(requestRecord.host),
|
||||
|
||||
@@ -173,16 +173,16 @@ export class AcpTranslatorAgentEvents {
|
||||
runId?: string,
|
||||
opts: { denyActive?: boolean } = {},
|
||||
): void {
|
||||
for (const [approvalId, relay] of this.approvalRelays) {
|
||||
for (const [relayKey, relay] of this.approvalRelays) {
|
||||
if (relay.sessionId !== sessionId) {
|
||||
continue;
|
||||
}
|
||||
if (runId && relay.runId !== runId) {
|
||||
continue;
|
||||
}
|
||||
this.approvalRelays.delete(approvalId);
|
||||
this.approvalRelays.delete(relayKey);
|
||||
if (opts.denyActive && relay.state === "active") {
|
||||
void this.resolveGatewayApproval(approvalId, "deny");
|
||||
void this.resolveGatewayApproval(relay, "deny");
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -209,7 +209,8 @@ export class AcpTranslatorAgentEvents {
|
||||
approvalEvent: GatewayExecApprovalEvent;
|
||||
}): void {
|
||||
const approvalEvent = params.approvalEvent;
|
||||
if (this.approvalRelays.has(approvalEvent.approvalId)) {
|
||||
const relayKey = this.approvalRelayKey(approvalEvent);
|
||||
if (this.approvalRelays.has(relayKey)) {
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -227,12 +228,14 @@ export class AcpTranslatorAgentEvents {
|
||||
|
||||
const relay: AcpPendingApprovalRelay = {
|
||||
approvalId: approvalEvent.approvalId,
|
||||
instanceId: approvalEvent.instanceId,
|
||||
relayKey,
|
||||
runId: pending.idempotencyKey,
|
||||
sessionId: pending.sessionId,
|
||||
sessionKey: pending.sessionKey,
|
||||
state: "active",
|
||||
};
|
||||
this.approvalRelays.set(relay.approvalId, relay);
|
||||
this.approvalRelays.set(relay.relayKey, relay);
|
||||
void this.runApprovalRelay(relay, correlatedApprovalEvent);
|
||||
}
|
||||
|
||||
@@ -273,7 +276,7 @@ export class AcpTranslatorAgentEvents {
|
||||
try {
|
||||
const details = await this.getGatewayApprovalDetails(relay.approvalId);
|
||||
if (!this.isApprovalRelayActive(relay)) {
|
||||
resolved = await this.resolveGatewayApproval(relay.approvalId, "deny");
|
||||
resolved = await this.resolveGatewayApproval(relay, "deny");
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -291,15 +294,15 @@ export class AcpTranslatorAgentEvents {
|
||||
}
|
||||
|
||||
const selectedDecision = this.isApprovalRelayActive(relay) && decision ? decision : "deny";
|
||||
resolved = await this.resolveGatewayApproval(relay.approvalId, selectedDecision);
|
||||
resolved = await this.resolveGatewayApproval(relay, selectedDecision);
|
||||
} finally {
|
||||
const current = this.approvalRelays.get(relay.approvalId);
|
||||
const current = this.approvalRelays.get(relay.relayKey);
|
||||
if (current === relay && current.state === "active") {
|
||||
if (resolved) {
|
||||
// Keep completed relays until prompt cleanup as replay/dedup sentinels.
|
||||
current.state = "completed";
|
||||
} else {
|
||||
this.approvalRelays.delete(relay.approvalId);
|
||||
this.approvalRelays.delete(relay.relayKey);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -319,29 +322,36 @@ export class AcpTranslatorAgentEvents {
|
||||
}
|
||||
|
||||
private async resolveGatewayApproval(
|
||||
approvalId: string,
|
||||
relay: AcpPendingApprovalRelay,
|
||||
decision: GatewayExecApprovalDecision,
|
||||
): Promise<boolean> {
|
||||
try {
|
||||
await this.gateway.request("exec.approval.resolve", {
|
||||
id: approvalId,
|
||||
id: relay.approvalId,
|
||||
...(relay.instanceId ? { instanceId: relay.instanceId } : {}),
|
||||
decision,
|
||||
});
|
||||
return true;
|
||||
} catch (err) {
|
||||
this.log(`approval relay resolve failed for ${approvalId}: ${String(err)}`);
|
||||
this.log(`approval relay resolve failed for ${relay.approvalId}: ${String(err)}`);
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
private isApprovalRelayActive(relay: AcpPendingApprovalRelay): boolean {
|
||||
return (
|
||||
this.approvalRelays.get(relay.approvalId) === relay &&
|
||||
this.approvalRelays.get(relay.relayKey) === relay &&
|
||||
relay.state === "active" &&
|
||||
this.getPendingPrompt(relay.sessionId, relay.runId) !== undefined
|
||||
);
|
||||
}
|
||||
|
||||
private approvalRelayKey(event: GatewayExecApprovalEvent): string {
|
||||
// Approval ids can be reused after settlement; the Gateway instance token
|
||||
// keeps an older ACP prompt from deduping or resolving its replacement.
|
||||
return `${event.approvalId}\u0000${event.instanceId ?? ""}`;
|
||||
}
|
||||
|
||||
private findUniquePendingBySessionKey(sessionKey: string): AcpPendingPrompt | undefined {
|
||||
let match: AcpPendingPrompt | undefined;
|
||||
for (const pending of this.pendingPrompts.values()) {
|
||||
|
||||
@@ -31,6 +31,7 @@ function createApprovalEvent(params: {
|
||||
runId: string;
|
||||
sessionKey?: string;
|
||||
toolCallId?: string;
|
||||
instanceId?: string;
|
||||
}): EventFrame {
|
||||
return {
|
||||
type: "event",
|
||||
@@ -45,6 +46,7 @@ function createApprovalEvent(params: {
|
||||
status: "pending",
|
||||
title: "Command approval requested",
|
||||
approvalId: params.approvalId ?? "approval-1",
|
||||
instanceId: params.instanceId,
|
||||
toolCallId: params.toolCallId,
|
||||
command: "echo event",
|
||||
host: "gateway",
|
||||
@@ -58,12 +60,14 @@ function createApprovalRequestEvent(params: {
|
||||
sessionKey?: string;
|
||||
command?: string;
|
||||
toolCallId?: string;
|
||||
instanceId?: string;
|
||||
}): EventFrame {
|
||||
return {
|
||||
type: "event",
|
||||
event: "exec.approval.requested",
|
||||
payload: {
|
||||
id: params.approvalId ?? "approval-1",
|
||||
instanceId: params.instanceId,
|
||||
createdAtMs: 1,
|
||||
expiresAtMs: 2,
|
||||
request: {
|
||||
@@ -169,7 +173,9 @@ function hasApprovalRelay(agent: AcpGatewayAgent, approvalId: string): boolean {
|
||||
approvalRelays: Map<string, unknown>;
|
||||
}
|
||||
).approvalRelays;
|
||||
return relayMap.has(approvalId);
|
||||
return [...relayMap.values()].some(
|
||||
(relay) => (relay as { approvalId?: string }).approvalId === approvalId,
|
||||
);
|
||||
}
|
||||
|
||||
function requireRecord(value: unknown): Record<string, unknown> {
|
||||
@@ -250,6 +256,31 @@ describe("ACP translator permission relay", () => {
|
||||
await cleanupHarness(harness);
|
||||
});
|
||||
|
||||
it("keeps same-id replacement relays distinct and echoes each instance", async () => {
|
||||
const harness = await createHarness();
|
||||
const first = createApprovalEvent({
|
||||
runId: harness.runId,
|
||||
approvalId: "approval-reused",
|
||||
instanceId: "instance:first",
|
||||
});
|
||||
const replacement = createApprovalEvent({
|
||||
runId: harness.runId,
|
||||
approvalId: "approval-reused",
|
||||
instanceId: "instance:replacement",
|
||||
});
|
||||
|
||||
await harness.agent.handleGatewayEvent(first);
|
||||
await vi.waitFor(() => expect(approvalResolveCalls(harness.request)).toHaveLength(1));
|
||||
await harness.agent.handleGatewayEvent(replacement);
|
||||
await vi.waitFor(() => expect(approvalResolveCalls(harness.request)).toHaveLength(2));
|
||||
|
||||
expect(approvalResolveCalls(harness.request).map(([, params]) => params)).toEqual([
|
||||
{ id: "approval-reused", instanceId: "instance:first", decision: "allow-once" },
|
||||
{ id: "approval-reused", instanceId: "instance:replacement", decision: "allow-once" },
|
||||
]);
|
||||
await cleanupHarness(harness);
|
||||
});
|
||||
|
||||
it("relays exec approval request events before the later agent approval event", async () => {
|
||||
const harness = await createHarness();
|
||||
const approvalId = "approval-raw";
|
||||
|
||||
@@ -23,6 +23,8 @@ export type AcpPendingPrompt = {
|
||||
|
||||
export type AcpPendingApprovalRelay = {
|
||||
approvalId: string;
|
||||
instanceId?: string;
|
||||
relayKey: string;
|
||||
runId: string;
|
||||
sessionId: string;
|
||||
sessionKey: string;
|
||||
|
||||
@@ -446,13 +446,14 @@ describe("GatewayChatClient", () => {
|
||||
(client as unknown as { client: { request: typeof request } }).client.request = request;
|
||||
|
||||
await expect(client.listPluginApprovals()).resolves.toEqual(pending);
|
||||
await expect(client.resolvePluginApproval("plugin:skill-1", "allow-once")).resolves.toEqual({
|
||||
ok: true,
|
||||
});
|
||||
await expect(
|
||||
client.resolvePluginApproval("plugin:skill-1", "allow-once", "instance:plugin-1"),
|
||||
).resolves.toEqual({ ok: true });
|
||||
|
||||
expect(request).toHaveBeenNthCalledWith(1, "plugin.approval.list", {});
|
||||
expect(request).toHaveBeenNthCalledWith(2, "plugin.approval.resolve", {
|
||||
id: "plugin:skill-1",
|
||||
instanceId: "instance:plugin-1",
|
||||
decision: "allow-once",
|
||||
});
|
||||
});
|
||||
|
||||
@@ -469,9 +469,10 @@ export class GatewayChatClient implements TuiBackend {
|
||||
return await this.client.request("plugin.approval.list", {});
|
||||
}
|
||||
|
||||
async resolvePluginApproval(id: string, decision: TuiApprovalDecision) {
|
||||
async resolvePluginApproval(id: string, decision: TuiApprovalDecision, instanceId?: string) {
|
||||
return await this.client.request<{ ok?: boolean }>("plugin.approval.resolve", {
|
||||
id,
|
||||
...(instanceId ? { instanceId } : {}),
|
||||
decision,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -42,6 +42,7 @@ export type TuiTaskSuggestionAcceptMode = NonNullable<TaskSuggestionsAcceptParam
|
||||
|
||||
export type TuiPluginApproval = {
|
||||
id: string;
|
||||
instanceId?: string;
|
||||
request: {
|
||||
title: string;
|
||||
description?: string | null;
|
||||
@@ -213,7 +214,11 @@ export type TuiBackend = {
|
||||
listModels: (opts?: { agentId?: string }) => Promise<TuiModelChoice[]>;
|
||||
listCommands?: (opts?: CommandsListParams) => Promise<CommandEntry[]>;
|
||||
listPluginApprovals?: () => Promise<unknown>;
|
||||
resolvePluginApproval?: (id: string, decision: TuiApprovalDecision) => Promise<{ ok?: boolean }>;
|
||||
resolvePluginApproval?: (
|
||||
id: string,
|
||||
decision: TuiApprovalDecision,
|
||||
instanceId?: string,
|
||||
) => Promise<{ ok?: boolean }>;
|
||||
getTaskSuggestionActionCapabilities?: () => TuiTaskSuggestionActionCapabilities;
|
||||
listTaskSuggestions?: () => Promise<TaskSuggestion[]>;
|
||||
listCloudWorkerProfiles?: () => Promise<string[]>;
|
||||
|
||||
@@ -15,6 +15,7 @@ type TestSelector = Component & {
|
||||
function approvalPayload(overrides: Record<string, unknown> = {}) {
|
||||
return {
|
||||
id: "plugin:skill-1",
|
||||
instanceId: "instance:plugin-1",
|
||||
request: {
|
||||
title: "Apply workspace skill proposal",
|
||||
description: "Apply a pending workspace skill proposal into live workspace skills.",
|
||||
@@ -151,7 +152,11 @@ describe("TUI plugin approvals", () => {
|
||||
harness.selectors[0]?.onSelectionChange?.({ value: "allow-once", label: "Allow once" });
|
||||
harness.selectors[0]?.onSelect?.({ value: "allow-once", label: "Allow once" });
|
||||
await vi.waitFor(() => {
|
||||
expect(harness.resolvePluginApproval).toHaveBeenCalledWith("plugin:skill-1", "allow-once");
|
||||
expect(harness.resolvePluginApproval).toHaveBeenCalledWith(
|
||||
"plugin:skill-1",
|
||||
"allow-once",
|
||||
"instance:plugin-1",
|
||||
);
|
||||
});
|
||||
expect(harness.closeOverlay).toHaveBeenCalledTimes(1);
|
||||
expect(harness.addSystem).toHaveBeenLastCalledWith("workspace skill approval: allowed once");
|
||||
@@ -308,7 +313,11 @@ describe("TUI plugin approvals", () => {
|
||||
|
||||
harness.selectors[0]?.onSelect?.({ value: "allow-once", label: "Allow once" });
|
||||
await vi.waitFor(() => {
|
||||
expect(harness.resolvePluginApproval).toHaveBeenCalledWith("plugin:skill-1", "allow-once");
|
||||
expect(harness.resolvePluginApproval).toHaveBeenCalledWith(
|
||||
"plugin:skill-1",
|
||||
"allow-once",
|
||||
"instance:plugin-1",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -166,6 +166,8 @@ function parseTuiPluginApproval(payload: unknown): TuiPluginApproval | null {
|
||||
}
|
||||
return {
|
||||
id,
|
||||
instanceId:
|
||||
typeof record.instanceId === "string" ? record.instanceId.trim() || undefined : undefined,
|
||||
request: {
|
||||
title,
|
||||
description: typeof request.description === "string" ? request.description : null,
|
||||
@@ -333,7 +335,11 @@ export function createTuiPluginApprovalController(deps: TuiPluginApprovalControl
|
||||
if (!deps.client.resolvePluginApproval) {
|
||||
throw new Error("plugin approval resolution is unavailable");
|
||||
}
|
||||
const result = await deps.client.resolvePluginApproval(approval.id, decision);
|
||||
const result = await deps.client.resolvePluginApproval(
|
||||
approval.id,
|
||||
decision,
|
||||
approval.instanceId,
|
||||
);
|
||||
if (result?.ok === false) {
|
||||
stale = true;
|
||||
} else {
|
||||
|
||||
Reference in New Issue
Block a user