diff --git a/src/agent/tools.test.ts b/src/agent/tools.test.ts index 90bc4a1c8..13062163e 100644 --- a/src/agent/tools.test.ts +++ b/src/agent/tools.test.ts @@ -173,6 +173,7 @@ const { const fakePermissionGate: PermissionGate = { evaluate: mock(async () => ({ allowed: true as const })), + evaluateApprovedWorkerCall: mock(async () => ({ allowed: true as const })), authorizeCall: mock(async () => ({ effect: "allow" as const })), executionVerdict: mock(async () => ({ effect: "allow" as const })), resolveSuspended: mock(async () => undefined), diff --git a/src/agent/tools.ts b/src/agent/tools.ts index 9b1b10097..a4a048ddb 100644 --- a/src/agent/tools.ts +++ b/src/agent/tools.ts @@ -628,6 +628,7 @@ export async function createAgentToolset( sessions: fleetSessions, fleetRecords, permissionGate: args.permissionGate, + approvalPrompted: true, }), ); // Exec-primary opt-in only; TUI primary and nested orchestrators diff --git a/src/permission/approve-worker-call.test.ts b/src/permission/approve-worker-call.test.ts new file mode 100644 index 000000000..f8483065f --- /dev/null +++ b/src/permission/approve-worker-call.test.ts @@ -0,0 +1,307 @@ +import { afterAll, afterEach, describe, expect, test } from "bun:test"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import type { ToolCall } from "@intx/types/runtime"; + +import { createPermissionGate, isRequestCoveredByApproval } from "./gate.js"; +import { createPathRestriction } from "./path-restriction.js"; +import type { Approval, PermissionRequest } from "./types.js"; +import { + createDeniedCallEnvelope, + getProcessWorkerGrantStore, +} from "./worker-grant.js"; +import { createWorktreeRootsProvider } from "./worktree-roots.js"; + +const cwd = mkdtempSync(join(tmpdir(), "approve-worker-call-")); +afterAll(() => rmSync(cwd, { recursive: true, force: true })); +afterEach(() => getProcessWorkerGrantStore().clear()); + +const SAVED_RULE: Approval = { tool: "approve_worker_call", pattern: "*" }; + +function countingGate( + opts: { + approvals?: Approval[]; + allow?: boolean; + // Per-prompt answers in order; once exhausted, `allow` answers. + answers?: boolean[]; + pluginEffect?: "allow" | "ask"; + } = {}, +) { + const prompts: PermissionRequest[] = []; + const answers = [...(opts.answers ?? [])]; + const { pluginEffect } = opts; + const gate = createPermissionGate({ + approvals: opts.approvals ?? [], + cwd, + interactive: true, + skipPermissions: false, + reactorGated: false, + requestApproval: async (request) => { + prompts.push(request); + return { allow: answers.shift() ?? opts.allow ?? true }; + }, + ...(pluginEffect !== undefined + ? { + pluginAuthorize: async () => ({ + effect: pluginEffect, + matchingGrants: [], + resolvedBy: null, + }), + } + : {}), + }); + return { gate, prompts }; +} + +const shellCall = (command: string): ToolCall => ({ + id: "frozen-shell", + name: "run_shell", + arguments: { command }, +}); + +function deniedShellCall(command: string, workerLabel?: string) { + return getProcessWorkerGrantStore().register( + createDeniedCallEnvelope({ + callId: "worker-call-1", + tool: "run_shell", + action: "Run shell command", + subject: command, + args: { command }, + cwd, + workerSessionId: "worker-1", + ...(workerLabel !== undefined ? { workerLabel } : {}), + }), + ); +} + +const approveCall = (requestId: string): ToolCall => ({ + id: "approve-1", + name: "approve_worker_call", + arguments: { + target: "worker-1", + question_id: "question-1", + request_id: requestId, + }, +}); + +describe("the approve_worker_call prompt", () => { + test("shows the worker's frozen call and offers no standing grant", async () => { + const envelope = deniedShellCall( + "printf changed > generated.txt", + "lint fixer", + ); + const { gate, prompts } = countingGate(); + + expect(await gate.evaluate(approveCall(envelope.requestId))).toEqual({ + allowed: true, + }); + + expect(prompts).toHaveLength(1); + const [prompt] = prompts; + expect(prompt?.tool).toBe("approve_worker_call"); + expect(prompt?.subject).toBe("printf changed > generated.txt"); + expect(prompt?.action).toContain("Run shell command"); + expect(prompt?.agentLabel).toBe("lint fixer"); + expect(prompt?.scopes).toEqual([]); + }); + + test("names the worker by id when its label was not recorded", async () => { + const envelope = deniedShellCall("printf changed > generated.txt"); + const { gate, prompts } = countingGate(); + + await gate.evaluate(approveCall(envelope.requestId)); + + expect(prompts[0]?.agentLabel).toBe("worker-1"); + }); + + test("still prompts when no live denied call matches, without a standing grant", async () => { + const { gate, prompts } = countingGate(); + + await gate.evaluate(approveCall("no-such-request")); + + expect(prompts).toHaveLength(1); + expect(prompts[0]?.tool).toBe("approve_worker_call"); + expect(prompts[0]?.scopes).toEqual([]); + }); + + test("prompts even when a saved rule would cover it", async () => { + const envelope = deniedShellCall("printf changed > generated.txt"); + const { gate, prompts } = countingGate({ approvals: [SAVED_RULE] }); + + await gate.evaluate(approveCall(envelope.requestId)); + + expect(prompts).toHaveLength(1); + }); + + test("is never drained from the queue by a saved rule", () => { + const envelope = deniedShellCall("printf changed > generated.txt"); + const request: PermissionRequest = { + tool: "approve_worker_call", + action: "Approve worker call", + subject: envelope.permissionSubject, + scopes: [], + cwd, + }; + const isRestricted = createPathRestriction( + cwd, + createWorktreeRootsProvider(cwd), + ).isRestricted; + + expect( + isRequestCoveredByApproval(request, SAVED_RULE, undefined, isRestricted, { + resolvedCwd: cwd, + roots: [], + }), + ).toBe(false); + }); +}); + +describe("evaluateApprovedWorkerCall", () => { + const writeCall: ToolCall = { + id: "frozen-1", + name: "write_file", + arguments: { path: join(cwd, "notes.txt"), content: "approved" }, + }; + + test("an ordinary ask is answered by the approve_worker_call prompt", async () => { + const control = countingGate(); + await control.gate.evaluate(writeCall); + expect(control.prompts).toHaveLength(1); + + const { gate, prompts } = countingGate(); + expect(await gate.evaluateApprovedWorkerCall(writeCall)).toEqual({ + allowed: true, + }); + expect(prompts).toHaveLength(0); + }); + + test.each([ + ["a sensitive path", "cat .env"], + ["a restricted target", "cat /etc/passwd"], + ])("a call touching %s still prompts on its own", async (_name, command) => { + const approved = countingGate(); + expect( + await approved.gate.evaluateApprovedWorkerCall({ + id: "frozen-1", + name: "run_shell", + arguments: { command }, + }), + ).toEqual({ allowed: true }); + expect(approved.prompts).toHaveLength(1); + + const declined = countingGate({ allow: false }); + const verdict = await declined.gate.evaluateApprovedWorkerCall({ + id: "frozen-2", + name: "run_shell", + arguments: { command }, + }); + expect(verdict.allowed).toBe(false); + }); + + test("a restricted non-shell target still prompts on its own", async () => { + const { gate, prompts } = countingGate(); + + // The session state root is write-ask even inside the workspace. + const verdict = await gate.evaluateApprovedWorkerCall({ + id: "frozen-1", + name: "write_file", + arguments: { path: join(cwd, ".agent-state", "x.json"), content: "{}" }, + }); + + expect(verdict).toEqual({ allowed: true }); + expect(prompts).toHaveLength(1); + }); + + test.each([ + ["the gate allows", "ls"], + ["the gate already asks for", "printf x > out.txt"], + ])( + "a plugin-policy ask on a call %s still prompts", + async (_name, command) => { + const { gate, prompts } = countingGate({ pluginEffect: "ask" }); + + await gate.evaluateApprovedWorkerCall(shellCall(command)); + + expect(prompts).toHaveLength(1); + }, + ); + + test("a plugin allow adds no prompt to an ordinary ask", async () => { + const { gate, prompts } = countingGate({ pluginEffect: "allow" }); + + await gate.evaluateApprovedWorkerCall(shellCall("printf x > out.txt")); + + expect(prompts).toHaveLength(0); + }); + + test("supersedes a decline the operator gave the same call earlier in the turn", async () => { + const { gate, prompts } = countingGate({ answers: [false] }); + expect((await gate.evaluate(writeCall)).allowed).toBe(false); + expect(prompts).toHaveLength(1); + + expect(await gate.evaluateApprovedWorkerCall(writeCall)).toEqual({ + allowed: true, + }); + expect(prompts).toHaveLength(1); + }); + + test("leaves the decline cached for an unapproved same-turn retry", async () => { + const { gate, prompts } = countingGate({ answers: [false] }); + const declined = await gate.evaluate(writeCall); + + await gate.evaluateApprovedWorkerCall(writeCall); + + expect(await gate.evaluate({ ...writeCall, id: "retry-1" })).toEqual( + declined, + ); + expect(prompts).toHaveLength(1); + }); + + test("leaves a different call's cached decline in force", async () => { + const { gate, prompts } = countingGate({ answers: [false] }); + const otherCall: ToolCall = { + id: "frozen-2", + name: "write_file", + arguments: { path: join(cwd, "other.txt"), content: "approved" }, + }; + const declined = await gate.evaluate(otherCall); + expect(declined.allowed).toBe(false); + + expect(await gate.evaluateApprovedWorkerCall(writeCall)).toEqual({ + allowed: true, + }); + + expect(await gate.evaluate({ ...otherCall, id: "retry-1" })).toEqual( + declined, + ); + expect(prompts).toHaveLength(1); + }); + + test("a declined forced ask prompts again when approved, and a second decline denies", async () => { + const { gate, prompts } = countingGate({ answers: [false, false] }); + const sensitiveCall = shellCall("cat .env"); + expect((await gate.evaluate(sensitiveCall)).allowed).toBe(false); + expect(prompts).toHaveLength(1); + + const verdict = await gate.evaluateApprovedWorkerCall(sensitiveCall); + + expect(verdict.allowed).toBe(false); + expect(prompts).toHaveLength(2); + expect((await gate.evaluate(sensitiveCall)).allowed).toBe(false); + expect(prompts).toHaveLength(2); + }); + + test("a hard deny still blocks without a prompt", async () => { + const { gate, prompts } = countingGate(); + + const verdict = await gate.evaluateApprovedWorkerCall({ + id: "frozen-1", + name: "run_shell", + arguments: { command: "rm -rf /" }, + }); + + expect(verdict.allowed).toBe(false); + expect(prompts).toHaveLength(0); + }); +}); diff --git a/src/permission/approve-worker-request.ts b/src/permission/approve-worker-request.ts new file mode 100644 index 000000000..7d6738346 --- /dev/null +++ b/src/permission/approve-worker-request.ts @@ -0,0 +1,42 @@ +import type { ToolCall } from "@intx/types/runtime"; +import { type } from "arktype"; + +import type { PermissionRequest } from "./types.js"; +import { getProcessWorkerGrantStore } from "./worker-grant.js"; + +const ApproveWorkerCallRequestId = type({ request_id: "string" }); + +// The operator approves the worker's frozen call through this prompt, and the +// frozen call does not prompt again for an ordinary ask, so the prompt shows +// that call rather than approve_worker_call's correlation ids. It offers no +// scopes: a standing grant would let the parent model approve worker calls +// with no operator involved. +export function approveWorkerCallRequest(call: ToolCall): PermissionRequest { + const parsed = ApproveWorkerCallRequestId(call.arguments); + const requestId = + parsed instanceof type.errors ? "" : parsed.request_id.trim(); + const envelope = + requestId.length > 0 + ? getProcessWorkerGrantStore().peek(requestId) + : undefined; + if (envelope === undefined) { + return { + tool: call.name, + action: "Approve worker call", + subject: requestId, + arguments: call.arguments, + scopes: [], + notice: + "No live denied worker call matches this request, so approving runs nothing.", + }; + } + return { + tool: call.name, + action: `Approve worker call: ${envelope.toolAction ?? envelope.canonicalTool}`, + subject: envelope.permissionSubject, + arguments: call.arguments, + scopes: [], + agentLabel: envelope.workerLabel ?? envelope.workerSessionId, + notice: "Approving lets the worker retry this exact call once.", + }; +} diff --git a/src/permission/classify.ts b/src/permission/classify.ts index 89a18eac4..6ee40372e 100644 --- a/src/permission/classify.ts +++ b/src/permission/classify.ts @@ -35,6 +35,7 @@ import { READ_ONLY_CONTROL_PLANE_TOOLS, } from "../agent/tool-classification.js"; import { canonicalToolName } from "../agent/canonical-tool-name.js"; +import { approveWorkerCallRequest } from "./approve-worker-request.js"; // Read-only tools never need approval while they stay off restricted paths — // they cannot change the workspace. `lsp` is included despite activating @@ -570,6 +571,9 @@ function shellApprovalScopes(command: string): ApprovalScope[] { // Shell is one request for the full command (security still splits under the // gate); file tools key on the target path. export function buildRequests(call: ToolCall): PermissionRequest[] { + if (call.name === "approve_worker_call") { + return [approveWorkerCallRequest(call)]; + } if (call.name === "run_shell") { const command = stringArg(call, "command"); // Pure comments / empty: nothing to approve. diff --git a/src/permission/gate.ts b/src/permission/gate.ts index 8c4c4c068..f84a7a03a 100644 --- a/src/permission/gate.ts +++ b/src/permission/gate.ts @@ -292,6 +292,7 @@ function isRequestCoveredByApprovals( rootsProvider?: RootsProvider, isExtraDenied: (value: string) => boolean = () => false, ): boolean { + if (UNGRANTABLE_TOOLS.has(request.tool)) return false; const scoped = approvals.filter((a) => grantScopeMatches( a, @@ -353,7 +354,14 @@ const AUTO_ALLOWED_TOOLS = new Set([ "resume_agent", ]); +// No saved approval covers these; they prompt on every use. +// approve_worker_call's prompt also answers the worker's frozen call, so a +// standing grant would let the parent model approve worker calls with no +// operator involved. +const UNGRANTABLE_TOOLS = new Set(["approve_worker_call"]); + const logger = getLogger([LOG_NAMESPACE_ROOT, "permission", "plugin-authz"]); +const gateLogger = getLogger([LOG_NAMESPACE_ROOT, "permission", "gate"]); export interface PermissionGateOptions { // Seed approvals; the gate copies them and never mutates the caller's array. @@ -422,6 +430,10 @@ export type AuthorizeVerdict = export interface PermissionGate { evaluate: (call: ToolCall) => Promise; + // A worker's frozen call once the operator approved approve_worker_call's + // prompt, which showed it: an ordinary ask is answered, a forced ask still + // prompts, and a deny still blocks. + evaluateApprovedWorkerCall: (call: ToolCall) => Promise; // Reactor-path policy: evaluate()'s decision as the vendored before-tool // authz hook's effect. authorizeCall: (call: ToolCall) => Promise; @@ -676,7 +688,9 @@ export function createPermissionGate( // Non-blocking policy decision for one tool call — every gate rule resolved // without waiting on an operator. `ask` carries the fully-built request so - // the middleware and reactor seams need no re-derivation. + // the middleware and reactor seams need no re-derivation. `forced` marks an + // ask no standing grant could answer (a guarded path or payload, or a plugin + // policy ask), so the operator must see this exact call. type GateDecision = | { kind: "allow" } | { kind: "deny"; reason: string; publicReason?: string | undefined } @@ -685,9 +699,20 @@ export function createPermissionGate( request: PermissionRequest; anySecret: boolean; segmentCount: number; + forced: boolean; }; - const decideNormally = async (rawCall: ToolCall): Promise => { + // `approved`: the operator approved this exact call after any decline cached + // for it, so that decline no longer speaks for them. Only the lookup is + // skipped; the entry stays to deny an unapproved same-turn retry. + interface DecideOptions { + approved: boolean; + } + + const decideNormally = async ( + rawCall: ToolCall, + { approved }: DecideOptions, + ): Promise => { let call: ToolCall; try { call = coercePolicyCall(rawCall); @@ -732,7 +757,7 @@ export function createPermissionGate( // Fingerprint the raw call, not the coerced `call` above — coercion is // idempotent, so all three decline paths share one key. const stableId = denialFingerprint(rawCall, effectiveCwd); - const cachedDenial = denialMemory.isDenied(stableId); + const cachedDenial = approved ? undefined : denialMemory.isDenied(stableId); if (cachedDenial !== undefined) return { kind: "deny", reason: cachedDenial }; if (skipPermissions) return { kind: "allow" }; @@ -806,7 +831,8 @@ export function createPermissionGate( // close_agent — permanently close a worker. // resume_agent — send a completed or interrupted worker its next turn. // - // Ask in auto and non-auto. Not in AUTO_ALLOWED_TOOLS; grants authority: + // Ask in auto and non-auto, on every use. Not in AUTO_ALLOWED_TOOLS, and + // in UNGRANTABLE_TOOLS; grants authority: // approve_worker_call — authorize one pending worker-denied call. // // Worker sessions do not use this prompt. submit_result, ask_director, @@ -879,6 +905,7 @@ export function createPermissionGate( let needsOperator = false; let anySecret = false; + let guarded = false; let mismatchNotice: string | undefined; for (const segment of segments) { // A secret-path reference or restricted target always requires the @@ -893,6 +920,7 @@ export function createPermissionGate( ); if (guard !== undefined) { if (guard.kind !== "restricted") anySecret = true; + guarded = true; needsOperator = true; // A standing grant may still cover this segment while the // pre-grant guard forces an ask — record why for the prompt. @@ -973,6 +1001,7 @@ export function createPermissionGate( request: requestForOperator, anySecret, segmentCount: segments.length, + forced: guarded, }; } @@ -987,7 +1016,8 @@ export function createPermissionGate( workspace: grantWorkspace(), }); if (alreadyApproved) { - continue; + if (!UNGRANTABLE_TOOLS.has(request.tool)) continue; + gateLogger.warn`ignoring a saved approval for ${request.tool}; it prompts on every use`; } if (!interactive || requestApproval === undefined) { @@ -998,7 +1028,13 @@ export function createPermissionGate( return { kind: "deny", reason }; } - return { kind: "ask", request, anySecret: false, segmentCount: 0 }; + return { + kind: "ask", + request, + anySecret: false, + segmentCount: 0, + forced: restricted, + }; } return { kind: "allow" }; }; @@ -1037,11 +1073,15 @@ export function createPermissionGate( (segment) => !isShellCommentOnly(segment), ).length : 0, + forced: true, }; }; - const decideWithPlugin = async (rawCall: ToolCall): Promise => { - const normal = await decideNormally(rawCall); + const decideWithPlugin = async ( + rawCall: ToolCall, + decideOptions: DecideOptions, + ): Promise => { + const normal = await decideNormally(rawCall, decideOptions); if (normal.kind === "deny") return normal; try { const policyCall = coercePolicyCall(rawCall); @@ -1073,7 +1113,8 @@ export function createPermissionGate( ...(publicReason !== undefined ? { publicReason } : {}), }; } - if (normal.kind === "ask" || result.effect === "allow") return normal; + if (result.effect === "allow") return normal; + if (normal.kind === "ask") return { ...normal, forced: true }; return pluginAskDecision(policyCall); } catch (err) { logger.warn`plugin authorization failed: ${scrubSecrets(safePluginDiagnostic(err))}`; @@ -1088,7 +1129,7 @@ export function createPermissionGate( // any grant the outcome carries (never for secret-path shell); undefined // when no outcome arrived. const resolveInteractiveAsk = async ( - decision: Extract, + decision: Omit, "forced">, stillCurrent?: () => boolean, ) => { const { request, anySecret, segmentCount } = decision; @@ -1156,8 +1197,25 @@ export function createPermissionGate( // Middleware path: blocking evaluation for tool-runner consumers whose calls // never pass through the reactor. Reactor-gated gates use executionVerdict // instead, so ask never re-prompts. - const evaluate = async (rawCall: ToolCall): Promise => { - const decision = await decide(rawCall); + const evaluate = async (rawCall: ToolCall): Promise => + verdictForDecision(rawCall, await decide(rawCall, { approved: false })); + + // The operator already approved this call through approve_worker_call's + // prompt, which showed it, so an ordinary ask is answered and an earlier + // decline of the same call is superseded. A forced ask still prompts on its + // own; that is the conservative trade for sensitive and restricted targets. + const evaluateApprovedWorkerCall = async ( + rawCall: ToolCall, + ): Promise => { + const decision = await decide(rawCall, { approved: true }); + if (decision.kind === "ask" && !decision.forced) return { allowed: true }; + return verdictForDecision(rawCall, decision); + }; + + const verdictForDecision = async ( + rawCall: ToolCall, + decision: GateDecision, + ): Promise => { if (decision.kind === "allow") return { allowed: true }; if (decision.kind === "deny") return { allowed: false, reason: decision.reason }; @@ -1204,7 +1262,9 @@ export function createPermissionGate( }; const authorizeCall = async (call: ToolCall): Promise => { - const verdict = mapAuthorizeVerdict(await decide(call)); + const verdict = mapAuthorizeVerdict( + await decide(call, { approved: false }), + ); const identityCwd = getSubAgentIdentity()?.cwd ?? resolvedCwd; const identityCall = callForIdentity(call); authorizedByCallId.set(call.id, { @@ -1240,7 +1300,7 @@ export function createPermissionGate( authorizedByCallId.delete(call.id); return cached.verdict; } - return mapAuthorizeVerdict(await decide(call)); + return mapAuthorizeVerdict(await decide(call, { approved: false })); }; // Resolve a suspended reactor approval once the operator answers. The @@ -1351,6 +1411,7 @@ export function createPermissionGate( return { evaluate, + evaluateApprovedWorkerCall, authorizeCall, executionVerdict, resolveSuspended, diff --git a/src/permission/reactor-authorize.ts b/src/permission/reactor-authorize.ts index f089a3bd9..3e23c52cd 100644 --- a/src/permission/reactor-authorize.ts +++ b/src/permission/reactor-authorize.ts @@ -157,6 +157,7 @@ function denyWorkerCallWithEnvelope( if (sessionId === undefined) return { effect: "deny", reason: baseReason }; const store = options?.store ?? getProcessWorkerGrantStore(); const identity = workerCallIdentity(sessionId, call, cwd); + const workerLabel = getSubAgentIdentity()?.description; const existing = store.pendingMatch( identity.sessionId, identity.canonicalTool, @@ -174,6 +175,7 @@ function denyWorkerCallWithEnvelope( args: identity.args, cwd: identity.cwd, workerSessionId: identity.sessionId, + ...(workerLabel !== undefined ? { workerLabel } : {}), ...(options?.workspaceRoot !== undefined ? { workspaceRoot: options.workspaceRoot } : {}), @@ -315,6 +317,9 @@ export function workerPermissionGate( ): PermissionGate { return { evaluate: (call) => evaluateWorkerCall(gate, call, grantOptions), + // A worker has no operator prompt of its own to stand in for one. + evaluateApprovedWorkerCall: (call) => + evaluateWorkerCall(gate, call, grantOptions), authorizeCall: (call) => authorizeWorkerCall(gate, call, grantOptions), executionVerdict: (call) => executionVerdictWorkerCall(gate, call, grantOptions), diff --git a/src/permission/worker-grant.ts b/src/permission/worker-grant.ts index aeab7451d..c2a8fce1b 100644 --- a/src/permission/worker-grant.ts +++ b/src/permission/worker-grant.ts @@ -39,6 +39,8 @@ export interface WorkerGrantAuditEvent { export interface WorkerDeniedCallEnvelope { requestId: string; workerSessionId: string; + /** The worker's dispatch label, so the operator prompt can name it. */ + workerLabel?: string; turnId?: string; deniedCallId: string; canonicalTool: string; @@ -66,6 +68,7 @@ export interface DeniedCallDescriptor { args: Record; cwd: string; workerSessionId: string; + workerLabel?: string; turnId?: string; workspaceRoot?: string; now?: number; @@ -125,6 +128,9 @@ export function createDeniedCallEnvelope( const envelope: WorkerDeniedCallEnvelope = { requestId: randomUUID(), workerSessionId: descriptor.workerSessionId, + ...(descriptor.workerLabel !== undefined + ? { workerLabel: descriptor.workerLabel } + : {}), ...(descriptor.turnId !== undefined ? { turnId: descriptor.turnId } : {}), deniedCallId: descriptor.callId, canonicalTool, diff --git a/src/subagent/lifecycle-tools.test.ts b/src/subagent/lifecycle-tools.test.ts index 1d5cb97d3..e6ab65b93 100644 --- a/src/subagent/lifecycle-tools.test.ts +++ b/src/subagent/lifecycle-tools.test.ts @@ -1,4 +1,7 @@ import { describe, expect, test } from "bun:test"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import { createCloseAgentTool, @@ -1495,6 +1498,55 @@ describe("approve_worker_call", () => { }, ); + test("an ordinary denied call reaches the operator once, through the approve_worker_call prompt", async () => { + const grants = getProcessWorkerGrantStore(); + grants.clear(); + // The gate's path checks resolve against cwd, so it must exist. + const cwd = mkdtempSync(join(tmpdir(), "worker-approval-one-prompt-")); + const sessions = createSubAgentSessionStore(); + const { session, envelope } = pendingDeniedAsk(sessions, { + id: "worker-one-prompt", + cwd, + command: "printf changed > generated.txt", + }); + const prompts: string[] = []; + const gate = createPermissionGate({ + approvals: [], + cwd, + interactive: true, + skipPermissions: false, + reactorGated: false, + requestApproval: async (request) => { + prompts.push(request.tool); + return { allow: true }; + }, + }); + const call = { + id: "approve-once", + name: "approve_worker_call", + arguments: { + target: session.id, + question_id: `${session.id}-question`, + request_id: envelope.requestId, + }, + }; + + expect(await gate.evaluate(call)).toEqual({ allowed: true }); + const approved = await callTool( + createApproveWorkerCallTool({ + sessions, + permissionGate: gate, + approvalPrompted: true, + }), + call.arguments, + ); + + expect(approved.status).toBe("authorized"); + expect(prompts).toEqual(["approve_worker_call"]); + grants.clear(); + rmSync(cwd, { recursive: true, force: true }); + }); + test("an in-flight allowed retry cannot leave an authorized replay", async () => { const grants = getProcessWorkerGrantStore(); grants.clear(); diff --git a/src/subagent/lifecycle-tools.ts b/src/subagent/lifecycle-tools.ts index 2b189c861..03a2929bb 100644 --- a/src/subagent/lifecycle-tools.ts +++ b/src/subagent/lifecycle-tools.ts @@ -141,6 +141,11 @@ export interface LifecycleToolDeps { export type ApproveWorkerCallToolDeps = LifecycleToolDeps & { permissionGate: PermissionGate; + // True only where approve_worker_call itself always prompts the operator + // (the top-level parent), so that prompt answers the frozen call's ordinary + // ask. A nested orchestrator's approve_worker_call runs without a prompt, + // so its frozen call must prompt instead. + approvalPrompted?: boolean; }; /** close_agent always terminalizes the wait mailbox — no silent skip. */ @@ -739,7 +744,10 @@ export function createApproveWorkerCallTool( try { verdict = await runWithSubAgentIdentity( { description: session.description, cwd: validatedEnvelope.cwd }, - () => deps.permissionGate.evaluate(frozenCall), + () => + deps.approvalPrompted === true + ? deps.permissionGate.evaluateApprovedWorkerCall(frozenCall) + : deps.permissionGate.evaluate(frozenCall), ); } catch { return lifecycleResult(