From b7bc6109f119d2ece4df5525fad642ffd6968fd5 Mon Sep 17 00:00:00 2001 From: Dallin Romney Date: Thu, 20 Aug 2026 23:36:06 -0700 Subject: [PATCH] fix(approvals): bind all clients to request instances --- .../ai/openclaw/app/GatewayExecApprovals.kt | 19 ++++-- .../java/ai/openclaw/app/MainViewModel.kt | 3 +- .../main/java/ai/openclaw/app/NodeRuntime.kt | 30 +++++++-- .../ai/openclaw/app/ui/SettingsScreens.kt | 8 +-- .../app/GatewayExecApprovalParsingTest.kt | 21 ++++++ .../app/GatewayExecApprovalRuntimeTest.kt | 65 +++++++++++++++++++ src/acp/permission-relay.test.ts | 5 ++ src/acp/permission-relay.ts | 3 + src/acp/translator.agent-events.ts | 36 ++++++---- src/acp/translator.permission-relay.test.ts | 33 +++++++++- src/acp/translator.prompt-state.ts | 2 + src/tui/gateway-chat.test.ts | 7 +- src/tui/gateway-chat.ts | 3 +- src/tui/tui-backend.ts | 7 +- src/tui/tui-plugin-approvals.test.ts | 13 +++- src/tui/tui-plugin-approvals.ts | 8 ++- 16 files changed, 227 insertions(+), 36 deletions(-) diff --git a/apps/android/app/src/main/java/ai/openclaw/app/GatewayExecApprovals.kt b/apps/android/app/src/main/java/ai/openclaw/app/GatewayExecApprovals.kt index 67bdfa35a4b3..3c3c1e1eb486 100644 --- a/apps/android/app/src/main/java/ai/openclaw/app/GatewayExecApprovals.kt +++ b/apps/android/app/src/main/java/ai/openclaw/app/GatewayExecApprovals.kt @@ -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): Boolean = keys == expected private fun JsonObject.hasOnlyKeys(allowed: Set): Boolean = keys.all(allowed::contains) diff --git a/apps/android/app/src/main/java/ai/openclaw/app/MainViewModel.kt b/apps/android/app/src/main/java/ai/openclaw/app/MainViewModel.kt index 1c392efaffee..ba432fd508db 100644 --- a/apps/android/app/src/main/java/ai/openclaw/app/MainViewModel.kt +++ b/apps/android/app/src/main/java/ai/openclaw/app/MainViewModel.kt @@ -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) { diff --git a/apps/android/app/src/main/java/ai/openclaw/app/NodeRuntime.kt b/apps/android/app/src/main/java/ai/openclaw/app/NodeRuntime.kt index fcf3f297d433..a12952fa2559 100644 --- a/apps/android/app/src/main/java/ai/openclaw/app/NodeRuntime.kt +++ b/apps/android/app/src/main/java/ai/openclaw/app/NodeRuntime.kt @@ -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 = diff --git a/apps/android/app/src/main/java/ai/openclaw/app/ui/SettingsScreens.kt b/apps/android/app/src/main/java/ai/openclaw/app/ui/SettingsScreens.kt index 492982a0596d..f4641999fced 100644 --- a/apps/android/app/src/main/java/ai/openclaw/app/ui/SettingsScreens.kt +++ b/apps/android/app/src/main/java/ai/openclaw/app/ui/SettingsScreens.kt @@ -2392,7 +2392,7 @@ internal data class SettingsMetric( @Composable private fun ExecApprovalsPanel( approvals: List, - 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(), ) diff --git a/apps/android/app/src/test/java/ai/openclaw/app/GatewayExecApprovalParsingTest.kt b/apps/android/app/src/test/java/ai/openclaw/app/GatewayExecApprovalParsingTest.kt index 4a6b402d1183..29c302d9c881 100644 --- a/apps/android/app/src/test/java/ai/openclaw/app/GatewayExecApprovalParsingTest.kt +++ b/apps/android/app/src/test/java/ai/openclaw/app/GatewayExecApprovalParsingTest.kt @@ -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 diff --git a/apps/android/app/src/test/java/ai/openclaw/app/GatewayExecApprovalRuntimeTest.kt b/apps/android/app/src/test/java/ai/openclaw/app/GatewayExecApprovalRuntimeTest.kt index a84061674845..1fd9dbe0c0cf 100644 --- a/apps/android/app/src/test/java/ai/openclaw/app/GatewayExecApprovalRuntimeTest.kt +++ b/apps/android/app/src/test/java/ai/openclaw/app/GatewayExecApprovalRuntimeTest.kt @@ -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, diff --git a/src/acp/permission-relay.test.ts b/src/acp/permission-relay.test.ts index d69a1297be17..48fa8ea5d507 100644 --- a/src/acp/permission-relay.test.ts +++ b/src/acp/permission-relay.test.ts @@ -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", diff --git a/src/acp/permission-relay.ts b/src/acp/permission-relay.ts index 5b2c4148e8c1..f5861f928547 100644 --- a/src/acp/permission-relay.ts +++ b/src/acp/permission-relay.ts @@ -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; return { approvalId, + instanceId: readNonEmptyString(payload.instanceId), command: readNonEmptyString(requestRecord.command) ?? readNonEmptyString(requestRecord.commandPreview), host: readNonEmptyString(requestRecord.host), diff --git a/src/acp/translator.agent-events.ts b/src/acp/translator.agent-events.ts index 7b3389bae2ff..a089e5130ca4 100644 --- a/src/acp/translator.agent-events.ts +++ b/src/acp/translator.agent-events.ts @@ -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 { 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()) { diff --git a/src/acp/translator.permission-relay.test.ts b/src/acp/translator.permission-relay.test.ts index f66d9e51af74..b138c5ec6be5 100644 --- a/src/acp/translator.permission-relay.test.ts +++ b/src/acp/translator.permission-relay.test.ts @@ -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; } ).approvalRelays; - return relayMap.has(approvalId); + return [...relayMap.values()].some( + (relay) => (relay as { approvalId?: string }).approvalId === approvalId, + ); } function requireRecord(value: unknown): Record { @@ -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"; diff --git a/src/acp/translator.prompt-state.ts b/src/acp/translator.prompt-state.ts index 440b69609141..a1c5d84fc219 100644 --- a/src/acp/translator.prompt-state.ts +++ b/src/acp/translator.prompt-state.ts @@ -23,6 +23,8 @@ export type AcpPendingPrompt = { export type AcpPendingApprovalRelay = { approvalId: string; + instanceId?: string; + relayKey: string; runId: string; sessionId: string; sessionKey: string; diff --git a/src/tui/gateway-chat.test.ts b/src/tui/gateway-chat.test.ts index 0156d8a13639..bc0746a64c08 100644 --- a/src/tui/gateway-chat.test.ts +++ b/src/tui/gateway-chat.test.ts @@ -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", }); }); diff --git a/src/tui/gateway-chat.ts b/src/tui/gateway-chat.ts index feac4ac333d9..5ad7f5c8b209 100644 --- a/src/tui/gateway-chat.ts +++ b/src/tui/gateway-chat.ts @@ -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, }); } diff --git a/src/tui/tui-backend.ts b/src/tui/tui-backend.ts index 44113ff909cd..9ffbeeec063a 100644 --- a/src/tui/tui-backend.ts +++ b/src/tui/tui-backend.ts @@ -42,6 +42,7 @@ export type TuiTaskSuggestionAcceptMode = NonNullable Promise; listCommands?: (opts?: CommandsListParams) => Promise; listPluginApprovals?: () => Promise; - resolvePluginApproval?: (id: string, decision: TuiApprovalDecision) => Promise<{ ok?: boolean }>; + resolvePluginApproval?: ( + id: string, + decision: TuiApprovalDecision, + instanceId?: string, + ) => Promise<{ ok?: boolean }>; getTaskSuggestionActionCapabilities?: () => TuiTaskSuggestionActionCapabilities; listTaskSuggestions?: () => Promise; listCloudWorkerProfiles?: () => Promise; diff --git a/src/tui/tui-plugin-approvals.test.ts b/src/tui/tui-plugin-approvals.test.ts index 2482d2457bb0..b8a6634c02bb 100644 --- a/src/tui/tui-plugin-approvals.test.ts +++ b/src/tui/tui-plugin-approvals.test.ts @@ -15,6 +15,7 @@ type TestSelector = Component & { function approvalPayload(overrides: Record = {}) { 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", + ); }); }); diff --git a/src/tui/tui-plugin-approvals.ts b/src/tui/tui-plugin-approvals.ts index 0b47f8a1f91d..94ed8b05f512 100644 --- a/src/tui/tui-plugin-approvals.ts +++ b/src/tui/tui-plugin-approvals.ts @@ -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 {