From 2067e67fbe65300d37b610fb37baa22cff9cd880 Mon Sep 17 00:00:00 2001 From: Ben Wachman Date: Thu, 15 Oct 2026 13:58:18 -0400 Subject: [PATCH 1/2] feat(permissions): add typed parent approval for denied worker calls --- src/agent/tools.ts | 6 + src/permission/reactor-authorize.test.ts | 27 ++ src/permission/reactor-authorize.ts | 49 ++-- src/permission/worker-grant-flow.test.ts | 7 +- src/permission/worker-grant.test.ts | 42 ++- src/permission/worker-grant.ts | 156 ++++++++-- src/subagent/agent-fleet.test.ts | 177 ++++++++++++ src/subagent/agent-fleet.ts | 11 + src/subagent/authority.test.ts | 3 + src/subagent/authority.ts | 1 + src/subagent/fleet-test-harness.ts | 5 + src/subagent/lifecycle-tools.test.ts | 346 +++++++++++++++++++++++ src/subagent/lifecycle-tools.ts | 188 ++++++++++++ src/subagent/run-authority.test.ts | 29 ++ src/subagent/run.ts | 8 + src/subagent/session-store.ts | 4 +- 16 files changed, 983 insertions(+), 76 deletions(-) diff --git a/src/agent/tools.ts b/src/agent/tools.ts index c9db63114..70ae8534d 100644 --- a/src/agent/tools.ts +++ b/src/agent/tools.ts @@ -86,6 +86,7 @@ import { createResumeAgentTool, createInterruptAgentTool, createSendInputTool, + createApproveWorkerCallTool, } from "../subagent/lifecycle-tools.js"; import { createManageTasksRunner } from "./tasks.js"; import { createSpillingBackgroundShellExitNotifier } from "./background-shell-tool.js"; @@ -609,6 +610,11 @@ export async function createAgentToolset( createResumeAgentTool({ sessions: fleetSessions, fleetRecords }), createInterruptAgentTool({ sessions: fleetSessions, fleetRecords }), createSendInputTool({ sessions: fleetSessions, fleetRecords }), + createApproveWorkerCallTool({ + sessions: fleetSessions, + fleetRecords, + permissionGate: args.permissionGate, + }), ); // Exec-primary opt-in only; TUI primary and nested orchestrators // collect via mailbox mail, so wait_agents stays unmounted there. diff --git a/src/permission/reactor-authorize.test.ts b/src/permission/reactor-authorize.test.ts index d1b7a1343..d0be9479e 100644 --- a/src/permission/reactor-authorize.test.ts +++ b/src/permission/reactor-authorize.test.ts @@ -234,6 +234,33 @@ test("nested orchestrator wait_agents allows with only a spawn_agent grant", asy ).toBe("allow"); }); +test("nested parent approval tool passes the worker authorization seam", async () => { + const policy = gate({ interactive: true, auto: false }); + const authorize = createWorkerAuthorize(policy); + expect( + ( + await authorize( + "tool:approve_worker_call", + "invoke", + namedCall("approve_worker_call", { + target: "child-session", + question_id: "child-question", + request_id: "child-request", + }), + ) + ).effect, + ).toBe("allow"); + expect( + ( + await authorize( + "tool:write_file", + "invoke", + namedCall("write_file", { path: "ordinary.txt", content: "data" }), + ) + ).effect, + ).toBe("deny"); +}); + test("worker spawn_agent still needs a parent grant", async () => { const policy = gate({ interactive: true, auto: false }); expect( diff --git a/src/permission/reactor-authorize.ts b/src/permission/reactor-authorize.ts index f4831e04b..f1e5666e6 100644 --- a/src/permission/reactor-authorize.ts +++ b/src/permission/reactor-authorize.ts @@ -23,7 +23,6 @@ import type { AuthorizeVerdict, GateVerdict, PermissionGate } from "./gate.js"; import type { PermissionRequest } from "./types.js"; import { createDeniedCallEnvelope, - fingerprintDeniedCall, formatWorkerDenyWithGrantId, getProcessWorkerGrantStore, type WorkerGrantStore, @@ -164,17 +163,6 @@ function denyWorkerCallWithEnvelope( }; } -/** Mutex key covering the envelope match: concurrent identical worker calls - * serialize through precheck-to-consume so one envelope allows exactly once. */ -function workerGrantKey( - sessionId: string, - call: ToolCallType, - cwd: string, -): string { - const identity = workerCallIdentity(sessionId, call, cwd); - return `${sessionId}\n${fingerprintDeniedCall(identity.canonicalTool, identity.args, identity.cwd)}`; -} - async function authorizeWorkerCall( gate: PermissionGate, call: ToolCallType, @@ -187,11 +175,13 @@ async function authorizeWorkerCall( if (sessionId === undefined) return authorizeWorkerCallInner(gate, call, grantOptions, workerCwd); const store = grantOptions?.store ?? getProcessWorkerGrantStore(); - return store.runExclusive(workerGrantKey(sessionId, call, workerCwd), () => - authorizeWorkerCallInner(gate, call, grantOptions, workerCwd, { - sessionId, - store, - }), + return store.runExclusiveIdentity( + workerCallIdentity(sessionId, call, workerCwd), + () => + authorizeWorkerCallInner(gate, call, grantOptions, workerCwd, { + sessionId, + store, + }), ); } @@ -203,10 +193,10 @@ async function authorizeWorkerCallInner( grant?: { sessionId: string; store: WorkerGrantStore }, ): Promise<{ effect: "allow" } | { effect: "deny"; reason: string }> { if (grant !== undefined) { - const precheck = grant.store.precheck( - workerCallIdentity(grant.sessionId, call, workerCwd), - ); + const identity = workerCallIdentity(grant.sessionId, call, workerCwd); + const precheck = grant.store.precheck(identity); if (!precheck.ok) return { effect: "deny", reason: precheck.blocker }; + if (grant.store.hasAuthorizedRetry(identity)) return { effect: "allow" }; } const verdict = await gate.authorizeCall(call); // Authorize never consumes: it only permits the call to proceed to the @@ -248,11 +238,13 @@ async function executionVerdictWorkerCall( if (sessionId === undefined) return executionVerdictWorkerCallInner(gate, call, grantOptions, workerCwd); const store = grantOptions?.store ?? getProcessWorkerGrantStore(); - return store.runExclusive(workerGrantKey(sessionId, call, workerCwd), () => - executionVerdictWorkerCallInner(gate, call, grantOptions, workerCwd, { - sessionId, - store, - }), + return store.runExclusiveIdentity( + workerCallIdentity(sessionId, call, workerCwd), + () => + executionVerdictWorkerCallInner(gate, call, grantOptions, workerCwd, { + sessionId, + store, + }), ); } @@ -264,10 +256,11 @@ async function executionVerdictWorkerCallInner( grant?: { sessionId: string; store: WorkerGrantStore }, ): Promise { if (grant !== undefined) { - const precheck = grant.store.precheck( - workerCallIdentity(grant.sessionId, call, workerCwd), - ); + const identity = workerCallIdentity(grant.sessionId, call, workerCwd); + const precheck = grant.store.precheck(identity); if (!precheck.ok) return { effect: "deny", reason: precheck.blocker }; + if (grant.store.consumeAuthorizedRetry(identity) !== undefined) + return { effect: "allow" }; } const verdict = await gate.executionVerdict(call); // Sole consumption point: the execution backstop spends the envelope when diff --git a/src/permission/worker-grant-flow.test.ts b/src/permission/worker-grant-flow.test.ts index 255a5d9df..707c90cc9 100644 --- a/src/permission/worker-grant-flow.test.ts +++ b/src/permission/worker-grant-flow.test.ts @@ -186,7 +186,7 @@ describe("worker grant-request flow: deny → parent replay grant → one retry" expect(store.peek(requestId)?.status).toBe("pending"); }); - test("send_input text answers the ask but grants nothing", () => { + test("send_input text answers an unbound ask but grants nothing", () => { const sessions = createSubAgentSessionStore(); const session = sessions.start({ id: "fleet-1", @@ -221,9 +221,8 @@ describe("worker grant-request flow: deny → parent replay grant → one retry" }, }), ).toBe(true); - expect(sessions.peekAsk(session.id)?.deniedCall?.requestId).toBe( - envelope.requestId, - ); + // A request ID quoted in prose is not an authority-bearing binding. + expect(sessions.peekAsk(session.id)?.deniedCall).toBeUndefined(); const echo = JSON.stringify({ requestId: envelope.requestId, diff --git a/src/permission/worker-grant.test.ts b/src/permission/worker-grant.test.ts index 78c5c3f7c..57a13716b 100644 --- a/src/permission/worker-grant.test.ts +++ b/src/permission/worker-grant.test.ts @@ -45,12 +45,15 @@ describe("fingerprintDeniedCall", () => { expect(a).toBe(b); }); - test("differs across args and tool (cwd binds at precheck, not in the hash)", () => { + test("differs across args, tool, and cwd", () => { const base = fingerprintDeniedCall("run_shell", ARGS, CWD); expect( fingerprintDeniedCall("run_shell", { command: "npm run evil" }, CWD), ).not.toBe(base); expect(fingerprintDeniedCall("read_file", ARGS, CWD)).not.toBe(base); + expect(fingerprintDeniedCall("run_shell", ARGS, "/elsewhere")).not.toBe( + base, + ); expect( fingerprintDeniedCall("read_file", { path: "a.md" }, "/repo") === fingerprintDeniedCall("read_file", { path: "b.md" }, "/repo"), @@ -283,19 +286,32 @@ describe("WorkerGrantStore lifecycle", () => { expect(store.invalidateSession("worker-1", "again")).toBe(0); }); - test("attachToAsk stamps the questionId and keeps pending", () => { + test("attachToAsk binds only an explicit requestId and keeps it pending", () => { const store = new WorkerGrantStore(); const envelope = store.register(createDeniedCallEnvelope(descriptor())); // Reads pin the clock: fixture now predates wall time, so an unpinned // read would sweep the envelope as expired. - expect(store.attachToAsk("worker-1", "ask-7", undefined, 1_000_001)).toBe( - envelope, - ); - expect(envelope.questionId).toBe("ask-7"); + expect( + store.attachToAsk("worker-1", "ask-generic", undefined, 1_000_001), + ).toBeUndefined(); + expect(envelope.questionId).toBeUndefined(); expect(envelope.status).toBe("pending"); expect(store.peek(envelope.requestId)).toBe(envelope); expect( - store.attachToAsk("worker-9", "ask-8", undefined, 1_000_001), + store.attachToAsk("worker-1", "ask-7", envelope.requestId, 1_000_001), + ).toBe(envelope); + expect(envelope.questionId).toBe("ask-7"); + expect( + store.attachToAsk( + "worker-1", + "ask-rebound", + envelope.requestId, + 1_000_001, + ), + ).toBeUndefined(); + expect(envelope.questionId).toBe("ask-7"); + expect( + store.attachToAsk("worker-9", "ask-8", envelope.requestId, 1_000_001), ).toBeUndefined(); expect(envelope.audit.map((event) => event.event)).toEqual([ "denied", @@ -329,12 +345,12 @@ describe("WorkerGrantStore lifecycle", () => { expect(store.peek(envelopeB.requestId)?.questionId).toBe("ask-B"); }); - test("unnamed ask keeps the legacy first-pending bind", () => { + test("unnamed ask does not bind a pending denial", () => { const { store, envelopeA, envelopeB } = twoDenies(); - expect(store.attachToAsk("worker-1", "ask-x", undefined, 1_000_001)).toBe( - envelopeA, - ); - expect(envelopeA.questionId).toBe("ask-x"); + expect( + store.attachToAsk("worker-1", "ask-x", undefined, 1_000_001), + ).toBeUndefined(); + expect(envelopeA.questionId).toBeUndefined(); expect(envelopeB.questionId).toBeUndefined(); }); @@ -370,7 +386,7 @@ describe("WorkerGrantStore lifecycle", () => { test("audit trail orders deny → ask → consume", () => { const store = new WorkerGrantStore(); const envelope = store.register(createDeniedCallEnvelope(descriptor())); - store.attachToAsk("worker-1", "ask-1", undefined, 1_000_001); + store.attachToAsk("worker-1", "ask-1", envelope.requestId, 1_000_001); store.consumeOnAllow({ sessionId: "worker-1", canonicalTool: "run_shell", diff --git a/src/permission/worker-grant.ts b/src/permission/worker-grant.ts index 1c4510b3b..78219f8e6 100644 --- a/src/permission/worker-grant.ts +++ b/src/permission/worker-grant.ts @@ -17,12 +17,19 @@ export const WORKER_GRANT_TTL_MS = 10 * 60 * 1000; export type WorkerGrantStatus = | "pending" + | "authorized" | "consumed" | "expired" | "interrupted"; export interface WorkerGrantAuditEvent { - event: "denied" | "asked" | "consumed" | "expired" | "interrupted"; + event: + | "denied" + | "asked" + | "authorized" + | "consumed" + | "expired" + | "interrupted"; at: number; detail?: string; } @@ -73,7 +80,11 @@ export function fingerprintDeniedCall( { name: canonicalTool, arguments: sortKeys(args) } as ToolCall, cwd, ); - return createHash("sha256").update(stable).digest("hex"); + return createHash("sha256") + .update(stable) + .update("\0") + .update(cwd) + .digest("hex"); } function sortKeys(value: unknown): unknown { @@ -173,6 +184,8 @@ function terminalBlocker( switch (envelope.status) { case "consumed": return `Grant request ${envelope.requestId} was already consumed by its one retry — replaying ${expected} is denied.`; + case "authorized": + return `Grant request ${envelope.requestId} is reserved for its exact pending retry — replaying ${expected} is denied.`; case "expired": return `Grant request ${envelope.requestId} expired — retrying ${expected} is denied. Re-ask instead.`; case "interrupted": @@ -208,6 +221,17 @@ export class WorkerGrantStore { } } + /** Serialize work that mutates an exact worker-call envelope. */ + runExclusiveIdentity( + identity: WorkerCallIdentity, + fn: () => Promise, + ): Promise { + return this.runExclusive( + `${identity.sessionId}\n${fingerprintOf(identity)}`, + fn, + ); + } + register(envelope: WorkerDeniedCallEnvelope): WorkerDeniedCallEnvelope { this.envelopes.set(envelope.requestId, envelope); return envelope; @@ -257,10 +281,10 @@ export class WorkerGrantStore { return undefined; } - /** Stamp the questionId the parent's retry references. A named requestId - * binds its exact denial (first-pending would join a question about B to - * call A); an unnamed ask keeps the legacy first-pending bind. Unresolvable - * ids fail closed; overdue envelopes sweep first. */ + /** Stamp the questionId the parent's retry references. Only the exact + * requestId supplied by the worker can bind a denial: session-wide fallback + * would attach a stale denial to an unrelated ask. Unresolvable or already + * attached ids fail closed; overdue envelopes sweep first. */ attachToAsk( sessionId: string, questionId: string, @@ -268,26 +292,59 @@ export class WorkerGrantStore { now: number = Date.now(), ): WorkerDeniedCallEnvelope | undefined { this.sweepExpired(now); - const named = requestId?.trim() || undefined; - if (named !== undefined) { - const envelope = this.envelopes.get(named); - if ( - envelope === undefined || - envelope.workerSessionId !== sessionId || - envelope.status !== "pending" - ) - return undefined; - envelope.questionId = questionId; - audit(envelope, "asked", questionId); - return envelope; - } - const envelope = this.pendingForSession(sessionId, now); - if (envelope === undefined) return undefined; + const named = requestId?.trim(); + if (named === undefined || named.length === 0) return undefined; + const envelope = this.envelopes.get(named); + if ( + envelope === undefined || + envelope.workerSessionId !== sessionId || + envelope.status !== "pending" || + envelope.questionId !== undefined + ) + return undefined; envelope.questionId = questionId; audit(envelope, "asked", questionId); return envelope; } + /** Parent-only transition after it has approved this exact live envelope. + * The authorized state remains bound to the retained envelope and is spent + * only by consumeAuthorizedRetry at the worker execution seam. */ + authorizeRetry(requestId: string, now: number = Date.now()): boolean { + this.sweepExpired(now); + const envelope = this.envelopes.get(requestId); + if (envelope === undefined || envelope.status !== "pending") return false; + if (envelope.expiresAt <= now) return false; + envelope.status = "authorized"; + audit(envelope, "authorized"); + return true; + } + + /** Undo an authorization when the parent could not wake its still-pending + * ask. No worker can receive the authorization without that wake. */ + revokeAuthorizedRetry(requestId: string): void { + const envelope = this.envelopes.get(requestId); + if (envelope?.status === "authorized") envelope.status = "pending"; + } + + hasAuthorizedRetry(identity: WorkerCallIdentity): boolean { + const now = identity.now ?? Date.now(); + this.sweepExpired(now); + const fingerprint = fingerprintOf(identity); + for (const envelope of this.envelopes.values()) { + if ( + envelope.argsFingerprint === fingerprint && + envelope.cwd === identity.cwd && + envelope.canonicalTool === identity.canonicalTool && + envelope.workerSessionId === identity.sessionId && + envelope.status === "authorized" && + envelope.expiresAt > now + ) + return true; + } + return false; + } + /** * Execution backstop: fail closed when the exact call matches a terminal * envelope (replay, interrupt) or a tampered cwd. Expired envelopes are @@ -301,9 +358,7 @@ export class WorkerGrantStore { ): { ok: true } | { ok: false; blocker: string } { const now = identity.now ?? Date.now(); this.sweepExpired(now); - const fingerprint = fingerprintOf(identity); for (const envelope of this.envelopes.values()) { - if (envelope.argsFingerprint !== fingerprint) continue; // Own-session envelopes only (reactor retries share this session, so // consume-once survives). if ( @@ -312,8 +367,23 @@ export class WorkerGrantStore { ) { continue; } + // Compare against the envelope's CWD so a changed retry CWD reaches + // the explicit CWD rejection below rather than looking unrelated. + if ( + envelope.argsFingerprint !== + fingerprintDeniedCall( + identity.canonicalTool, + identity.args, + envelope.cwd, + ) + ) + continue; if (envelope.cwd !== identity.cwd) { - if (envelope.status === "pending" || envelope.status === "consumed") + if ( + envelope.status === "pending" || + envelope.status === "authorized" || + envelope.status === "consumed" + ) return { ok: false, blocker: @@ -327,7 +397,7 @@ export class WorkerGrantStore { }; } if (envelope.status === "expired") continue; - if (envelope.status !== "pending") { + if (envelope.status !== "pending" && envelope.status !== "authorized") { return { ok: false, blocker: terminalBlocker(envelope, identity.canonicalTool), @@ -369,6 +439,29 @@ export class WorkerGrantStore { return undefined; } + /** Atomically spend the parent-authorized exact retry at execution. */ + consumeAuthorizedRetry( + identity: WorkerCallIdentity, + ): WorkerDeniedCallEnvelope | undefined { + const now = identity.now ?? Date.now(); + const fingerprint = fingerprintOf(identity); + for (const envelope of this.envelopes.values()) { + if ( + envelope.argsFingerprint !== fingerprint || + envelope.cwd !== identity.cwd || + envelope.canonicalTool !== identity.canonicalTool || + envelope.workerSessionId !== identity.sessionId || + envelope.status !== "authorized" || + envelope.expiresAt <= now + ) + continue; + envelope.status = "consumed"; + audit(envelope, "consumed", identity.canonicalTool); + return envelope; + } + return undefined; + } + /** Interrupt invalidation: tombstone the session's envelopes so a later * replay fails closed instead of riding whatever grant the gate now holds. * Retained completion does not invalidate — the resume_agent retry is a @@ -378,7 +471,7 @@ export class WorkerGrantStore { for (const envelope of this.envelopes.values()) { if ( envelope.workerSessionId !== sessionId || - envelope.status !== "pending" + (envelope.status !== "pending" && envelope.status !== "authorized") ) continue; envelope.status = "interrupted"; @@ -393,7 +486,11 @@ export class WorkerGrantStore { sweepExpired(now = Date.now()): number { let expired = 0; for (const envelope of this.envelopes.values()) { - if (envelope.status !== "pending" || envelope.expiresAt > now) continue; + if ( + (envelope.status !== "pending" && envelope.status !== "authorized") || + envelope.expiresAt > now + ) + continue; envelope.status = "expired"; audit(envelope, "expired"); expired += 1; @@ -408,7 +505,7 @@ export class WorkerGrantStore { for (const envelope of this.envelopes.values()) { if ( envelope.workerSessionId !== sessionId || - envelope.status !== "pending" + (envelope.status !== "pending" && envelope.status !== "authorized") ) continue; envelope.status = "expired"; @@ -423,7 +520,8 @@ export class WorkerGrantStore { invalidateAll(reason: string): number { let invalidated = 0; for (const envelope of this.envelopes.values()) { - if (envelope.status !== "pending") continue; + if (envelope.status !== "pending" && envelope.status !== "authorized") + continue; envelope.status = "interrupted"; audit(envelope, "interrupted", reason); invalidated += 1; diff --git a/src/subagent/agent-fleet.test.ts b/src/subagent/agent-fleet.test.ts index 9b956e302..5a86311d3 100644 --- a/src/subagent/agent-fleet.test.ts +++ b/src/subagent/agent-fleet.test.ts @@ -23,6 +23,10 @@ import { agentLaneIsLive, fleetProgress } from "./agent-progress.js"; import { forcedStopReport } from "./stop-policy.js"; import type { RunSubAgentParams, RunSubAgentResult } from "./types.js"; import { INTERVENTION_FILE } from "./intervention-log.js"; +import { + createDeniedCallEnvelope, + getProcessWorkerGrantStore, +} from "../permission/worker-grant.js"; import { defined } from "../../testkit/defined.js"; import { callFleetTool, @@ -1937,6 +1941,179 @@ describe("list_agents", () => { gate.resolve({ report: "done" }); }); + test("projects a bound grant_request_id without relying on ask prose", async () => { + getProcessWorkerGrantStore().clear(); + const sessions = createSubAgentSessionStore(); + const fleetRecords = createFleetMailbox(sessions); + const id = "grant-projection"; + sessions.start({ + id, + description: "need approval", + agentId: "a", + brief: "b", + }); + sessions.markRunning(id); + fleetRecords.register(id); + const envelope = getProcessWorkerGrantStore().register( + createDeniedCallEnvelope({ + callId: "denied-call", + tool: "run_shell", + action: "Run", + subject: "git status", + args: { command: "git status" }, + cwd: process.cwd(), + workerSessionId: id, + }), + ); + expect( + sessions.registerAsk(id, { + question: "Please decide how to proceed.", + questionId: "ask-1", + grantRequestId: envelope.requestId, + assessment: assessedAsk, + resolve: () => undefined, + reject: () => undefined, + }), + ).toBe(true); + + const list = createListAgentsTool({ sessions, fleetRecords }); + const listed = parseFleetJson( + (await callFleetToolRaw(list, {})).content, + ) as { + agents: { question?: string; grant_request_id?: string }[]; + }; + const listedAgent = defined(listed.agents[0]); + expect(listedAgent.question).toBe("Please decide how to proceed."); + expect(listedAgent.grant_request_id).toBe(envelope.requestId); + + const wait = createWaitAgentsTool({ sessions, fleetRecords }); + const waited = await callFleetTool(wait, { + targets: [id], + timeout_ms: 1, + }); + const waitedAgent = defined( + (waited.results as { grant_request_id?: string }[])[0], + ); + expect(waitedAgent.grant_request_id).toBe(envelope.requestId); + }); + + test("does not rebind a resolved denial to a later generic ask", async () => { + getProcessWorkerGrantStore().clear(); + const sessions = createSubAgentSessionStore(); + const fleetRecords = createFleetMailbox(sessions); + const id = "grant-projection-no-rebind"; + sessions.start({ + id, + description: "need approval", + agentId: "a", + brief: "b", + }); + sessions.markRunning(id); + fleetRecords.register(id); + const grantStore = getProcessWorkerGrantStore(); + const envelopeA = grantStore.register( + createDeniedCallEnvelope({ + callId: "denied-call-A", + tool: "run_shell", + action: "Run", + subject: "git status", + args: { command: "git status" }, + cwd: process.cwd(), + workerSessionId: id, + }), + ); + expect( + sessions.registerAsk(id, { + question: "Please decide how to proceed.", + questionId: "ask-1", + grantRequestId: envelopeA.requestId, + assessment: assessedAsk, + resolve: () => undefined, + reject: () => undefined, + }), + ).toBe(true); + expect( + sessions.resolveEscalationAsk(id, { + kind: "minimum_grant_available", + answer: "continue", + questionId: "ask-1", + }), + ).toBe(true); + expect(grantStore.peek(envelopeA.requestId)?.status).toBe("pending"); + + expect( + sessions.registerAsk(id, { + question: "What should I do next?", + questionId: "ask-2", + assessment: assessedAsk, + resolve: () => undefined, + reject: () => undefined, + }), + ).toBe(true); + expect(sessions.peekAsk(id)?.deniedCall).toBeUndefined(); + + const list = createListAgentsTool({ sessions, fleetRecords }); + const listed = parseFleetJson( + (await callFleetToolRaw(list, {})).content, + ) as { agents: { grant_request_id?: string }[] }; + expect(defined(listed.agents[0]).grant_request_id).toBeUndefined(); + + const wait = createWaitAgentsTool({ sessions, fleetRecords }); + const waited = await callFleetTool(wait, { + targets: [id], + timeout_ms: 1, + }); + expect( + defined((waited.results as { grant_request_id?: string }[])[0]) + .grant_request_id, + ).toBeUndefined(); + + expect(sessions.resolveAsk(id, "continue")).toBe(true); + const envelopeB = grantStore.register( + createDeniedCallEnvelope({ + callId: "denied-call-B", + tool: "run_shell", + action: "Run", + subject: "git diff", + args: { command: "git diff" }, + cwd: process.cwd(), + workerSessionId: id, + }), + ); + expect( + sessions.registerAsk(id, { + question: "May I retry git diff?", + questionId: "ask-3", + grantRequestId: envelopeB.requestId, + assessment: assessedAsk, + resolve: () => undefined, + reject: () => undefined, + }), + ).toBe(true); + expect(sessions.peekAsk(id)?.deniedCall?.requestId).toBe( + envelopeB.requestId, + ); + expect(grantStore.peek(envelopeA.requestId)?.questionId).toBe("ask-1"); + + const listB = createListAgentsTool({ sessions, fleetRecords }); + const listedB = parseFleetJson( + (await callFleetToolRaw(listB, {})).content, + ) as { agents: { grant_request_id?: string }[] }; + expect(defined(listedB.agents[0]).grant_request_id).toBe( + envelopeB.requestId, + ); + + const waitB = createWaitAgentsTool({ sessions, fleetRecords }); + const waitedB = await callFleetTool(waitB, { + targets: [id], + timeout_ms: 1, + }); + expect( + defined((waitedB.results as { grant_request_id?: string }[])[0]) + .grant_request_id, + ).toBe(envelopeB.requestId); + }); + test("errors after wait_agents surfaces awaiting_director with a question", async () => { const { gate, deps, list, id } = await spawnParkedAsk(); const wait = waitTool(deps); diff --git a/src/subagent/agent-fleet.ts b/src/subagent/agent-fleet.ts index cf8a55df5..2eb0be89b 100644 --- a/src/subagent/agent-fleet.ts +++ b/src/subagent/agent-fleet.ts @@ -152,6 +152,7 @@ interface FleetRecord { hint?: string; question?: string; questionId?: string; + grantRequestId?: string; assessment?: EscalationAssessment; terminalOutcome?: TerminalOutcome; description?: string; @@ -539,6 +540,9 @@ class FleetMailbox { ? { question: ask.question, questionId: ask.questionId, + ...(ask.deniedCall !== undefined + ? { grantRequestId: ask.deniedCall.requestId } + : {}), ...(ask.assessment !== undefined ? { assessment: ask.assessment } : {}), @@ -2042,6 +2046,9 @@ export function createWaitAgentsTool(deps: WaitAgentsDeps): AgentTool { ...(record.questionId !== undefined ? { question_id: record.questionId } : {}), + ...(record.grantRequestId !== undefined + ? { grant_request_id: record.grantRequestId } + : {}), ...(record.assessment !== undefined ? { assessment: record.assessment } : {}), @@ -2196,6 +2203,10 @@ export function createListAgentsTool(deps: WaitAgentsDeps): AgentTool { record.questionId !== undefined ? { question_id: record.questionId } : {}), + ...(record?.status === "awaiting_director" && + record.grantRequestId !== undefined + ? { grant_request_id: record.grantRequestId } + : {}), ...(record?.assessment !== undefined ? { assessment: record.assessment } : {}), diff --git a/src/subagent/authority.test.ts b/src/subagent/authority.test.ts index 604d11368..6f0fbbdcc 100644 --- a/src/subagent/authority.test.ts +++ b/src/subagent/authority.test.ts @@ -2,6 +2,7 @@ import { describe, expect, test } from "bun:test"; import { assertCanTargetAgent, assertTierMayMountFleetVerb, + FLEET_VERBS, FleetAuthorityError, isFleetVerb, } from "./authority.js"; @@ -70,6 +71,8 @@ describe("assertTierMayMountFleetVerb", () => { test("isFleetVerb matches the same set used for the gate", () => { expect(isFleetVerb("spawn_agent")).toBe(true); + expect(FLEET_VERBS.has("approve_worker_call")).toBe(true); + expect(isFleetVerb("approve_worker_call")).toBe(true); expect(isFleetVerb("task")).toBe(false); expect(isFleetVerb("write_file")).toBe(false); }); diff --git a/src/subagent/authority.ts b/src/subagent/authority.ts index 015f3e263..88f3e78d4 100644 --- a/src/subagent/authority.ts +++ b/src/subagent/authority.ts @@ -23,6 +23,7 @@ export const FLEET_VERBS = new Set([ "close_agent", "resume_agent", "read_agent_trace", + "approve_worker_call", ]); /** Fleet discovery is Tier 1 only: nested orchestrators spawn from a diff --git a/src/subagent/fleet-test-harness.ts b/src/subagent/fleet-test-harness.ts index bf95fc92b..ec88d71c1 100644 --- a/src/subagent/fleet-test-harness.ts +++ b/src/subagent/fleet-test-harness.ts @@ -18,6 +18,7 @@ import { createInterruptAgentTool, createResumeAgentTool, createSendInputTool, + createApproveWorkerCallTool, } from "./lifecycle-tools.js"; import type { FleetDryLane, @@ -85,6 +86,10 @@ export function fleetTools(deps: AgentFleetDeps) { wait: createWaitAgentsTool(scope), list: createListAgentsTool(scope), sendInput: createSendInputTool(scope), + approveWorkerCall: createApproveWorkerCallTool({ + ...scope, + permissionGate: deps.permissionGate, + }), interrupt: createInterruptAgentTool(scope), close: createCloseAgentTool(scope), resume: createResumeAgentTool(scope), diff --git a/src/subagent/lifecycle-tools.test.ts b/src/subagent/lifecycle-tools.test.ts index 6ca0bfc62..1d5cb97d3 100644 --- a/src/subagent/lifecycle-tools.test.ts +++ b/src/subagent/lifecycle-tools.test.ts @@ -5,6 +5,7 @@ import { createResumeAgentTool, createInterruptAgentTool, createSendInputTool, + createApproveWorkerCallTool, resumeAgentToolDefinition, } from "./lifecycle-tools.js"; import { @@ -18,6 +19,16 @@ import { type SubAgentSession, } from "./session-store.js"; import { createAdmissionQueue } from "./admission.js"; +import { runWithSubAgentIdentity } from "./identity-context.js"; +import { createPermissionGate } from "../permission/gate.js"; +import { + createWorkerAuthorize, + workerPermissionGate, +} from "../permission/reactor-authorize.js"; +import { + createDeniedCallEnvelope, + getProcessWorkerGrantStore, +} from "../permission/worker-grant.js"; import { defined } from "../../testkit/defined.js"; import { callFleetTool, callFleetToolRaw } from "./fleet-test-harness.js"; @@ -25,6 +36,67 @@ const callTool = callFleetTool; type SessionStore = ReturnType; +function parentGate(cwd: string, allow: boolean) { + return createPermissionGate({ + approvals: [], + cwd, + interactive: true, + skipPermissions: false, + reactorGated: false, + requestApproval: async (request) => { + if (!allow) return { allow: false }; + const exact = request.scopes[0]; + return exact === undefined + ? { allow: true } + : { allow: true, persist: { ...exact, grant: "session" as const } }; + }, + }); +} + +function pendingDeniedAsk( + sessions: SessionStore, + options: { + id: string; + cwd: string; + command: string; + expiresAt?: number; + parentSessionId?: string; + }, +) { + const session = startSession(sessions, { + id: options.id, + description: "worker", + ...(options.parentSessionId !== undefined + ? { parentSessionId: options.parentSessionId } + : {}), + }); + sessions.markRunning(session.id); + const envelope = getProcessWorkerGrantStore().register( + createDeniedCallEnvelope({ + callId: `${session.id}-denied`, + tool: "run_shell", + action: "Run", + subject: options.command, + args: { command: options.command }, + cwd: options.cwd, + workerSessionId: session.id, + }), + ); + if (options.expiresAt !== undefined) envelope.expiresAt = options.expiresAt; + const received: string[] = []; + expect( + sessions.registerAsk(session.id, { + question: "approve exact denied call", + questionId: `${session.id}-question`, + grantRequestId: envelope.requestId, + assessment: assessedAsk, + resolve: (answer) => received.push(answer), + reject: () => undefined, + }), + ).toBe(true); + return { session, envelope, received }; +} + /** Valid assessed ask payload for the structured-resolution tests. */ const assessedAsk = { policyVersion: "1", @@ -1306,3 +1378,277 @@ describe("nested lifecycle authority", () => { }, ); }); + +describe("approve_worker_call", () => { + test("nested parent reaches the approval handler through worker authz while its gate still denies the child call", async () => { + const grants = getProcessWorkerGrantStore(); + grants.clear(); + const cwd = "/tmp/nested-worker-approval"; + const sessions = createSubAgentSessionStore(); + const nested = startSession(sessions, { + id: "nested-parent", + description: "nested parent", + }); + const { session, envelope } = pendingDeniedAsk(sessions, { + id: "nested-child", + cwd, + command: "printf changed > generated.txt", + parentSessionId: nested.id, + }); + const permissionGate = parentGate(cwd, false); + const args = { + target: session.id, + question_id: `${session.id}-question`, + request_id: envelope.requestId, + }; + + expect( + ( + await createWorkerAuthorize(permissionGate)( + "tool:approve_worker_call", + "invoke", + { id: "approve-1", name: "approve_worker_call", arguments: args }, + ) + ).effect, + ).toBe("allow"); + + const approve = createApproveWorkerCallTool({ + sessions, + authority: { + actorId: nested.id, + tier: "nested-orchestrator", + getNodes: () => sessions.list(), + }, + permissionGate, + }); + const denied = await callFleetToolRaw(approve, args); + expect(denied.isError).toBe(true); + expect(denied.content).toContain("parent permission evaluation denied"); + expect(sessions.hasPendingAsk(session.id)).toBe(true); + expect(envelope.status).toBe("pending"); + grants.clear(); + }); + + test.each([ + ["read-only Git-shaped", "git status --short"], + ["consequential shell", "printf changed > generated.txt"], + ])( + "%s authorizes only the exact pending retry once", + async (_name, command) => { + const grants = getProcessWorkerGrantStore(); + grants.clear(); + const cwd = "/tmp/worker-approval"; + const sessions = createSubAgentSessionStore(); + const gate = parentGate(cwd, true); + const { session, envelope, received } = pendingDeniedAsk(sessions, { + id: `worker-${command.length}`, + cwd, + command, + }); + const approve = createApproveWorkerCallTool({ + sessions, + permissionGate: gate, + }); + + const approved = await callTool(approve, { + target: session.id, + question_id: `${session.id}-question`, + request_id: envelope.requestId, + }); + expect(approved.status).toBe("authorized"); + expect(received).toEqual([ + "Parent approved the exact denied call; retry it once.", + ]); + expect(sessions.hasPendingAsk(session.id)).toBe(false); + + const workerGate = workerPermissionGate(gate, { sessionId: session.id }); + expect( + await runWithSubAgentIdentity({ description: "worker", cwd }, () => + workerGate.authorizeCall({ + id: "retry-1", + name: "run_shell", + arguments: { command }, + }), + ), + ).toEqual({ effect: "allow" }); + expect( + await runWithSubAgentIdentity({ description: "worker", cwd }, () => + workerGate.executionVerdict({ + id: "retry-1", + name: "run_shell", + arguments: { command }, + }), + ), + ).toEqual({ effect: "allow" }); + expect(grants.peek(envelope.requestId)?.status).toBe("consumed"); + const duplicateExecution = await runWithSubAgentIdentity( + { description: "worker", cwd }, + () => + workerGate.executionVerdict({ + id: "retry-2", + name: "run_shell", + arguments: { command }, + }), + ); + expect(duplicateExecution.effect).toBe("deny"); + grants.clear(); + }, + ); + + test("an in-flight allowed retry cannot leave an authorized replay", async () => { + const grants = getProcessWorkerGrantStore(); + grants.clear(); + const cwd = "/tmp/worker-approval-race"; + const command = "printf changed > generated.txt"; + const sessions = createSubAgentSessionStore(); + const { session, envelope } = pendingDeniedAsk(sessions, { + id: "worker-race", + cwd, + command, + }); + const baseGate = parentGate(cwd, true); + let enteredExecution!: () => void; + const executionEntered = new Promise((resolve) => { + enteredExecution = resolve; + }); + let resumeExecution!: () => void; + const executionGate = new Promise((resolve) => { + resumeExecution = resolve; + }); + const gate = { + ...baseGate, + executionVerdict: async () => { + enteredExecution(); + await executionGate; + return { effect: "allow" } as const; + }, + }; + const workerGate = workerPermissionGate(gate, { sessionId: session.id }); + const execution = runWithSubAgentIdentity( + { description: "worker", cwd }, + () => + workerGate.executionVerdict({ + id: "retry-racing", + name: "run_shell", + arguments: { command }, + }), + ); + await executionEntered; + + const approve = createApproveWorkerCallTool({ + sessions, + permissionGate: gate, + }); + const approval = callFleetToolRaw(approve, { + target: session.id, + question_id: `${session.id}-question`, + request_id: envelope.requestId, + }); + resumeExecution(); + + await expect(execution).resolves.toEqual({ effect: "allow" }); + const approvalResult = await approval; + expect(approvalResult.isError).toBe(true); + expect(String(approvalResult.content)).toContain("does not match"); + expect(envelope.status).toBe("consumed"); + await expect( + runWithSubAgentIdentity({ description: "worker", cwd }, () => + workerGate.executionVerdict({ + id: "retry-replay", + name: "run_shell", + arguments: { command }, + }), + ), + ).resolves.toMatchObject({ effect: "deny" }); + grants.clear(); + }); + + test("rejects wrong or stale IDs, altered envelope args/CWD, expiry, decline, failed evaluation, and duplicate approval", async () => { + const grants = getProcessWorkerGrantStore(); + grants.clear(); + const cwd = "/tmp/worker-approval-fail-closed"; + const sessions = createSubAgentSessionStore(); + const { session, envelope } = pendingDeniedAsk(sessions, { + id: "worker-fail-closed", + cwd, + command: "git status --short", + }); + const approve = createApproveWorkerCallTool({ + sessions, + permissionGate: parentGate(cwd, true), + }); + const args = { + target: session.id, + question_id: `${session.id}-question`, + request_id: envelope.requestId, + }; + + for (const invalid of [ + { ...args, target: "wrong-worker" }, + { ...args, question_id: "wrong-question" }, + { ...args, request_id: "wrong-request" }, + ]) { + const result = await callFleetToolRaw(approve, invalid); + expect(result.isError).toBe(true); + expect(sessions.hasPendingAsk(session.id)).toBe(true); + } + + envelope.args.command = "git reset --hard"; + expect((await callFleetToolRaw(approve, args)).isError).toBe(true); + envelope.args.command = "git status --short"; + envelope.cwd = "/tmp/altered-cwd"; + expect((await callFleetToolRaw(approve, args)).isError).toBe(true); + envelope.cwd = cwd; + + envelope.expiresAt = Date.now() - 1; + expect((await callFleetToolRaw(approve, args)).isError).toBe(true); + expect(envelope.status).toBe("expired"); + grants.clear(); + + const declinedSessions = createSubAgentSessionStore(); + const declined = pendingDeniedAsk(declinedSessions, { + id: "worker-declined", + cwd, + command: "printf changed > generated.txt", + }); + const declineTool = createApproveWorkerCallTool({ + sessions: declinedSessions, + permissionGate: parentGate(cwd, false), + }); + const declinedArgs = { + target: declined.session.id, + question_id: `${declined.session.id}-question`, + request_id: declined.envelope.requestId, + }; + expect((await callFleetToolRaw(declineTool, declinedArgs)).isError).toBe( + true, + ); + expect(declinedSessions.hasPendingAsk(declined.session.id)).toBe(true); + + const failedTool = createApproveWorkerCallTool({ + sessions: declinedSessions, + permissionGate: { + ...parentGate(cwd, true), + evaluate: async () => { + throw new Error("gate unavailable"); + }, + }, + }); + expect((await callFleetToolRaw(failedTool, declinedArgs)).isError).toBe( + true, + ); + expect(declinedSessions.hasPendingAsk(declined.session.id)).toBe(true); + + const successTool = createApproveWorkerCallTool({ + sessions: declinedSessions, + permissionGate: parentGate(cwd, true), + }); + expect((await callTool(successTool, declinedArgs)).status).toBe( + "authorized", + ); + expect((await callFleetToolRaw(successTool, declinedArgs)).isError).toBe( + true, + ); + grants.clear(); + }); +}); diff --git a/src/subagent/lifecycle-tools.ts b/src/subagent/lifecycle-tools.ts index c04051af0..2b189c861 100644 --- a/src/subagent/lifecycle-tools.ts +++ b/src/subagent/lifecycle-tools.ts @@ -31,6 +31,13 @@ import { type SubAgentSessionStore, } from "./session-store.js"; import { parseEscalationResolution } from "./escalation-policy.js"; +import { runWithSubAgentIdentity } from "./identity-context.js"; +import type { PermissionGate } from "../permission/gate.js"; +import { + canonicalDeniedTool, + fingerprintDeniedCall, + getProcessWorkerGrantStore, +} from "../permission/worker-grant.js"; import { assertCanTargetAgent, FleetAuthorityError, @@ -132,6 +139,10 @@ export interface LifecycleToolDeps { authority?: LifecycleAuthority; } +export type ApproveWorkerCallToolDeps = LifecycleToolDeps & { + permissionGate: PermissionGate; +}; + /** close_agent always terminalizes the wait mailbox — no silent skip. */ export type CloseAgentToolDeps = LifecycleToolDeps & { fleetRecords: FleetMailboxHandle; @@ -623,3 +634,180 @@ export function createSendInputTool(deps: LifecycleToolDeps): AgentTool { }, }); } + +const ApproveWorkerCallArgs = type({ + target: "string", + question_id: "string", + request_id: "string", +}); + +export const approveWorkerCallToolDefinition: ToolDefinition = { + name: "approve_worker_call", + description: + "Approve one exact pending worker-denied call. The target, question_id, and request_id must match the live retained envelope; this action evaluates only its frozen tool call through the parent permission gate before waking the worker.", + inputSchema: { + type: "object", + properties: { + target: { type: "string", description: "agent_id." }, + question_id: { + type: "string", + description: "Pending ask_director question id.", + }, + request_id: { + type: "string", + description: "Denied-call grant request id.", + }, + }, + required: ["target", "question_id", "request_id"], + }, +}; + +export function createApproveWorkerCallTool( + deps: ApproveWorkerCallToolDeps, +): AgentTool { + return tool({ + definition: approveWorkerCallToolDefinition, + handler: async (call, _signal): Promise => { + const parsed = ApproveWorkerCallArgs(call.arguments); + if (parsed instanceof type.errors) { + return lifecycleResult( + call.id, + `Error: approve_worker_call arguments invalid: ${parsed.summary}`, + ); + } + const target = parsed.target.trim(); + const questionId = parsed.question_id.trim(); + const requestId = parsed.request_id.trim(); + const denied = gateTarget(deps, "approve_worker_call", target, call.id); + if (denied !== undefined) return denied; + if ( + target.length === 0 || + questionId.length === 0 || + requestId.length === 0 + ) { + return lifecycleResult( + call.id, + "Error: approve_worker_call requires non-empty target, question_id, and request_id.", + ); + } + + const grants = getProcessWorkerGrantStore(); + const envelope = grants.peek(requestId); + if (envelope === undefined) + return lifecycleResult( + call.id, + "Error: approve_worker_call does not match a live pending worker-denied call.", + ); + const approve = async (): Promise => { + return grants.runExclusive(requestId, async () => { + const pending = deps.sessions.peekAsk(target); + const session = deps.sessions.get(target); + const validatedEnvelope = grants.peek(requestId); + if ( + pending === undefined || + session === undefined || + pending.questionId !== questionId || + pending.deniedCall !== validatedEnvelope || + validatedEnvelope === undefined || + validatedEnvelope.workerSessionId !== target || + validatedEnvelope.questionId !== questionId || + validatedEnvelope.status !== "pending" || + validatedEnvelope.expiresAt <= Date.now() || + canonicalDeniedTool(validatedEnvelope.canonicalTool) !== + validatedEnvelope.canonicalTool || + fingerprintDeniedCall( + validatedEnvelope.canonicalTool, + validatedEnvelope.args, + validatedEnvelope.cwd, + ) !== validatedEnvelope.argsFingerprint + ) { + grants.sweepExpired(); + return lifecycleResult( + call.id, + "Error: approve_worker_call does not match a live pending worker-denied call.", + ); + } + + const frozenCall = { + id: validatedEnvelope.deniedCallId, + name: validatedEnvelope.canonicalTool, + arguments: JSON.parse( + JSON.stringify(validatedEnvelope.args), + ) as Record, + }; + let verdict: Awaited>; + try { + verdict = await runWithSubAgentIdentity( + { description: session.description, cwd: validatedEnvelope.cwd }, + () => deps.permissionGate.evaluate(frozenCall), + ); + } catch { + return lifecycleResult( + call.id, + "Error: parent permission evaluation failed; the worker remains blocked.", + ); + } + if (!verdict.allowed) { + return lifecycleResult( + call.id, + `Error: parent permission evaluation denied the pending worker call: ${verdict.reason}`, + ); + } + + const current = deps.sessions.peekAsk(target); + const currentEnvelope = grants.peek(requestId); + if ( + current === undefined || + current.questionId !== questionId || + current.deniedCall !== currentEnvelope || + currentEnvelope !== envelope || + currentEnvelope.status !== "pending" || + currentEnvelope.expiresAt <= Date.now() + ) { + grants.sweepExpired(); + return lifecycleResult( + call.id, + "Error: approve_worker_call became stale before authorization completed.", + ); + } + if (!grants.authorizeRetry(requestId)) { + return lifecycleResult( + call.id, + "Error: approve_worker_call became stale before retry authorization completed.", + ); + } + if ( + !deps.sessions.resolveAsk( + target, + "Parent approved the exact denied call; retry it once.", + ) + ) { + grants.revokeAuthorizedRetry(requestId); + return lifecycleResult( + call.id, + "Error: approve_worker_call could not wake the pending worker.", + ); + } + return lifecycleResult( + call.id, + fleetJson({ + agent_id: target, + question_id: questionId, + request_id: requestId, + status: "authorized", + }), + ); + }); + }; + return grants.runExclusiveIdentity( + { + sessionId: envelope.workerSessionId, + canonicalTool: envelope.canonicalTool, + args: envelope.args, + cwd: envelope.cwd, + }, + approve, + ); + }, + }); +} diff --git a/src/subagent/run-authority.test.ts b/src/subagent/run-authority.test.ts index 36f2c63de..83fde057d 100644 --- a/src/subagent/run-authority.test.ts +++ b/src/subagent/run-authority.test.ts @@ -226,3 +226,32 @@ describe("runSubAgent list_agents mount (mailbox-scoped, nested ok)", () => { expect(listAgentsMounts).toBe(1); }, 15_000); }); + +describe("runSubAgent approve_worker_call mount (nested control plane)", () => { + test("nested-orchestrator mounts approve_worker_call", async () => { + const cwd = await tmpCwd(); + let approvalMounts = 0; + + await probeMount( + cwd, + import.meta.resolve("./lifecycle-tools.js"), + (real: typeof import("./lifecycle-tools.js")) => ({ + ...real, + createApproveWorkerCallTool: ( + deps: Parameters[0], + ) => { + approvalMounts++; + return real.createApproveWorkerCallTool(deps); + }, + }), + (baseURL) => ({ + id: "nested-parent-session", + orchestrator: true, + orchestratorTier: "nested-orchestrator", + nestedDispatch: nestedDispatch(cwd, baseURL), + }), + ); + + expect(approvalMounts).toBe(1); + }, 15_000); +}); diff --git a/src/subagent/run.ts b/src/subagent/run.ts index bb10f40d2..885c17c1a 100644 --- a/src/subagent/run.ts +++ b/src/subagent/run.ts @@ -195,6 +195,7 @@ import { createResumeAgentTool, createInterruptAgentTool, createSendInputTool, + createApproveWorkerCallTool, } from "./lifecycle-tools.js"; import { createSubAgentSessionStore } from "./session-store.js"; import type { @@ -1021,6 +1022,7 @@ async function runSubAgentInner( "resume_agent", "interrupt_agent", "send_input", + "approve_worker_call", ]) { assertTierMayMountFleetVerb(tier, verb); } @@ -1129,6 +1131,12 @@ async function runSubAgentInner( fleetRecords, authority: lifecycleAuthority, }), + createApproveWorkerCallTool({ + sessions: fleetSessions, + fleetRecords, + authority: lifecycleAuthority, + permissionGate: nd.permissionGate, + }), ]; } diff --git a/src/subagent/session-store.ts b/src/subagent/session-store.ts index ad9fec28a..5381e5efd 100644 --- a/src/subagent/session-store.ts +++ b/src/subagent/session-store.ts @@ -2094,8 +2094,8 @@ export function createSubAgentSessionStore( if (pendingAsks.has(id)) return false; // Attach the harness-owned denied-call envelope (exact ToolCall + hash) // to the ask record, stamping its questionId for the parent's retry - // ref. A named grantRequestId binds that exact denial; an unnamed ask - // falls back to the session's first pending envelope. + // ref. Only a named grantRequestId binds its exact denial; generic asks + // must not inherit a pending envelope from an earlier question. let deniedCall: WorkerDeniedCallEnvelope | undefined; try { deniedCall = From a92761f63ecf37c10da1644902b969790d85af80 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 15 Oct 2026 12:14:10 -0700 Subject: [PATCH 2/2] fix(permissions): steer parent grants to approve_worker_call Parked denied-call asks name approve_worker_call and the grant request id. Revoking an authorization records a revoked audit event. --- src/permission/worker-grant.test.ts | 19 +++++++++++++ src/permission/worker-grant.ts | 14 ++++++---- src/subagent/agent-fleet.test.ts | 14 ++++++++++ src/subagent/agent-fleet.ts | 43 ++++++++++++++++++++++------- 4 files changed, 75 insertions(+), 15 deletions(-) diff --git a/src/permission/worker-grant.test.ts b/src/permission/worker-grant.test.ts index 57a13716b..e8f79b671 100644 --- a/src/permission/worker-grant.test.ts +++ b/src/permission/worker-grant.test.ts @@ -401,6 +401,25 @@ describe("WorkerGrantStore lifecycle", () => { ]); }); + test("revokeAuthorizedRetry returns authorized to pending and audits revoked", () => { + const store = new WorkerGrantStore(); + const envelope = store.register(createDeniedCallEnvelope(descriptor())); + expect(store.authorizeRetry(envelope.requestId, 1_000_001)).toBe(true); + store.revokeAuthorizedRetry(envelope.requestId); + expect(envelope.status).toBe("pending"); + expect(envelope.audit.map((event) => event.event)).toEqual([ + "denied", + "authorized", + "revoked", + ]); + store.revokeAuthorizedRetry(envelope.requestId); + expect(envelope.audit.map((event) => event.event)).toEqual([ + "denied", + "authorized", + "revoked", + ]); + }); + test("no text API: state moves only on typed identities, never prose", () => { const store = new WorkerGrantStore(); const envelope = store.register(createDeniedCallEnvelope(descriptor())); diff --git a/src/permission/worker-grant.ts b/src/permission/worker-grant.ts index 78219f8e6..aeab7451d 100644 --- a/src/permission/worker-grant.ts +++ b/src/permission/worker-grant.ts @@ -1,8 +1,9 @@ -// Worker denied-call envelope: a worker tool call denied pending operator +// Worker denied-call envelope: a worker tool call denied pending parent // approval. The deny text names only the requestId; the envelope carries the -// exact denied call so the parent replays it through its own gate and the -// worker retries via resume_agent. Prose is never authority: nothing here -// parses message text, and send_input stays text-only. +// exact denied call so the parent replays it through its own gate. +// approve_worker_call authorizes the retry, then resolveAsk continues the +// parked turn. Prose is never authority: nothing here parses message text, +// and send_input stays text-only. import { createHash, randomUUID } from "node:crypto"; @@ -27,6 +28,7 @@ export interface WorkerGrantAuditEvent { | "denied" | "asked" | "authorized" + | "revoked" | "consumed" | "expired" | "interrupted"; @@ -324,7 +326,9 @@ export class WorkerGrantStore { * ask. No worker can receive the authorization without that wake. */ revokeAuthorizedRetry(requestId: string): void { const envelope = this.envelopes.get(requestId); - if (envelope?.status === "authorized") envelope.status = "pending"; + if (envelope?.status !== "authorized") return; + envelope.status = "pending"; + audit(envelope, "revoked"); } hasAuthorizedRetry(identity: WorkerCallIdentity): boolean { diff --git a/src/subagent/agent-fleet.test.ts b/src/subagent/agent-fleet.test.ts index 5a86311d3..0ef7f8196 100644 --- a/src/subagent/agent-fleet.test.ts +++ b/src/subagent/agent-fleet.test.ts @@ -10,6 +10,7 @@ import { createListAgentsTool, MAX_FLEET_RECORDS, type AgentFleetDeps, + waitAgentsToolDefinition, } from "./agent-fleet.js"; import { createAdmissionQueue } from "./admission.js"; import { @@ -1995,6 +1996,19 @@ describe("list_agents", () => { (waited.results as { grant_request_id?: string }[])[0], ); expect(waitedAgent.grant_request_id).toBe(envelope.requestId); + expect(waitAgentsToolDefinition.description).toContain( + "approve_worker_call", + ); + expect(waitAgentsToolDefinition.description).toContain("grant_request_id"); + expect(waitAgentsToolDefinition.description).toContain( + "send_input does not grant", + ); + + const refused = await callFleetToolRaw(list, {}); + expect(refused.isError).toBe(true); + expect(refused.content).toContain("approve_worker_call"); + expect(refused.content).toContain(envelope.requestId); + expect(refused.content).toContain("send_input does not grant"); }); test("does not rebind a resolved denial to a later generic ask", async () => { diff --git a/src/subagent/agent-fleet.ts b/src/subagent/agent-fleet.ts index 2eb0be89b..b8a5e3538 100644 --- a/src/subagent/agent-fleet.ts +++ b/src/subagent/agent-fleet.ts @@ -687,7 +687,7 @@ export const MAX_WAIT_TIMEOUT_MS = 300_000; export const waitAgentsToolDefinition: ToolDefinition = { name: "wait_agents", description: - "Block until targets finish, fail, or ask (awaiting_director), or timeout_ms. mode any (default) or all. Timeout is not an error; workers keep running. Answer awaiting_director with send_input. failed+continuable:true: recover once via spawn_agent recovers=agent_id.", + "Block until targets finish, fail, or ask (awaiting_director), or timeout_ms. mode any (default) or all. Timeout is not an error; workers keep running. awaiting_director with grant_request_id: approve_worker_call using that id (send_input does not grant). Other asks: send_input (soft). failed+continuable:true: recover once via spawn_agent recovers=agent_id.", inputSchema: { type: "object", properties: { @@ -2129,10 +2129,18 @@ export const listAgentsToolDefinition: ToolDefinition = { }, }; -function collectParkedAsks( - fleetRecords: FleetMailboxHandle, -): { id: string; questionId: string; question: string }[] { - const parked: { id: string; questionId: string; question: string }[] = []; +function collectParkedAsks(fleetRecords: FleetMailboxHandle): { + id: string; + questionId: string; + question: string; + grantRequestId?: string; +}[] { + const parked: { + id: string; + questionId: string; + question: string; + grantRequestId?: string; + }[] = []; for (const id of fleetRecords.ids()) { const record = fleetRecords.peek(id); if ( @@ -2146,6 +2154,9 @@ function collectParkedAsks( id, questionId: record.questionId, question: record.question, + ...(record.grantRequestId !== undefined + ? { grantRequestId: record.grantRequestId } + : {}), }); } return parked; @@ -2153,16 +2164,28 @@ function collectParkedAsks( function refuseSurfacedParkedAsks( callId: string, - parked: readonly { id: string; questionId: string; question: string }[], + parked: readonly { + id: string; + questionId: string; + question: string; + grantRequestId?: string; + }[], ): ToolResult { const targets = parked.map((row) => row.id).join(", "); - const lines = parked.map( - (row) => `target ${row.id} question_id ${row.questionId}: ${row.question}`, - ); + const lines = parked.map((row) => { + const grant = + row.grantRequestId !== undefined + ? ` grant_request_id ${row.grantRequestId}` + : ""; + return `target ${row.id} question_id ${row.questionId}${grant}: ${row.question}`; + }); + const steer = parked.some((row) => row.grantRequestId !== undefined) + ? `Approve a denied worker call with approve_worker_call using its grant_request_id (send_input does not grant). Other parked asks: send_input (soft) using target ${targets}, or escalate with ask_operator.` + : `Answer with send_input (soft) using target ${targets}, or escalate with ask_operator.`; return fleetResult( callId, [ - `Error: list_agents is not progress while parked ask_director questions are already surfaced. Answer with send_input (soft) using target ${targets}, or escalate with ask_operator.`, + `Error: list_agents is not progress while parked ask_director questions are already surfaced. ${steer}`, ...lines, ].join("\n"), );