diff --git a/src/permission/reactor-authorize.ts b/src/permission/reactor-authorize.ts index 3f44afdd8..496f64396 100644 --- a/src/permission/reactor-authorize.ts +++ b/src/permission/reactor-authorize.ts @@ -31,6 +31,7 @@ import { type WorkerGrantStore, } from "./worker-grant.js"; import { canonicalToolName } from "../agent/canonical-tool-name.js"; +import { prepareDispatchedToolCall } from "../agent/tool-aliases.js"; import { getSubAgentIdentity } from "../subagent/identity-context.js"; import { FLEET_VERBS, @@ -100,6 +101,23 @@ function resolveWorkerCwd(cwd: string | undefined): string { return cwd ?? getSubAgentIdentity()?.cwd ?? process.cwd(); } +/** + * The call as DynamicToolRunner dispatches it: canonical engine name plus + * hidden-alias argument coercion (gpt `shell` -> run_shell via + * coerceShellArgs). Authorize sees the raw wire call; the middleware that + * consumes an authorized retry sees the dispatched call. Keying both on the + * dispatched shape keeps one fingerprint per executed call. Invalid hidden + * arguments never dispatch, so they keep their raw shape. + */ +function dispatchedCall(call: ToolCallType): ToolCallType { + const engine = canonicalToolName(call.name); + try { + return prepareDispatchedToolCall(call, engine); + } catch { + return { ...call, name: engine }; + } +} + function workerCallIdentity( sessionId: string, call: ToolCallType, @@ -110,10 +128,11 @@ function workerCallIdentity( args: Record; cwd: string; } { + const dispatched = dispatchedCall(call); return { sessionId, - canonicalTool: canonicalToolName(call.name), - args: (call.arguments ?? {}) as Record, + canonicalTool: dispatched.name, + args: (dispatched.arguments ?? {}) as Record, cwd, }; } diff --git a/src/permission/worker-grant-dispatch-identity.test.ts b/src/permission/worker-grant-dispatch-identity.test.ts new file mode 100644 index 000000000..a419ae7fd --- /dev/null +++ b/src/permission/worker-grant-dispatch-identity.test.ts @@ -0,0 +1,147 @@ +// gpt-family workers call run_shell as `shell`. Reactor authz sees that raw +// wire call; the permission middleware sees DynamicToolRunner's +// prepareDispatchedToolCall output, whose coerceShellArgs rebuilds the +// arguments. The authorized retry must match the same envelope at both +// stages. +import { describe, expect, test } from "bun:test"; +import type { ToolCall } from "@intx/types/runtime"; + +import { prepareDispatchedToolCall } from "../agent/tool-aliases.js"; +import { gateToolCall } from "../plugins/permission-plugin.js"; +import { createPermissionGate } from "./gate.js"; +import { workerPermissionGate } from "./reactor-authorize.js"; +import { WorkerGrantStore } from "./worker-grant.js"; + +const CWD = process.cwd(); + +// The call shape a gpt-family worker sends for run_shell. +const wireCall = (id: string): ToolCall => ({ + id, + name: "shell", + arguments: { + command: "git status --short", + timeout: 120000, + cwd: CWD, + background: false, + stop: "", + }, +}); + +function extractRequestId(reason: string): string { + const match = /grant request ([0-9a-f-]{36})/.exec(reason); + if (match?.[1] === undefined) throw new Error(`no grant id: ${reason}`); + return match[1]; +} + +describe("gpt-family worker authorized retry identity", () => { + test("approved retry of a `shell` call executes once instead of minting a new grant request", async () => { + // Parent: reactor-gated interactive gate, nothing pre-approved, no auto. + const parentGate = createPermissionGate({ + approvals: [], + cwd: CWD, + skipPermissions: false, + interactive: true, + reactorGated: true, + auto: false, + requestApproval: async () => ({ allow: false }), + }); + const store = new WorkerGrantStore(); + const workerGate = workerPermissionGate(parentGate, { + sessionId: "worker-gpt", + store, + }); + + // 1. First attempt: reactor authz denies and records envelope A. + const denied = await workerGate.authorizeCall(wireCall("call-1")); + if (denied.effect !== "deny") throw new Error("expected worker deny"); + const requestId = extractRequestId(denied.reason); + const envelope = store.peek(requestId); + expect(envelope?.canonicalTool).toBe("run_shell"); + + // 2. approve_worker_call: parent approved the frozen call (allow-once, + // no session grant) and flipped the envelope to authorized. + expect(store.authorizeRetry(requestId)).toBe(true); + + // 3. Retry, stage 1: reactor authz on the raw wire call allows via + // hasAuthorizedRetry (does not consume). + const retryAuthz = await workerGate.authorizeCall(wireCall("call-2")); + expect(retryAuthz).toEqual({ effect: "allow" }); + expect(store.peek(requestId)?.status).toBe("authorized"); + + // 4. Retry, stage 2: DynamicToolRunner coerces the `shell` call onto + // run_shell, then the posix middleware runs gateToolCall. + const dispatched = prepareDispatchedToolCall( + wireCall("call-2"), + "run_shell", + ); + expect(dispatched.arguments).not.toEqual(wireCall("x").arguments); + + let executed = 0; + const result = await gateToolCall( + workerGate, + dispatched, + new AbortController().signal, + async (call) => { + executed++; + return { callId: call.id, content: "ran" }; + }, + ); + + expect(result.isError).not.toBe(true); + expect(executed).toBe(1); + expect(store.peek(requestId)?.status).toBe("consumed"); + + // One grant, one execution: a replay of the same call fails closed at + // both stages, and no second envelope was minted. + const replay = await gateToolCall( + workerGate, + prepareDispatchedToolCall(wireCall("call-3"), "run_shell"), + new AbortController().signal, + async (call) => { + executed++; + return { callId: call.id, content: "ran" }; + }, + ); + expect(replay.isError).toBe(true); + expect(String(replay.content)).toContain("already consumed"); + expect((await workerGate.authorizeCall(wireCall("call-4"))).effect).toBe( + "deny", + ); + expect(executed).toBe(1); + }); + + test("an altered retry (different command) still fails closed", async () => { + const parentGate = createPermissionGate({ + approvals: [], + cwd: CWD, + skipPermissions: false, + interactive: true, + reactorGated: true, + auto: false, + requestApproval: async () => ({ allow: false }), + }); + const store = new WorkerGrantStore(); + const workerGate = workerPermissionGate(parentGate, { + sessionId: "worker-gpt-alt", + store, + }); + const denied = await workerGate.authorizeCall(wireCall("call-1")); + if (denied.effect !== "deny") throw new Error("expected worker deny"); + const requestId = extractRequestId(denied.reason); + expect(store.authorizeRetry(requestId)).toBe(true); + + const altered: ToolCall = { + ...wireCall("call-2"), + arguments: { ...wireCall("x").arguments, command: "git push --force" }, + }; + expect((await workerGate.authorizeCall(altered)).effect).toBe("deny"); + const result = await gateToolCall( + workerGate, + prepareDispatchedToolCall(altered, "run_shell"), + new AbortController().signal, + async (call) => ({ callId: call.id, content: "ran" }), + ); + expect(result.isError).toBe(true); + expect(store.peek(requestId)?.status).toBe("authorized"); + }); +}); diff --git a/src/session/approval-resume.test.ts b/src/session/approval-resume.test.ts index 1c5bce3da..98abb98ae 100644 --- a/src/session/approval-resume.test.ts +++ b/src/session/approval-resume.test.ts @@ -23,6 +23,7 @@ import { resolveSuspendedApprovalFromStore, } from "./approval-resume.js"; import { createSessionOperationQueue } from "../tui/delivery-queue.js"; +import { createParkedOverlayAbortBinding } from "../tui/runner/parked-overlay-abort.js"; import { defined } from "../../testkit/defined.js"; import { redactedCorrelationId } from "./redacted-correlation.js"; import { @@ -1222,3 +1223,30 @@ describe("approval resume retry re-await", () => { expect(delivered).toHaveLength(1); }); }); + +describe("parked overlay registration after the operator answers", () => { + // approve_worker_call raises a nested permission prompt as soon as its + // redispatched call starts, while the decision that resumed it is still being + // delivered. That prompt's abort signal merges the parked overlay controller. + test("a permission prompt raised while the decision is delivered is not pre-aborted", async () => { + const binding = createParkedOverlayAbortBinding(); + const identity = new AbortController(); + const { agent, gate } = createApprovalResumeHarness(); + let promptSignal: AbortSignal | undefined; + const resume = createApprovalResume({ + getAgent: () => agent, + gate, + resolveParkedCallId: () => "call-A", + registerOverlayAbort: binding.registerOverlayAbort, + deliver: async (message) => { + promptSignal = binding.identitySignal(identity.signal); + agent.deliver(message); + }, + }); + expect( + await resume.handle(suspension("corr-A", "git status --short")), + ).toBe(true); + expect(promptSignal).toBeDefined(); + expect(promptSignal?.aborted).toBe(false); + }); +}); diff --git a/src/session/approval-resume.ts b/src/session/approval-resume.ts index df51054d2..5be09404c 100644 --- a/src/session/approval-resume.ts +++ b/src/session/approval-resume.ts @@ -545,6 +545,11 @@ export function createApprovalResume(args: { const outcome = await args.gate.resolveSuspended(request, stillCurrent); if (!overlayAbort.signal.aborted) overlayAbort.abort(); + // The overlay is gone, but its aborted controller still feeds every + // permission prompt's signal while registered. A resumed call can prompt + // (approve_worker_call checks the worker's frozen call) before delivery + // below resolves, and would be denied unseen. + args.registerOverlayAbort?.(undefined); const timedOutDuringOverlay = overlayAbort.signal.reason === APPROVAL_TIMEOUT_RESULT_TEXT || (await timeoutWatch);