From 1adfa5c4b42550180764b9cf4c64ae88173f6799 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Wed, 28 Oct 2026 20:04:53 +0000 Subject: [PATCH 1/4] test(acp): reproduce #26620 ID-only permission losing the tracked command An ACP agent in Supervised mode reports its execute tool call, then asks session/request_permission with only the toolCallId. The approval prompt falls back to "Session " and approving it records no execute grant, so the agent's terminal/create is refused. These tests reproduce #26620. Co-Authored-By: Claude Opus 5.5 --- apps/server/scripts/acp-mock-agent.ts | 16 ++ .../Adapters/AcpAdapterV2.test.ts | 138 ++++++++++++++++++ 2 files changed, 154 insertions(+) diff --git a/apps/server/scripts/acp-mock-agent.ts b/apps/server/scripts/acp-mock-agent.ts index 3d11bc8e7083c..bf7d1f9dbab9d 100644 --- a/apps/server/scripts/acp-mock-agent.ts +++ b/apps/server/scripts/acp-mock-agent.ts @@ -38,6 +38,7 @@ const omitInitializeModelState = process.env.T3_ACP_OMIT_INITIALIZE_MODEL_STATE const completeFirstPromptOnCancel = process.env.T3_ACP_COMPLETE_FIRST_PROMPT_ON_CANCEL === "1"; const floodStderr = process.env.T3_ACP_FLOOD_STDERR === "1"; const hangPromptForever = process.env.T3_ACP_HANG_PROMPT_FOREVER === "1"; +const emitExecuteToolCallThenHang = process.env.T3_ACP_EMIT_EXECUTE_TOOL_CALL_THEN_HANG === "1"; const clientFsProbePath = process.env.T3_ACP_CLIENT_FS_PROBE_PATH; const clientFsProbeLogPath = process.env.T3_ACP_CLIENT_FS_PROBE_LOG_PATH; const encodeJson = Schema.encodeSync(Schema.fromJsonString(Schema.Unknown)); @@ -1265,6 +1266,21 @@ const program = Effect.gen(function* () { return yield* AcpError.AcpRequestError.internalError("Mock prompt failure"); } + if (emitExecuteToolCallThenHang) { + yield* agent.client.sessionUpdate({ + sessionId: requestedSessionId, + update: { + sessionUpdate: "tool_call_update", + toolCallId: "execute-tool-call", + title: "git status", + kind: "execute", + status: "pending", + rawInput: { command: "git status" }, + }, + }); + return yield* Effect.never; + } + if (hangPromptForever || (emitEmptySuccessfulBashThenHang && promptCount === 2)) { return yield* Effect.never; } diff --git a/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts b/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts index f05ace7ff750d..25ad04325f839 100644 --- a/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts +++ b/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts @@ -3115,6 +3115,144 @@ describe("AcpAdapterV2", () => { }).pipe(Effect.provide(layerTest), Effect.scoped), ); + it.effect("resolves an ID-only permission request from the tracked execute tool call", () => + Effect.gen(function* () { + const childProcessSpawner = yield* ChildProcessSpawner.ChildProcessSpawner; + const path = yield* Path.Path; + const selfInvocation = yield* resolveSelfInvocation(); + const mockAgentPath = yield* path.fromFileUrl( + new URL("../../../scripts/acp-mock-agent.ts", import.meta.url), + ); + type RuntimeService = AcpSessionRuntime.AcpSessionRuntime["Service"]; + let requestPermission: Parameters[0] | undefined; + let createTerminal: Parameters[0] | undefined; + let runtimeInput: AcpAdapterV2RuntimeInput | undefined; + const makeRuntime = makeMockRuntime({ + childProcessSpawner, + mockAgentPath, + environment: { T3_ACP_EMIT_EXECUTE_TOOL_CALL_THEN_HANG: "1" }, + wrapRuntime: (runtime) => ({ + ...runtime, + handleRequestPermission: (handler) => + Effect.sync(() => { + requestPermission = handler; + }).pipe(Effect.andThen(runtime.handleRequestPermission(handler))), + handleCreateTerminal: (handler) => + Effect.sync(() => { + createTerminal = handler; + }).pipe(Effect.andThen(runtime.handleCreateTerminal(handler))), + }), + }); + const instanceId = ProviderInstanceId.make("acp-test-id-only-permission"); + const adapter = yield* makeAcpAdapterV2({ + instanceId, + flavor: { + driver: ACP_TEST_DRIVER, + capabilities: AcpProviderCapabilitiesV2, + interruptPromptOnCancel: true, + makeRuntime: (input) => + Effect.sync(() => { + runtimeInput = input; + }).pipe(Effect.andThen(makeRuntime(input))), + }, + selfInvocation, + clientTerminals: {}, + }); + const threadId = ThreadId.make("thread-acp-id-only-permission"); + const runtimePolicy = ProviderAdapter.ProviderAdapterV2RuntimePolicy.make({ + runtimeMode: "approval-required", + interactionMode: "default", + cwd: process.cwd(), + }); + const modelSelection = { instanceId, model: "default" } as const; + const runtime = yield* adapter.openSession({ + threadId, + providerSessionId: ProviderSessionId.make("provider-session-acp-id-only-permission"), + modelSelection, + runtimePolicy, + }); + const providerThread = yield* runtime.ensureThread({ + threadId, + modelSelection, + runtimePolicy, + }); + const turnFiber = yield* runtime + .startTurn( + makeTurnInput({ + threadId, + providerThread, + instanceId, + runtimePolicy, + now: yield* DateTime.now, + }), + ) + .pipe(Effect.forkScoped); + if (requestPermission === undefined || createTerminal === undefined) { + return yield* Effect.die("ACP runtime must register permission and terminal handlers"); + } + yield* runtime.events.pipe( + Stream.filter( + (event) => + event.type === "turn_item.updated" && + event.turnItem.nativeItemRef?.nativeId === "execute-tool-call", + ), + Stream.runHead, + ); + + const permissionFiber = yield* requestPermission( + { + sessionId: "mock-session-1", + toolCall: { toolCallId: "execute-tool-call" }, + options: [{ optionId: "allow-once", name: "Allow once", kind: "allow_once" }], + }, + { requestId: "id-only-permission-request", method: "session/request_permission" }, + ).pipe(Effect.forkScoped); + const approvalEvent = Option.getOrThrow( + yield* runtime.events.pipe( + Stream.filter( + (event) => + event.type === "turn_item.updated" && event.turnItem.type === "approval_request", + ), + Stream.runHead, + ), + ); + if ( + approvalEvent.type !== "turn_item.updated" || + approvalEvent.turnItem.type !== "approval_request" || + approvalEvent.turnItem.providerTurnId === null + ) { + return yield* Effect.die("Expected an ACP approval request"); + } + const approval = approvalEvent.turnItem; + const providerTurnId = approvalEvent.turnItem.providerTurnId; + const responseFiber = yield* runtime + .respondToRuntimeRequest({ requestId: approval.requestId, decision: "accept" }) + .pipe(Effect.forkScoped); + assert.equal((yield* Fiber.join(permissionFiber)).outcome.outcome, "selected"); + if (runtimeInput?.onOutgoingResponse === undefined) { + return yield* Effect.die("ACP runtime must expose native response acknowledgements"); + } + yield* runtimeInput.onOutgoingResponse("id-only-permission-request"); + yield* Fiber.join(responseFiber); + + const terminalExit = yield* createTerminal( + { + sessionId: "mock-session-1", + command: process.execPath, + args: ["-e", ""], + }, + { requestId: "id-only-permission-terminal", method: "terminal/create" }, + ).pipe(Effect.exit); + assert.deepEqual( + { prompt: approval.prompt, terminalCreated: Exit.isSuccess(terminalExit) }, + { prompt: "git status", terminalCreated: true }, + ); + + yield* runtime.interruptTurn({ providerThread, providerTurnId }); + yield* Fiber.join(turnFiber); + }).pipe(Effect.provide(layerTest), Effect.scoped), + ); + it.effect("replaces the ACP session and clears conversation state on rollback", () => Effect.gen(function* () { const childProcessSpawner = yield* ChildProcessSpawner.ChildProcessSpawner; From 801d933f1c243e597ffb88f088d2df99539a791c Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Wed, 28 Oct 2026 20:13:45 +0000 Subject: [PATCH 2/4] fix(acp): ID-only permission requests use the tracked tool call ACP allows session/request_permission to name its tool call by ID only. Devin does this after announcing the execute call, so the prompt showed "Session ", the kind was unknown, and approving recorded no command grant, which made the agent's terminal/create fail. The permission handler now fills the omitted title, kind and rawInput from the tool call the active turn tracks for that toolCallId in the same session, before the prompt, disposition, flavor hooks and grant read the request. Fields the request sends still win, and an untracked toolCallId is left unchanged. Co-Authored-By: Claude Opus 5.5 --- packages/provider-acp/src/server/adapter.ts | 11 +++++++++- .../src/server/runtimeModel.test.ts | 18 +++++++++++++++ .../provider-acp/src/server/runtimeModel.ts | 22 +++++++++++++++++++ 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/packages/provider-acp/src/server/adapter.ts b/packages/provider-acp/src/server/adapter.ts index b88e31d203948..1cdbd20f5508b 100644 --- a/packages/provider-acp/src/server/adapter.ts +++ b/packages/provider-acp/src/server/adapter.ts @@ -76,6 +76,7 @@ import { parsePermissionRequest, parseSessionUpdateEvent, toolCallVisibleOutputChanged, + withTrackedPermissionToolCall, type AcpPlanUpdate, type AcpAgentTerminalState, type AcpSessionModeState, @@ -5974,8 +5975,16 @@ export const makeAcpAdapterV2 = Effect.fn("makeAcpAdapterV2")(function* ( projectAcpRuntimeSessionUpdate(handlerGeneration, rawNotification), ); } - yield* targetRuntime.handleRequestPermission((params, requestContext) => + yield* targetRuntime.handleRequestPermission((nativeParams, requestContext) => Effect.gen(function* () { + const trackedTurn = yield* Ref.get(activeTurn); + const params = + trackedTurn !== null && nativeParams.sessionId === (yield* Ref.get(activeSessionId)) + ? withTrackedPermissionToolCall( + nativeParams, + trackedTurn.tools.get(nativeParams.toolCall.toolCallId), + ) + : nativeParams; const transportRequestId = requestContext.requestId; const permissionQuestion = flavor.extractPermissionQuestion?.(params); if (permissionQuestion !== undefined) { diff --git a/packages/provider-acp/src/server/runtimeModel.test.ts b/packages/provider-acp/src/server/runtimeModel.test.ts index 70682ec4824a5..33b0cd2735663 100644 --- a/packages/provider-acp/src/server/runtimeModel.test.ts +++ b/packages/provider-acp/src/server/runtimeModel.test.ts @@ -17,6 +17,7 @@ import { sessionUpdateIsReplay, syntheticLoadSessionResponseFromInitialize, toolCallProgressLength, + withTrackedPermissionToolCall, type AcpToolCallState, } from "./runtimeModel.ts"; @@ -432,6 +433,23 @@ describe("AcpRuntimeModel", () => { }); }); + it("keeps an explicit permission request kind over the tracked tool call's kind", () => { + const request = withTrackedPermissionToolCall( + { + sessionId: "session-1", + options: [{ optionId: "allow-once", name: "Allow once", kind: "allow_once" }], + toolCall: { toolCallId: "tool-1", kind: "read" }, + }, + { + toolCallId: "tool-1", + kind: "execute", + data: { toolCallId: "tool-1", title: "git status", rawInput: { command: "git status" } }, + }, + ); + + expect(parsePermissionRequest(request)).toMatchObject({ kind: "read", detail: "git status" }); + }); + it("bounds an oversized cumulative tool_call_update content buffer to a tail window", () => { const hugeText = Array.from({ length: 2_000 }, (_, i) => `line ${i}: ${"x".repeat(50)}`).join( "\n", diff --git a/packages/provider-acp/src/server/runtimeModel.ts b/packages/provider-acp/src/server/runtimeModel.ts index 87f24c5afdbaa..63193c6384de1 100644 --- a/packages/provider-acp/src/server/runtimeModel.ts +++ b/packages/provider-acp/src/server/runtimeModel.ts @@ -1179,6 +1179,28 @@ export function embeddedTerminalIdsFromSessionUpdate( return terminalIds.length === 0 ? undefined : { toolCallId: update.toolCallId, terminalIds }; } +/** ACP lets a permission request name its tool call by ID only; omitted fields come from the tracked call. */ +export function withTrackedPermissionToolCall( + params: EffectAcpSchema.RequestPermissionRequest, + tracked: AcpToolCallState | undefined, +): EffectAcpSchema.RequestPermissionRequest { + if (tracked === undefined) return params; + const title = + params.toolCall.title ?? + (typeof tracked.data.title === "string" ? tracked.data.title : undefined); + const kind = params.toolCall.kind ?? tracked.kind; + const rawInput = params.toolCall.rawInput ?? tracked.data.rawInput; + return { + ...params, + toolCall: { + ...params.toolCall, + ...(title === undefined ? {} : { title }), + ...(kind === undefined ? {} : { kind }), + ...(rawInput === undefined ? {} : { rawInput }), + }, + }; +} + export function parsePermissionRequest( params: EffectAcpSchema.RequestPermissionRequest, ): AcpPermissionRequest { From 51c60ebe0047cdba1d6500e50031270b1e3f4d98 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Wed, 28 Oct 2026 20:31:23 +0000 Subject: [PATCH 3/4] fix(acp): only a tracked execute kind grants terminal execution Co-Authored-By: Claude Opus 5.5 --- apps/server/scripts/acp-mock-agent.ts | 8 +++---- .../Adapters/AcpAdapterV2.test.ts | 21 ++++++++++++++----- .../provider-acp/src/server/runtimeModel.ts | 3 ++- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/apps/server/scripts/acp-mock-agent.ts b/apps/server/scripts/acp-mock-agent.ts index bf7d1f9dbab9d..a10f9855cbbbf 100644 --- a/apps/server/scripts/acp-mock-agent.ts +++ b/apps/server/scripts/acp-mock-agent.ts @@ -38,7 +38,7 @@ const omitInitializeModelState = process.env.T3_ACP_OMIT_INITIALIZE_MODEL_STATE const completeFirstPromptOnCancel = process.env.T3_ACP_COMPLETE_FIRST_PROMPT_ON_CANCEL === "1"; const floodStderr = process.env.T3_ACP_FLOOD_STDERR === "1"; const hangPromptForever = process.env.T3_ACP_HANG_PROMPT_FOREVER === "1"; -const emitExecuteToolCallThenHang = process.env.T3_ACP_EMIT_EXECUTE_TOOL_CALL_THEN_HANG === "1"; +const emitTrackedToolCallKindThenHang = process.env.T3_ACP_EMIT_TRACKED_TOOL_CALL_KIND_THEN_HANG; const clientFsProbePath = process.env.T3_ACP_CLIENT_FS_PROBE_PATH; const clientFsProbeLogPath = process.env.T3_ACP_CLIENT_FS_PROBE_LOG_PATH; const encodeJson = Schema.encodeSync(Schema.fromJsonString(Schema.Unknown)); @@ -1266,14 +1266,14 @@ const program = Effect.gen(function* () { return yield* AcpError.AcpRequestError.internalError("Mock prompt failure"); } - if (emitExecuteToolCallThenHang) { + if (emitTrackedToolCallKindThenHang !== undefined) { yield* agent.client.sessionUpdate({ sessionId: requestedSessionId, update: { sessionUpdate: "tool_call_update", - toolCallId: "execute-tool-call", + toolCallId: "tracked-tool-call", title: "git status", - kind: "execute", + kind: emitTrackedToolCallKindThenHang as AcpSchema.ToolKind, status: "pending", rawInput: { command: "git status" }, }, diff --git a/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts b/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts index 25ad04325f839..8a68e92b89160 100644 --- a/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts +++ b/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts @@ -3115,7 +3115,18 @@ describe("AcpAdapterV2", () => { }).pipe(Effect.provide(layerTest), Effect.scoped), ); - it.effect("resolves an ID-only permission request from the tracked execute tool call", () => + it.effect.each([ + { + label: "grants an ID-only execute approval its tracked command", + kind: "execute", + granted: true, + }, + { + label: "does not grant execution to an ID-only approval of another tool", + kind: "other", + granted: false, + }, + ] as const)("$label", ({ kind, granted }) => Effect.gen(function* () { const childProcessSpawner = yield* ChildProcessSpawner.ChildProcessSpawner; const path = yield* Path.Path; @@ -3130,7 +3141,7 @@ describe("AcpAdapterV2", () => { const makeRuntime = makeMockRuntime({ childProcessSpawner, mockAgentPath, - environment: { T3_ACP_EMIT_EXECUTE_TOOL_CALL_THEN_HANG: "1" }, + environment: { T3_ACP_EMIT_TRACKED_TOOL_CALL_KIND_THEN_HANG: kind }, wrapRuntime: (runtime) => ({ ...runtime, handleRequestPermission: (handler) => @@ -3194,7 +3205,7 @@ describe("AcpAdapterV2", () => { Stream.filter( (event) => event.type === "turn_item.updated" && - event.turnItem.nativeItemRef?.nativeId === "execute-tool-call", + event.turnItem.nativeItemRef?.nativeId === "tracked-tool-call", ), Stream.runHead, ); @@ -3202,7 +3213,7 @@ describe("AcpAdapterV2", () => { const permissionFiber = yield* requestPermission( { sessionId: "mock-session-1", - toolCall: { toolCallId: "execute-tool-call" }, + toolCall: { toolCallId: "tracked-tool-call" }, options: [{ optionId: "allow-once", name: "Allow once", kind: "allow_once" }], }, { requestId: "id-only-permission-request", method: "session/request_permission" }, @@ -3245,7 +3256,7 @@ describe("AcpAdapterV2", () => { ).pipe(Effect.exit); assert.deepEqual( { prompt: approval.prompt, terminalCreated: Exit.isSuccess(terminalExit) }, - { prompt: "git status", terminalCreated: true }, + { prompt: "git status", terminalCreated: granted }, ); yield* runtime.interruptTurn({ providerThread, providerTurnId }); diff --git a/packages/provider-acp/src/server/runtimeModel.ts b/packages/provider-acp/src/server/runtimeModel.ts index 63193c6384de1..a1256bcec1bf7 100644 --- a/packages/provider-acp/src/server/runtimeModel.ts +++ b/packages/provider-acp/src/server/runtimeModel.ts @@ -1188,7 +1188,8 @@ export function withTrackedPermissionToolCall( const title = params.toolCall.title ?? (typeof tracked.data.title === "string" ? tracked.data.title : undefined); - const kind = params.toolCall.kind ?? tracked.kind; + // Unrecognised kinds map to command approvals, so only a tracked execute kind may grant execution. + const kind = params.toolCall.kind ?? (tracked.kind === "execute" ? "execute" : undefined); const rawInput = params.toolCall.rawInput ?? tracked.data.rawInput; return { ...params, From 553a9cbc6d0b54c0c1bd748de2156e96a634724f Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Wed, 28 Oct 2026 20:43:24 +0000 Subject: [PATCH 4/4] fix(acp): explicitly cleared permission fields are not refilled from the tracked call Co-Authored-By: Claude Opus 5.5 --- .../src/server/runtimeModel.test.ts | 17 ++++++++++++++++ .../provider-acp/src/server/runtimeModel.ts | 20 +++++++++---------- 2 files changed, 27 insertions(+), 10 deletions(-) diff --git a/packages/provider-acp/src/server/runtimeModel.test.ts b/packages/provider-acp/src/server/runtimeModel.test.ts index 33b0cd2735663..e60671c775d51 100644 --- a/packages/provider-acp/src/server/runtimeModel.test.ts +++ b/packages/provider-acp/src/server/runtimeModel.test.ts @@ -450,6 +450,23 @@ describe("AcpRuntimeModel", () => { expect(parsePermissionRequest(request)).toMatchObject({ kind: "read", detail: "git status" }); }); + it("keeps explicitly cleared permission request fields over the tracked tool call", () => { + const request = withTrackedPermissionToolCall( + { + sessionId: "session-1", + options: [{ optionId: "allow-once", name: "Allow once", kind: "allow_once" }], + toolCall: { toolCallId: "tool-1", kind: null, title: null, rawInput: null }, + }, + { + toolCallId: "tool-1", + kind: "execute", + data: { toolCallId: "tool-1", title: "git status", rawInput: { command: "git status" } }, + }, + ); + + expect(parsePermissionRequest(request).kind).toBe("unknown"); + }); + it("bounds an oversized cumulative tool_call_update content buffer to a tail window", () => { const hugeText = Array.from({ length: 2_000 }, (_, i) => `line ${i}: ${"x".repeat(50)}`).join( "\n", diff --git a/packages/provider-acp/src/server/runtimeModel.ts b/packages/provider-acp/src/server/runtimeModel.ts index a1256bcec1bf7..43fa2bf758e2f 100644 --- a/packages/provider-acp/src/server/runtimeModel.ts +++ b/packages/provider-acp/src/server/runtimeModel.ts @@ -1179,25 +1179,25 @@ export function embeddedTerminalIdsFromSessionUpdate( return terminalIds.length === 0 ? undefined : { toolCallId: update.toolCallId, terminalIds }; } -/** ACP lets a permission request name its tool call by ID only; omitted fields come from the tracked call. */ +/** ACP lets a permission request name its tool call by ID only; omitted (not null) fields come from the tracked call. */ export function withTrackedPermissionToolCall( params: EffectAcpSchema.RequestPermissionRequest, tracked: AcpToolCallState | undefined, ): EffectAcpSchema.RequestPermissionRequest { if (tracked === undefined) return params; - const title = - params.toolCall.title ?? - (typeof tracked.data.title === "string" ? tracked.data.title : undefined); - // Unrecognised kinds map to command approvals, so only a tracked execute kind may grant execution. - const kind = params.toolCall.kind ?? (tracked.kind === "execute" ? "execute" : undefined); - const rawInput = params.toolCall.rawInput ?? tracked.data.rawInput; + const { title, kind, rawInput } = params.toolCall; return { ...params, toolCall: { ...params.toolCall, - ...(title === undefined ? {} : { title }), - ...(kind === undefined ? {} : { kind }), - ...(rawInput === undefined ? {} : { rawInput }), + ...(title === undefined && typeof tracked.data.title === "string" + ? { title: tracked.data.title } + : {}), + // Unrecognised kinds map to command approvals, so only a tracked execute kind may grant execution. + ...(kind === undefined && tracked.kind === "execute" ? { kind: "execute" as const } : {}), + ...(rawInput === undefined && tracked.data.rawInput !== undefined + ? { rawInput: tracked.data.rawInput } + : {}), }, }; }