diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 8759a8a17..b8d6f926f 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -255,7 +255,7 @@ Workers ask the spawning parent with **`ask_director`** (not the human). That pa The ask is never prose-only: `ask_director` requires a typed **`escalation`** object (`src/subagent/escalation-policy.ts`, `ask-director.ts`) with `policyVersion` plus the required facts — `classification`, `blockedOutcome`, `unavailableDirectorPath`, `permittedAlternatives`, `minimumAddition`, and `declineConsequence` (each a non-empty string, byte-bounded). It may also carry `recommendation`, `safeDefault`, `requestedMechanism` versus `minimumAuthority`, and an optional `verification` detail (`verificationOutcome`, `rootCause`, `attemptedNarrowChecks`, `reducedConfidence`). The five `classification` values decide where the ask settles (`decideEscalation`): `routine` never parks — the worker continues with one of its displayed permitted alternatives and the evaluation is recorded for audit (`routineEvaluations`, newest-only cap); it requires at least one alternative with `comparableConfidence: true`. `director_resolvable` also continues internally when such a comparable alternative exists and parks otherwise. `operator_decision_required`, `outcome_blocked`, and `irreversible_or_sensitive` always park — the first and last require an explicit `minimumAuthority`, which must differ from `requestedMechanism` when both are present — and every parked classification requires at least one permitted alternative to show what was already considered. -The parent resolves a parked ask with **`send_input`**'s optional structured `resolution` (`questionId`, `kind`, `answer`; `parseEscalationResolution` validates it at the tool boundary). The four kinds are `director_answer` (normal answer), `declined`, `unavailable`, and `minimum_grant_available` (resume by text only — it grants nothing and leaves a denied-call envelope unconsumed). A resolution is bound to the pending `questionId` — a stale or mismatched id is refused so a late answer cannot settle a newer question — and is mutually exclusive with `interrupt`. The resolution's message is delivered to the worker together with the structured answer; neither can silently drop the other. A resolution never grants authority or retries a tool — a denied call still needs the envelope-owned covering-grant retry path described above. +The parent resolves a parked ask with **`send_input`**'s optional structured `resolution` (`questionId`, `kind`, `answer`; `parseEscalationResolution` validates it at the tool boundary). The four kinds are `director_answer` (normal answer), `declined`, `unavailable`, and `minimum_grant_available` (resume by text only — it grants nothing and leaves a denied-call envelope unconsumed). A resolution is bound to the pending `questionId` — a stale or mismatched id is refused so a late answer cannot settle a newer question — and, while that ask is pending, cannot be combined with `interrupt` (answering resumes the parked turn, interrupting stops it). With no ask pending there is nothing to resolve, so `send_input` ignores the resolution, delivers the message as plain input (interrupting first when `interrupt` is set, exactly as without a resolution), and reports `resolution_ignored` in its result; models that fill every optional field would otherwise fail every plain message. The resolution's message is delivered to the worker together with the structured answer; neither can silently drop the other. A resolution never grants authority or retries a tool — a denied call still needs the envelope-owned covering-grant retry path described above. Projections keep the policy facts visible: wait JSON and `list_agents` carry the pending ask's `question`, `questionId`, and `assessment`, and the session's `terminal_outcome` (`verification_blocked` when a `declined`/`unavailable` resolution lands on an ask that carried a `verification` detail, `declined` for a plain decline/unavailability, or `verified` when the worker reports a successful verification in place of a blocked one) is projected through wait JSON, `list_agents`, and mailbox mail (`src/subagent/agent-fleet.ts`, `fleet-report.ts`). diff --git a/src/subagent/lifecycle-tools.test.ts b/src/subagent/lifecycle-tools.test.ts index 1d5cb97d3..671a37ce3 100644 --- a/src/subagent/lifecycle-tools.test.ts +++ b/src/subagent/lifecycle-tools.test.ts @@ -1287,6 +1287,186 @@ describe("send_input", () => { resolution: "declined", }); }); + + test("a resolution with no pending escalation is ignored and the message is delivered as plain input", async () => { + const sessions = createSubAgentSessionStore(); + const fleetRecords = createFleetMailbox(sessions); + const sendInput = createSendInputTool({ sessions, fleetRecords }); + const worker = startSession(sessions, { description: "worker" }); + sessions.markRunning(worker.id); + const delivered: string[] = []; + sessions.registerDeliver(worker.id, (message) => { + delivered.push(message); + }); + + // Filler a field-filling model sent after approve_worker_call had + // already answered the worker's only ask. + const fillers = [ + { questionId: "", kind: "director_answer", answer: "" }, + { + questionId: "ask-2", + kind: "director_answer", + answer: "Additional requirement applies after rebase.", + }, + { + questionId: "none", + kind: "unavailable", + answer: "No escalation; normal task update only.", + }, + ]; + for (const resolution of fillers) { + const result = await callTool(sendInput, { + target: worker.id, + message: "also require the PR checklist", + interrupt: false, + resolution, + }); + expect(result).toEqual({ + agent_id: worker.id, + status: "running", + resolution_ignored: "no pending escalation", + }); + } + expect(delivered).toEqual( + fillers.map(() => "also require the PR checklist"), + ); + }); + + test("an interrupt carrying a resolution with no pending escalation interrupts as a plain interrupt would", async () => { + let interrupted = false; + const followups: string[] = []; + const { sessions, fleetRecords, worker } = liveLane({ + interrupt: () => { + interrupted = true; + }, + followup: async (message) => { + followups.push(message); + return "queued turn finished"; + }, + deliver: () => { + throw new Error("interrupt:true should not soft-deliver"); + }, + }); + const sendInput = createSendInputTool({ sessions, fleetRecords }); + + const result = await callTool(sendInput, { + target: worker.id, + message: "patch only the test", + interrupt: true, + resolution: { + questionId: "none", + kind: "unavailable", + answer: "No escalation; normal task update only.", + }, + }); + expect(result).toEqual({ + agent_id: worker.id, + status: "interrupted", + resolution_ignored: "no pending escalation", + }); + expect(interrupted).toBe(true); + sessions.attachReport(worker.id, "salvage", { stopReason: "interrupted" }); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(followups).toEqual(["patch only the test"]); + }); + + test("an interrupt carrying a resolution is refused while an ask is pending", async () => { + let interrupted = false; + const { sessions, fleetRecords, worker } = liveLane({ + interrupt: () => { + interrupted = true; + }, + followup: async () => "unused", + }); + sessions.registerAsk(worker.id, { + question: "which file?", + questionId: "ask-1", + assessment: assessedAsk, + resolve: () => { + throw new Error("should not resolve"); + }, + reject: () => undefined, + }); + const sendInput = createSendInputTool({ sessions, fleetRecords }); + + const result = await callFleetToolRaw(sendInput, { + target: worker.id, + message: "use branch X", + interrupt: true, + resolution: { + questionId: "ask-1", + kind: "director_answer", + answer: "use branch X", + }, + }); + expect(result.isError).toBe(true); + expect(result.content).toContain("cannot interrupt"); + expect(interrupted).toBe(false); + expect(sessions.hasPendingAsk(worker.id)).toBe(true); + }); + + test("an interrupt carrying a malformed resolution while an ask is pending reports the interrupt conflict", async () => { + let interrupted = false; + const { sessions, fleetRecords, worker } = liveLane({ + interrupt: () => { + interrupted = true; + }, + followup: async () => "unused", + }); + sessions.registerAsk(worker.id, { + question: "which file?", + questionId: "ask-1", + assessment: assessedAsk, + resolve: () => { + throw new Error("should not resolve"); + }, + reject: () => undefined, + }); + const sendInput = createSendInputTool({ sessions, fleetRecords }); + + const result = await callFleetToolRaw(sendInput, { + target: worker.id, + message: "use branch X", + interrupt: true, + resolution: { questionId: "", kind: "not_a_kind", answer: "" }, + }); + expect(result.isError).toBe(true); + expect(result.content).toContain("cannot interrupt"); + expect(result.content).not.toContain("resolution invalid"); + expect(interrupted).toBe(false); + expect(sessions.hasPendingAsk(worker.id)).toBe(true); + }); + + test("an unusable resolution against a pending ask names the plain-message alternative", async () => { + const sessions = createSubAgentSessionStore(); + const fleetRecords = createFleetMailbox(sessions); + const sendInput = createSendInputTool({ sessions, fleetRecords }); + const worker = startSession(sessions, { description: "worker" }); + sessions.markRunning(worker.id); + sessions.registerAsk(worker.id, { + question: "which file?", + questionId: "ask-1", + assessment: assessedAsk, + resolve: () => undefined, + reject: () => undefined, + }); + + for (const resolution of [ + { questionId: "", kind: "director_answer", answer: "" }, + { questionId: "ask-0", kind: "declined", answer: "No." }, + ]) { + const result = await callFleetToolRaw(sendInput, { + target: worker.id, + message: "No.", + resolution, + }); + expect(result.isError).toBe(true); + expect(result.content).toContain( + "Omit resolution to answer with a plain message.", + ); + } + expect(sessions.hasPendingAsk(worker.id)).toBe(true); + }); }); describe("nested lifecycle authority", () => { diff --git a/src/subagent/lifecycle-tools.ts b/src/subagent/lifecycle-tools.ts index 2b189c861..fb110d6f5 100644 --- a/src/subagent/lifecycle-tools.ts +++ b/src/subagent/lifecycle-tools.ts @@ -448,6 +448,8 @@ export function createInterruptAgentTool( }); } +const OMIT_RESOLUTION_HINT = "Omit resolution to answer with a plain message."; + const SendInputArgs = type({ target: "string", message: "string", @@ -458,7 +460,7 @@ const SendInputArgs = type({ export const sendInputToolDefinition: ToolDefinition = { name: "send_input", description: - "Message a running worker only (check list_agents for status) — completed or interrupted workers need resume_agent instead; shutdown workers are gone (see read_agent_trace or spawn anew). Answers a pending ask_director; interrupt:true stops the current turn first and queues message as the next turn.", + "Message a running worker only (check list_agents for status) — completed or interrupted workers need resume_agent instead; shutdown workers are gone (see read_agent_trace or spawn anew). Answers a pending ask_director; a resolution sent with no ask pending is ignored and the message is delivered plainly (the result carries resolution_ignored). interrupt:true stops the current turn first and queues message as the next turn.", inputSchema: { type: "object", properties: { @@ -477,7 +479,7 @@ export const sendInputToolDefinition: ToolDefinition = { resolution: { type: "object", description: - "Optional explicit pending-escalation resolution (questionId, kind, answer). It never grants or retries a tool. The message is delivered together with the structured answer.", + "Optional explicit pending-escalation resolution (questionId, kind, answer). Omit it unless answering a pending ask_director; with no ask pending it is ignored and the message is delivered as plain input. It never grants or retries a tool. The message is delivered together with the structured answer.", properties: { questionId: { type: "string", @@ -534,31 +536,32 @@ export function createSendInputTool(deps: LifecycleToolDeps): AgentTool { `(got ${message.length}).`, ); } - if (parsed.resolution !== undefined) { - if (parsed.interrupt === true) { - return lifecycleResult( - call.id, - "Error: structured escalation resolution cannot interrupt a worker.", - ); - } + // Field-filling models send a resolution on every call. With no ask + // pending it has nothing to answer, so the message goes out as plain + // input instead of failing on filler. + const pending = deps.sessions.peekAsk(target); + const answersAsk = + parsed.resolution !== undefined && pending !== undefined; + // Answering resumes the parked turn and an interrupt would stop it. + // Checked before parsing so a malformed resolution still gets this error. + if (answersAsk && parsed.interrupt === true) { + return lifecycleResult( + call.id, + "Error: structured escalation resolution cannot interrupt a worker.", + ); + } + if (answersAsk) { const resolution = parseEscalationResolution(parsed.resolution); if (resolution instanceof Error) { return lifecycleResult( call.id, - `Error: send_input resolution invalid: ${resolution.message}`, - ); - } - const pending = deps.sessions.peekAsk(target); - if (pending === undefined) { - return lifecycleResult( - call.id, - `Error: no pending escalation for "${target}".`, + `Error: send_input resolution invalid: ${resolution.message} ${OMIT_RESOLUTION_HINT}`, ); } if (pending.questionId !== resolution.questionId) { return lifecycleResult( call.id, - `Error: send_input resolution questionId mismatch: pending ask is "${pending.questionId}", resolution names "${resolution.questionId}".`, + `Error: send_input resolution questionId mismatch: pending ask is "${pending.questionId}", resolution names "${resolution.questionId}". ${OMIT_RESOLUTION_HINT}`, ); } // SF6: deliver the parent's message together with the structured @@ -629,7 +632,13 @@ export function createSendInputTool(deps: LifecycleToolDeps): AgentTool { } return lifecycleResult( call.id, - fleetJson({ agent_id: target, status: outcome.status }), + fleetJson({ + agent_id: target, + status: outcome.status, + ...(parsed.resolution !== undefined + ? { resolution_ignored: "no pending escalation" } + : {}), + }), ); }, });