diff --git a/apps/server/scripts/acp-mock-agent.ts b/apps/server/scripts/acp-mock-agent.ts index 3d11bc8e7083c..a10f9855cbbbf 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 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)); @@ -1265,6 +1266,21 @@ const program = Effect.gen(function* () { return yield* AcpError.AcpRequestError.internalError("Mock prompt failure"); } + if (emitTrackedToolCallKindThenHang !== undefined) { + yield* agent.client.sessionUpdate({ + sessionId: requestedSessionId, + update: { + sessionUpdate: "tool_call_update", + toolCallId: "tracked-tool-call", + title: "git status", + kind: emitTrackedToolCallKindThenHang as AcpSchema.ToolKind, + 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..8a68e92b89160 100644 --- a/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts +++ b/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts @@ -3115,6 +3115,155 @@ describe("AcpAdapterV2", () => { }).pipe(Effect.provide(layerTest), Effect.scoped), ); + 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; + 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_TRACKED_TOOL_CALL_KIND_THEN_HANG: kind }, + 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 === "tracked-tool-call", + ), + Stream.runHead, + ); + + const permissionFiber = yield* requestPermission( + { + sessionId: "mock-session-1", + toolCall: { toolCallId: "tracked-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: granted }, + ); + + 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; 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..e60671c775d51 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,40 @@ 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("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 87f24c5afdbaa..43fa2bf758e2f 100644 --- a/packages/provider-acp/src/server/runtimeModel.ts +++ b/packages/provider-acp/src/server/runtimeModel.ts @@ -1179,6 +1179,29 @@ 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 (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, kind, rawInput } = params.toolCall; + return { + ...params, + toolCall: { + ...params.toolCall, + ...(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 } + : {}), + }, + }; +} + export function parsePermissionRequest( params: EffectAcpSchema.RequestPermissionRequest, ): AcpPermissionRequest {