Skip to content

Commit 247b2ee

Browse files
committed
fix(subagent): deliver send_input plainly when no escalation is pending
Models that fill every optional field attach a resolution to ordinary messages. With no ask pending the resolution has nothing to answer, yet it routed the call into the escalation path, which rejected it, so the parent could not message its worker at all.
1 parent 83c6eef commit 247b2ee

3 files changed

Lines changed: 209 additions & 20 deletions

File tree

‎docs/ARCHITECTURE.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ Workers ask the spawning parent with **`ask_director`** (not the human). That pa
255255

256256
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.
257257

258-
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.
258+
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.
259259

260260
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`).
261261

‎src/subagent/lifecycle-tools.test.ts‎

Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1287,6 +1287,186 @@ describe("send_input", () => {
12871287
resolution: "declined",
12881288
});
12891289
});
1290+
1291+
test("a resolution with no pending escalation is ignored and the message is delivered as plain input", async () => {
1292+
const sessions = createSubAgentSessionStore();
1293+
const fleetRecords = createFleetMailbox(sessions);
1294+
const sendInput = createSendInputTool({ sessions, fleetRecords });
1295+
const worker = startSession(sessions, { description: "worker" });
1296+
sessions.markRunning(worker.id);
1297+
const delivered: string[] = [];
1298+
sessions.registerDeliver(worker.id, (message) => {
1299+
delivered.push(message);
1300+
});
1301+
1302+
// Filler a field-filling model sent after approve_worker_call had
1303+
// already answered the worker's only ask.
1304+
const fillers = [
1305+
{ questionId: "", kind: "director_answer", answer: "" },
1306+
{
1307+
questionId: "ask-2",
1308+
kind: "director_answer",
1309+
answer: "Additional requirement applies after rebase.",
1310+
},
1311+
{
1312+
questionId: "none",
1313+
kind: "unavailable",
1314+
answer: "No escalation; normal task update only.",
1315+
},
1316+
];
1317+
for (const resolution of fillers) {
1318+
const result = await callTool(sendInput, {
1319+
target: worker.id,
1320+
message: "also require the PR checklist",
1321+
interrupt: false,
1322+
resolution,
1323+
});
1324+
expect(result).toEqual({
1325+
agent_id: worker.id,
1326+
status: "running",
1327+
resolution_ignored: "no pending escalation",
1328+
});
1329+
}
1330+
expect(delivered).toEqual(
1331+
fillers.map(() => "also require the PR checklist"),
1332+
);
1333+
});
1334+
1335+
test("an interrupt carrying a resolution with no pending escalation interrupts as a plain interrupt would", async () => {
1336+
let interrupted = false;
1337+
const followups: string[] = [];
1338+
const { sessions, fleetRecords, worker } = liveLane({
1339+
interrupt: () => {
1340+
interrupted = true;
1341+
},
1342+
followup: async (message) => {
1343+
followups.push(message);
1344+
return "queued turn finished";
1345+
},
1346+
deliver: () => {
1347+
throw new Error("interrupt:true should not soft-deliver");
1348+
},
1349+
});
1350+
const sendInput = createSendInputTool({ sessions, fleetRecords });
1351+
1352+
const result = await callTool(sendInput, {
1353+
target: worker.id,
1354+
message: "patch only the test",
1355+
interrupt: true,
1356+
resolution: {
1357+
questionId: "none",
1358+
kind: "unavailable",
1359+
answer: "No escalation; normal task update only.",
1360+
},
1361+
});
1362+
expect(result).toEqual({
1363+
agent_id: worker.id,
1364+
status: "interrupted",
1365+
resolution_ignored: "no pending escalation",
1366+
});
1367+
expect(interrupted).toBe(true);
1368+
sessions.attachReport(worker.id, "salvage", { stopReason: "interrupted" });
1369+
await new Promise((resolve) => setTimeout(resolve, 0));
1370+
expect(followups).toEqual(["patch only the test"]);
1371+
});
1372+
1373+
test("an interrupt carrying a resolution is refused while an ask is pending", async () => {
1374+
let interrupted = false;
1375+
const { sessions, fleetRecords, worker } = liveLane({
1376+
interrupt: () => {
1377+
interrupted = true;
1378+
},
1379+
followup: async () => "unused",
1380+
});
1381+
sessions.registerAsk(worker.id, {
1382+
question: "which file?",
1383+
questionId: "ask-1",
1384+
assessment: assessedAsk,
1385+
resolve: () => {
1386+
throw new Error("should not resolve");
1387+
},
1388+
reject: () => undefined,
1389+
});
1390+
const sendInput = createSendInputTool({ sessions, fleetRecords });
1391+
1392+
const result = await callFleetToolRaw(sendInput, {
1393+
target: worker.id,
1394+
message: "use branch X",
1395+
interrupt: true,
1396+
resolution: {
1397+
questionId: "ask-1",
1398+
kind: "director_answer",
1399+
answer: "use branch X",
1400+
},
1401+
});
1402+
expect(result.isError).toBe(true);
1403+
expect(result.content).toContain("cannot interrupt");
1404+
expect(interrupted).toBe(false);
1405+
expect(sessions.hasPendingAsk(worker.id)).toBe(true);
1406+
});
1407+
1408+
test("an interrupt carrying a malformed resolution while an ask is pending reports the interrupt conflict", async () => {
1409+
let interrupted = false;
1410+
const { sessions, fleetRecords, worker } = liveLane({
1411+
interrupt: () => {
1412+
interrupted = true;
1413+
},
1414+
followup: async () => "unused",
1415+
});
1416+
sessions.registerAsk(worker.id, {
1417+
question: "which file?",
1418+
questionId: "ask-1",
1419+
assessment: assessedAsk,
1420+
resolve: () => {
1421+
throw new Error("should not resolve");
1422+
},
1423+
reject: () => undefined,
1424+
});
1425+
const sendInput = createSendInputTool({ sessions, fleetRecords });
1426+
1427+
const result = await callFleetToolRaw(sendInput, {
1428+
target: worker.id,
1429+
message: "use branch X",
1430+
interrupt: true,
1431+
resolution: { questionId: "", kind: "not_a_kind", answer: "" },
1432+
});
1433+
expect(result.isError).toBe(true);
1434+
expect(result.content).toContain("cannot interrupt");
1435+
expect(result.content).not.toContain("resolution invalid");
1436+
expect(interrupted).toBe(false);
1437+
expect(sessions.hasPendingAsk(worker.id)).toBe(true);
1438+
});
1439+
1440+
test("an unusable resolution against a pending ask names the plain-message alternative", async () => {
1441+
const sessions = createSubAgentSessionStore();
1442+
const fleetRecords = createFleetMailbox(sessions);
1443+
const sendInput = createSendInputTool({ sessions, fleetRecords });
1444+
const worker = startSession(sessions, { description: "worker" });
1445+
sessions.markRunning(worker.id);
1446+
sessions.registerAsk(worker.id, {
1447+
question: "which file?",
1448+
questionId: "ask-1",
1449+
assessment: assessedAsk,
1450+
resolve: () => undefined,
1451+
reject: () => undefined,
1452+
});
1453+
1454+
for (const resolution of [
1455+
{ questionId: "", kind: "director_answer", answer: "" },
1456+
{ questionId: "ask-0", kind: "declined", answer: "No." },
1457+
]) {
1458+
const result = await callFleetToolRaw(sendInput, {
1459+
target: worker.id,
1460+
message: "No.",
1461+
resolution,
1462+
});
1463+
expect(result.isError).toBe(true);
1464+
expect(result.content).toContain(
1465+
"Omit resolution to answer with a plain message.",
1466+
);
1467+
}
1468+
expect(sessions.hasPendingAsk(worker.id)).toBe(true);
1469+
});
12901470
});
12911471

12921472
describe("nested lifecycle authority", () => {

‎src/subagent/lifecycle-tools.ts‎

Lines changed: 28 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,8 @@ export function createInterruptAgentTool(
448448
});
449449
}
450450

451+
const OMIT_RESOLUTION_HINT = "Omit resolution to answer with a plain message.";
452+
451453
const SendInputArgs = type({
452454
target: "string",
453455
message: "string",
@@ -458,7 +460,7 @@ const SendInputArgs = type({
458460
export const sendInputToolDefinition: ToolDefinition = {
459461
name: "send_input",
460462
description:
461-
"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.",
463+
"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.",
462464
inputSchema: {
463465
type: "object",
464466
properties: {
@@ -477,7 +479,7 @@ export const sendInputToolDefinition: ToolDefinition = {
477479
resolution: {
478480
type: "object",
479481
description:
480-
"Optional explicit pending-escalation resolution (questionId, kind, answer). It never grants or retries a tool. The message is delivered together with the structured answer.",
482+
"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.",
481483
properties: {
482484
questionId: {
483485
type: "string",
@@ -534,31 +536,32 @@ export function createSendInputTool(deps: LifecycleToolDeps): AgentTool {
534536
`(got ${message.length}).`,
535537
);
536538
}
537-
if (parsed.resolution !== undefined) {
538-
if (parsed.interrupt === true) {
539-
return lifecycleResult(
540-
call.id,
541-
"Error: structured escalation resolution cannot interrupt a worker.",
542-
);
543-
}
539+
// Field-filling models send a resolution on every call. With no ask
540+
// pending it has nothing to answer, so the message goes out as plain
541+
// input instead of failing on filler.
542+
const pending = deps.sessions.peekAsk(target);
543+
const answersAsk =
544+
parsed.resolution !== undefined && pending !== undefined;
545+
// Answering resumes the parked turn and an interrupt would stop it.
546+
// Checked before parsing so a malformed resolution still gets this error.
547+
if (answersAsk && parsed.interrupt === true) {
548+
return lifecycleResult(
549+
call.id,
550+
"Error: structured escalation resolution cannot interrupt a worker.",
551+
);
552+
}
553+
if (answersAsk) {
544554
const resolution = parseEscalationResolution(parsed.resolution);
545555
if (resolution instanceof Error) {
546556
return lifecycleResult(
547557
call.id,
548-
`Error: send_input resolution invalid: ${resolution.message}`,
549-
);
550-
}
551-
const pending = deps.sessions.peekAsk(target);
552-
if (pending === undefined) {
553-
return lifecycleResult(
554-
call.id,
555-
`Error: no pending escalation for "${target}".`,
558+
`Error: send_input resolution invalid: ${resolution.message} ${OMIT_RESOLUTION_HINT}`,
556559
);
557560
}
558561
if (pending.questionId !== resolution.questionId) {
559562
return lifecycleResult(
560563
call.id,
561-
`Error: send_input resolution questionId mismatch: pending ask is "${pending.questionId}", resolution names "${resolution.questionId}".`,
564+
`Error: send_input resolution questionId mismatch: pending ask is "${pending.questionId}", resolution names "${resolution.questionId}". ${OMIT_RESOLUTION_HINT}`,
562565
);
563566
}
564567
// SF6: deliver the parent's message together with the structured
@@ -629,7 +632,13 @@ export function createSendInputTool(deps: LifecycleToolDeps): AgentTool {
629632
}
630633
return lifecycleResult(
631634
call.id,
632-
fleetJson({ agent_id: target, status: outcome.status }),
635+
fleetJson({
636+
agent_id: target,
637+
status: outcome.status,
638+
...(parsed.resolution !== undefined
639+
? { resolution_ignored: "no pending escalation" }
640+
: {}),
641+
}),
633642
);
634643
},
635644
});

0 commit comments

Comments
 (0)