From ac2e46e1db5cdf225d177bd9953c16e8929ae171 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 11 Sep 2026 13:00:45 -0700 Subject: [PATCH 1/2] Salvage stub plan findings as an incomplete report A four-heading envelope with placeholder Findings would otherwise complete and look like an attachable plan. --- CHANGELOG.md | 8 + docs/ARCHITECTURE.md | 2 +- docs/PRODUCT.md | 2 +- src/agent/directors/counsel/package.test.ts | 1 + src/agent/directors/counsel/package.ts | 2 +- src/agent/directors/skywalker/package.test.ts | 3 + src/agent/directors/skywalker/package.ts | 2 +- src/prompts.test.ts | 3 +- src/subagent/agent-fleet.test.ts | 33 ++++ src/subagent/index.test.ts | 167 ++++++++++++++++++ src/subagent/index.ts | 2 + src/subagent/nudge-director.test.ts | 132 ++++++++++++++ src/subagent/nudge-director.ts | 38 +++- src/subagent/report.ts | 84 +++++++++ src/subagent/run.ts | 12 ++ src/subagent/stop-policy.ts | 15 ++ 16 files changed, 497 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c6303c3bb..c2ed2cabb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,12 +13,20 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename ## [Unreleased] +### Added + +- Plan and counsel workers require substance in Findings (files/paths, + acceptance criteria, non-goals, risks, ordered steps). Four headings + with stub Findings salvage as `incomplete-report`, not an attachable + plan. Implement and review envelope completeness is unchanged. + ### Fixed - System-originated inbound (`message.received` without the operator flag — background shell exits, mailbox mail, fleet-dry and compact continuations) paints as a visible system row instead of a user prompt. + ## [0.3.21] - 2026-09-11 ### Added diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index c79cf62a9..85adb6a83 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -120,7 +120,7 @@ Two directors, selected by role: Auto mode is toggled by CLI flags (`--auto` / `--no-auto`); there is currently no in-session key to toggle it (default on; constrained envelope — workspace writes and unconstrained shell auto-allow; installs, recursive rm, force/uncontained worktree changes, sensitive-path and opaque-wrapper shell still ask; contained non-force `git worktree add`/`remove`/`prune` and `list` auto-allow; shell file-mutation denied). It is not a separate edit/plan mode. -- **SubAgentDirector** (delegated work, `src/subagent/index.ts`) — Drives a dispatched worker until a turn arrives with no tool calls, then replies with the final assistant text and ends the run. A tool-less turn **after tools** completes only with the four-heading envelope (Summary, Findings, Blockers, Paths). Assistant text that prints explicit `` markup is treated as attempted tool use, not narration: one **verbatim-tool-call** nudge asks the worker to re-issue a real `tool_call` and does not count toward the tool-less spiral. A missing envelope otherwise nudges once (**incomplete-report**) and a second tool-less turn still without the envelope salvages as **incomplete-report-stop**. Explore/read-only workers that used tools then replied with findings remain normal completes; `requireEvidence` (off by default, set per director) additionally requires at least one read before a tool-less spawn-only reply can complete. Reads done through `run_shell` count as evidence too — `src/subagent/shell-evidence.ts` classifies shell reads (`cat`, `grep`, `sed` without `-i`, …) over the same subject expansion the auto-shell policy uses — but there is no corresponding shell-write evidence or file-write requirement: a run that never touches a file still completes normally once it replies with the envelope. There is no turn budget. Operator/parent cancel after any progress returns a **cancelled** salvage report (partial findings + tool activity) instead of a bare cancel string; cancel before progress still surfaces as cancelled-by-operator. There is no repetition/no-progress/never-acted/never-edited hard stop and no fingerprint-based re-dispatch block — a genuinely stuck worker runs until it completes, stalls, hits an opt-in wall-clock deadline, or is cancelled. +- **SubAgentDirector** (delegated work, `src/subagent/index.ts`) — Drives a dispatched worker until a turn arrives with no tool calls, then replies with the final assistant text and ends the run. A tool-less turn **after tools** completes only with the four-heading envelope (Summary, Findings, Blockers, Paths). Assistant text that prints explicit `` markup is treated as attempted tool use, not narration: one **verbatim-tool-call** nudge asks the worker to re-issue a real `tool_call` and does not count toward the tool-less spiral. A missing envelope otherwise nudges once (**incomplete-report**) and a second tool-less turn still without the envelope salvages as **incomplete-report-stop**. Explore/read-only workers that used tools then replied with findings remain normal completes; `requireEvidence` (off by default, set per director) additionally requires at least one read before a tool-less spawn-only reply can complete. `requirePlanSubstance` (counsel or `intent=plan`, not `modelRole === "plan"`) additionally requires Findings to contain files/paths, acceptance criteria, non-goals, risks, and ordered steps with a non-placeholder line each — four headings with stub Findings are incomplete-report, not an attachable plan. Reads done through `run_shell` count as evidence too — `src/subagent/shell-evidence.ts` classifies shell reads (`cat`, `grep`, `sed` without `-i`, …) over the same subject expansion the auto-shell policy uses — but there is no corresponding shell-write evidence or file-write requirement: a run that never touches a file still completes normally once it replies with the envelope. There is no turn budget. Operator/parent cancel after any progress returns a **cancelled** salvage report (partial findings + tool activity) instead of a bare cancel string; cancel before progress still surfaces as cancelled-by-operator. There is no repetition/no-progress/never-acted/never-edited hard stop and no fingerprint-based re-dispatch block — a genuinely stuck worker runs until it completes, stalls, hits an opt-in wall-clock deadline, or is cancelled. `spawn_agent` starts each worker and records it in the caller's fleet mailbox. On the TUI primary, mailbox mail is the collect path: occupancy takes uncollected terminals and re-enters the parent as system inbound. Nested orchestrators still collect with `wait_agents`. TUI-primary `wait_agents` may yield as a timeout (workers untouched, no take) so occupancy can deliver mail or a queued Enter steer. Already-collected waits return status without a second report or error body. Wait JSON includes `stop_reason` from the session when present so a salvage that is wait-`done` is not mistaken for a clean complete, and so parent-initiated interrupt (`interrupted`) is not mistaken for operator-cancel (`cancelled`). Deadline salvage prepends an advisory parent hint suggesting continuation plus a longer deadline if more wall-clock time is warranted. Failed and incomplete-report salvage tell the parent to diagnose from the report or error and MAY spawn one successor with a changed brief. A parent-initiated interrupt is a resumable pause: wait unblocks with `stop_reason: interrupted` (often while the session is still running and has no report); the parent should `resume_agent` or re-wait, and must not spawn a successor against a still-live worker. Successor only if that session is no longer resumable. Operator-cancelled salvage asks the parent to synthesize Findings and Paths and wait for the operator instead of auto-starting another specialist. Identical re-dispatch of the same brief stays refused at the prompt / spawn-handoff layer; there is no fingerprint-based re-dispatch hard-block. Deadline hints are advisory only — an identical re-dispatch is still admitted at runtime. Parent hints are prepended on salvage reports returned to the parent. The runtime does not auto-spawn successors. #### Model-family policy (`src/agent/model-family-policy.ts`) diff --git a/docs/PRODUCT.md b/docs/PRODUCT.md index 276a0b9b4..7597b503f 100644 --- a/docs/PRODUCT.md +++ b/docs/PRODUCT.md @@ -172,7 +172,7 @@ Corbits Code fans work out to short-lived **fleet agents** — workers with thei - **Tasks** are checklist items owned by one agent via `manage_tasks`. - **Fleet agents** are spawned with `spawn_agent`. On the TUI primary, mailbox mail arrives as inbound when a worker finishes or fails — do not poll `wait_agents`. Nested orchestrators still collect with `wait_agents`. Workers ask the parent with `ask_director`. That parks a question while the worker stays `running`. Nested `wait_agents` returns `awaiting_director` with a question payload — that is not terminal. The parent answers with `send_input` (`target` = the worker's session id). When the parent TUI is not blocked in `wait_agents`, a parked question arrives as a synthetic idle-send wake. Escalate to the human only with `ask_operator`. -Dispatch uses a structured brief (context / goal / optional goals seed) and returns a structured report. The TUI Agents strip and fleet board show who is running; live tool progress updates the status bar without dumping the child transcript into the parent chat. There is no turn budget. A tool-less final turn completes only with the four-heading report envelope. Printed `` markup in assistant text gets one corrective nudge to issue a real tool call and does not count as the wrap-up; without the envelope, one incomplete-report nudge is given and a second tool-less turn without the envelope salvages as `incomplete-report-stop`. A silent worker (no activity for `stallTimeoutMs`, opt-in) gets one continuation nudge, then salvages as `stalled` only after a full `stallTimeoutMs` grace with still no activity — queued checks inside that window wait, they do not salvage. An opt-in `deadlineMs`, or an operator cancel, can also end a run early. Each of these returns a salvage report so a runaway or idle child cannot quietly burn a large token budget or look done after prose alone. +Dispatch uses a structured brief (context / goal / optional goals seed) and returns a structured report. The TUI Agents strip and fleet board show who is running; live tool progress updates the status bar without dumping the child transcript into the parent chat. There is no turn budget. A tool-less final turn completes only with the four-heading report envelope. Printed `` markup in assistant text gets one corrective nudge to issue a real tool call and does not count as the wrap-up; without the envelope, one incomplete-report nudge is given and a second tool-less turn without the envelope salvages as `incomplete-report-stop`. Counsel and `intent=plan` also require plan substance in Findings (files/paths, acceptance criteria, non-goals, risks, ordered steps); headings-only or stub Findings salvage as `incomplete-report`, not an attachable plan. A silent worker (no activity for `stallTimeoutMs`, opt-in) gets one continuation nudge, then salvages as `stalled` only after a full `stallTimeoutMs` grace with still no activity — queued checks inside that window wait, they do not salvage. An opt-in `deadlineMs`, or an operator cancel, can also end a run early. Each of these returns a salvage report so a runaway or idle child cannot quietly burn a large token budget or look done after prose alone. ## Roadmap (planned, not yet shipped) diff --git a/src/agent/directors/counsel/package.test.ts b/src/agent/directors/counsel/package.test.ts index 2dd0a0ac9..8c05b6240 100644 --- a/src/agent/directors/counsel/package.test.ts +++ b/src/agent/directors/counsel/package.test.ts @@ -60,6 +60,7 @@ describe("counselPackage", () => { expect(p).toContain("success_criteria"); expect(p).toMatch(/[Ss]top when/); expect(p).toContain("Blockers"); + expect(p).toContain("Headings-only Findings is not done"); }); test("spawn.maySpawn is false", () => { diff --git a/src/agent/directors/counsel/package.ts b/src/agent/directors/counsel/package.ts index ac7fd07eb..0f8758408 100644 --- a/src/agent/directors/counsel/package.ts +++ b/src/agent/directors/counsel/package.ts @@ -36,7 +36,7 @@ Author an agent-proof plan: When requirements are fuzzy, ask_director instead of guessing — after the cap, note remaining questions under Blockers. Do not invent scope. -DONE GATE: Stop when the plan covers every success_criteria item from the brief OR blockers are explicit. Do not expand into implementation, architecture essays, or review theater after the plan is complete. +DONE GATE: Stop when the plan covers every success_criteria item from the brief OR blockers are explicit. Headings-only Findings is not done. Do not expand into implementation, architecture essays, or review theater after the plan is complete. OUT OF LANE: shipping code, architecture gate sign-off, fleet orchestration, pure code review, becoming Builder/Critic/Greybeard/Explorer as primary. diff --git a/src/agent/directors/skywalker/package.test.ts b/src/agent/directors/skywalker/package.test.ts index 4f80b5e01..14fe8532c 100644 --- a/src/agent/directors/skywalker/package.test.ts +++ b/src/agent/directors/skywalker/package.test.ts @@ -156,6 +156,9 @@ describe("skywalkerPackage", () => { expect(p).toContain("Do not reclassify COMMUNICATION as ORCHESTRATION"); expect(p).toContain("synthesize what returned"); expect(p).toContain("do **not** re-fan-out another diagnostic wave"); + expect(p).toContain( + "`incomplete-report` from plan/counsel is not an attachable plan", + ); expect(p).not.toContain("Then start the next worker"); expect(p).not.toContain("if the job still needs doing"); }); diff --git a/src/agent/directors/skywalker/package.ts b/src/agent/directors/skywalker/package.ts index 3d06c85bf..f43d35c28 100644 --- a/src/agent/directors/skywalker/package.ts +++ b/src/agent/directors/skywalker/package.ts @@ -76,7 +76,7 @@ Do **not** turn a "why is this stalled / why no thinking / spawn looks broken" d - Answer from mounted tools + known architecture; at most **one** explorer worker if a single unknown path blocks the answer. - Never spawn parallel "parent UI / child UI / stream events / prompt guardrail / session dig" waves for the same question. - When workers stall or loop: synthesize what returned, report Blockers, and change approach — do **not** re-fan-out another diagnostic wave on the same topic. -- Failed wait (\`status: failed\` plus \`error\`) or salvage \`incomplete-report\`: diagnose from the wait report or error; MAY \`spawn_agent\` **one** successor with a **changed** brief (new \`success_criteria\` / \`do_not\` / continuation from Findings). Cap is one successor for that stall. Spawn the successor — do not search the repo as a substitute. +- Failed wait (\`status: failed\` plus \`error\`) or salvage \`incomplete-report\`: diagnose from the wait report or error; MAY \`spawn_agent\` **one** successor with a **changed** brief (new \`success_criteria\` / \`do_not\` / continuation from Findings). Cap is one successor for that stall. Spawn the successor — do not search the repo as a substitute. \`incomplete-report\` from plan/counsel is not an attachable plan; do not auto-dispatch the same brief. - Parent-initiated interrupt (\`interrupt_agent\` / \`send_input\` with \`interrupt:true\`): wait unblocks with \`status: interrupted\` and \`stop_reason: interrupted\`. That is a resumable pause, not fail or incomplete-report. The worker is often still running and often has no report. Call \`resume_agent\` (changed follow-up into retained context) or re-wait. Do **not** \`spawn_agent\` a successor against a still-live worker. Successor only if the session is no longer resumable. - Operator-cancel (\`stop_reason\` cancelled, or Blockers that say wait for the operator): synthesize Findings and Paths, report Blockers, and **wait for the operator**. Do not auto-retry. Do not spawn a successor because the worker was cancelled. - Do **not** search the repo yourself after a worker stops without finishing. diff --git a/src/prompts.test.ts b/src/prompts.test.ts index 83313fe15..9cba2ca2f 100644 --- a/src/prompts.test.ts +++ b/src/prompts.test.ts @@ -6,7 +6,7 @@ import { } from "./agent/director.js"; import { manageTasksDefinition } from "./agent/tasks.js"; import { CHAT_PROMPT_QUALITY_MARKERS } from "./agent/prompt-contract.js"; -import { hasReportEnvelope } from "./subagent/report.js"; +import { hasPlanFindings, hasReportEnvelope } from "./subagent/report.js"; import { buildActiveContext, buildAvailableTools, @@ -424,6 +424,7 @@ test("sub-agent report contract's headings satisfy hasReportEnvelope", () => { .filter((line) => line.startsWith("## ")) .join("\n"); expect(hasReportEnvelope(headingsOnly)).toBe(true); + expect(hasPlanFindings(headingsOnly)).toBe(false); }); test("sub-agent prompt does not advertise tool_search (it gets the full toolset)", () => { diff --git a/src/subagent/agent-fleet.test.ts b/src/subagent/agent-fleet.test.ts index e440ba8ca..21a2a1573 100644 --- a/src/subagent/agent-fleet.test.ts +++ b/src/subagent/agent-fleet.test.ts @@ -570,6 +570,39 @@ describe("spawn_agent + wait_agents", () => { expect(defined(results[0]).error).toBeUndefined(); }); + test("plan-lane incomplete-report salvage is wait done with stop_reason, not a clean complete", async () => { + const deps = makeDeps(async () => ({ + report: forcedStopReport( + "incomplete-report", + "Stub plan Findings (missing files/paths, acceptance criteria, non-goals, risks, or ordered steps). This is not an attachable plan.\n\nPlan ready.", + ), + stopReason: "incomplete-report", + })); + const spawn = createSpawnAgentTool(deps); + const wait = createWaitAgentsTool({ + sessions: deps.sessions, + fleetRecords: deps.fleetRecords, + }); + + const spawned = await callTool(spawn, { + description: "stub plan", + prompt: "outline it", + intent: "plan", + }); + const id = spawned.agent_id as string; + const waited = await callTool(wait, { targets: [id], timeout_ms: 5000 }); + const results = waited.results as { + status: string; + report?: string; + error?: string; + stop_reason?: string; + }[]; + expect(defined(results[0]).status).toBe("done"); + expect(defined(results[0]).stop_reason).toBe("incomplete-report"); + expect(defined(results[0]).report).toContain("not an attachable plan"); + expect(defined(results[0]).error).toBeUndefined(); + }); + test("failed spawn_agent wait_agents returns error not report", async () => { const deps = makeDeps(async () => { throw new Error("provider blew up"); diff --git a/src/subagent/index.test.ts b/src/subagent/index.test.ts index 9bd08ccf0..47ca14ec5 100644 --- a/src/subagent/index.test.ts +++ b/src/subagent/index.test.ts @@ -19,7 +19,10 @@ import { preferCompletedSubAgentReply, resolveSubAgentCatchOutcome, resolveSubAgentDeadlineMs, + hasReportEnvelope, + hasPlanFindings, shouldRequireEvidence, + shouldRequirePlanSubstance, subAgentToolName, SUBAGENT_DEADLINE_MARGIN_MS, SUBAGENT_PLUGIN_SPAWN_TEARDOWN_LIMITS, @@ -212,6 +215,102 @@ describe("sub-agent stop helpers", () => { "src/gate.ts", ].join("\n"); + const HEADINGS_ONLY_ENVELOPE = [ + "## Summary", + "", + "## Findings", + "", + "## Blockers", + "", + "## Paths", + ].join("\n"); + + const STUB_PLAN_ENVELOPE = [ + "## Summary", + "Plan ready.", + "", + "## Findings", + "None.", + "", + "## Blockers", + "None.", + "", + "## Paths", + "None.", + ].join("\n"); + + const NUMBERED_TBD_PLAN_FINDINGS = [ + "1. Files / paths", + " TBD", + "2. Acceptance criteria", + " TBD", + "3. Non-goals", + " TBD", + "4. Risks", + " TBD", + "5. Ordered steps", + " TBD", + ].join("\n"); + + const NUMBERED_TBD_PLAN_ENVELOPE = [ + "## Summary", + "Outline.", + "", + "## Findings", + NUMBERED_TBD_PLAN_FINDINGS, + "", + "## Blockers", + "None.", + "", + "## Paths", + "None.", + ].join("\n"); + + const OUTLINE_ONLY_PLAN_ENVELOPE = [ + "## Summary", + "Outline.", + "", + "## Findings", + "Files / paths, acceptance criteria, non-goals, risks, ordered steps.", + "", + "## Blockers", + "None.", + "", + "## Paths", + "None.", + ].join("\n"); + + const PASS_PLAN_FINDINGS = [ + "### Files / paths", + "src/subagent/report.ts", + "", + "### Acceptance criteria", + "Stub plan Findings salvage as incomplete-report.", + "", + "### Non-goals", + "Do not finish CL-6946.", + "", + "### Risks", + "A headings-only complete would auto-dispatch builder on a stub.", + "", + "### Ordered steps", + "Add hasPlanFindings, then wire evaluateSubAgentStop.", + ].join("\n"); + + const PASS_PLAN_ENVELOPE = [ + "## Summary", + "Plan for the salvage gate.", + "", + "## Findings", + PASS_PLAN_FINDINGS, + "", + "## Blockers", + "None.", + "", + "## Paths", + "src/subagent/report.ts", + ].join("\n"); + test("evaluateSubAgentStop returns incomplete-report for Summary-only tool-less narration after tools", () => { expect( evaluateSubAgentStop({ @@ -333,6 +432,74 @@ describe("sub-agent stop helpers", () => { ).toBe("complete"); }); + test("hasPlanFindings is false for empty Findings, None, numbered TBD titles, and outline-only", () => { + expect(hasPlanFindings(HEADINGS_ONLY_ENVELOPE)).toBe(false); + expect(hasPlanFindings(STUB_PLAN_ENVELOPE)).toBe(false); + expect(hasPlanFindings(NUMBERED_TBD_PLAN_ENVELOPE)).toBe(false); + expect(hasPlanFindings(OUTLINE_ONLY_PLAN_ENVELOPE)).toBe(false); + }); + + test("hasPlanFindings is true when Findings has the five labeled plan sections with substance", () => { + expect(hasPlanFindings(PASS_PLAN_ENVELOPE)).toBe(true); + }); + + test("hasReportEnvelope stays heading-presence only on headings-only text", () => { + expect(hasReportEnvelope(HEADINGS_ONLY_ENVELOPE)).toBe(true); + expect(hasPlanFindings(HEADINGS_ONLY_ENVELOPE)).toBe(false); + }); + + test("shouldRequirePlanSubstance is armed for plan intent or counsel, not other directors", () => { + expect(shouldRequirePlanSubstance({ directorId: "counsel" })).toBe(true); + expect(shouldRequirePlanSubstance({ intent: "plan" })).toBe(true); + expect( + shouldRequirePlanSubstance({ intent: "plan", directorId: "counsel" }), + ).toBe(true); + expect(shouldRequirePlanSubstance({ directorId: "critic" })).toBe(false); + expect(shouldRequirePlanSubstance({ directorId: "greybeard" })).toBe(false); + expect(shouldRequirePlanSubstance({ directorId: "builder" })).toBe(false); + expect(shouldRequirePlanSubstance({ directorId: "gaasbot" })).toBe(false); + expect(shouldRequirePlanSubstance({ intent: "implement" })).toBe(false); + expect(shouldRequirePlanSubstance({ intent: "review" })).toBe(false); + expect(shouldRequirePlanSubstance({})).toBe(false); + }); + + test("evaluateSubAgentStop salvages stub plan Findings when requirePlanSubstance is on", () => { + expect( + evaluateSubAgentStop({ + hasToolCalls: false, + requirePlanSubstance: true, + lastAssistantText: STUB_PLAN_ENVELOPE, + }), + ).toBe("incomplete-report"); + expect( + evaluateSubAgentStop({ + hasToolCalls: false, + requirePlanSubstance: true, + lastAssistantText: STUB_PLAN_ENVELOPE, + toolLessNarrationCycles: 2, + }), + ).toBe("incomplete-report-stop"); + }); + + test("evaluateSubAgentStop still completes the same stub when requirePlanSubstance is omitted", () => { + expect( + evaluateSubAgentStop({ + hasToolCalls: false, + lastAssistantText: STUB_PLAN_ENVELOPE, + }), + ).toBe("complete"); + }); + + test("evaluateSubAgentStop completes a pass plan fixture on the plan lane", () => { + expect( + evaluateSubAgentStop({ + hasToolCalls: false, + requirePlanSubstance: true, + lastAssistantText: PASS_PLAN_ENVELOPE, + }), + ).toBe("complete"); + }); + test("evaluateSubAgentStop does not stop for many unique reads while still calling tools", () => { let thrash = EMPTY_THRASH_STATE; for (let i = 0; i < 200; i++) { diff --git a/src/subagent/index.ts b/src/subagent/index.ts index 8e21f4e16..f56a435d9 100644 --- a/src/subagent/index.ts +++ b/src/subagent/index.ts @@ -45,6 +45,7 @@ export { buildDispatchBrief, demoteNestedReportHeadings, formatSubAgentReport, + hasPlanFindings, hasReportEnvelope, parseSubAgentReport, subAgentToolName, @@ -99,6 +100,7 @@ export { createSubAgentRunController, runSubAgent, shouldRequireEvidence, + shouldRequirePlanSubstance, type SubAgentRunController, } from "./run.js"; diff --git a/src/subagent/nudge-director.test.ts b/src/subagent/nudge-director.test.ts index 81de176d9..27647b516 100644 --- a/src/subagent/nudge-director.test.ts +++ b/src/subagent/nudge-director.test.ts @@ -838,6 +838,137 @@ describe("SubAgentDirector incomplete-report wiring", () => { }); }); +const STUB_PLAN_ENVELOPE = [ + "## Summary", + "Plan ready.", + "", + "## Findings", + "None.", + "", + "## Blockers", + "None.", + "", + "## Paths", + "None.", +].join("\n"); + +const PASS_PLAN_ENVELOPE = [ + "## Summary", + "Plan for the salvage gate.", + "", + "## Findings", + "### Files / paths", + "src/subagent/report.ts", + "", + "### Acceptance criteria", + "Stub plan Findings salvage as incomplete-report.", + "", + "### Non-goals", + "Do not finish CL-6946.", + "", + "### Risks", + "A headings-only complete would auto-dispatch builder on a stub.", + "", + "### Ordered steps", + "Add hasPlanFindings, then wire evaluateSubAgentStop.", + "", + "## Blockers", + "None.", + "", + "## Paths", + "src/subagent/report.ts", +].join("\n"); + +describe("SubAgentDirector plan-substance wiring", () => { + test("stub plan Findings with requirePlanSubstance nudges for the five parts, not four headings", async () => { + const director = new SubAgentDirector( + "system", + [], + undefined, + 30, + Date.now, + false, + true, + ); + const caps = capabilities(); + + const result = actions( + await director.decide(inferenceDoneText(STUB_PLAN_ENVELOPE), state, caps), + ); + expect(result).toContainEqual({ + type: "checkpoint", + message: "subagent-incomplete-report-nudge", + }); + expect(result.some((action) => action.type === "reply")).toBe(false); + const texts = ephemeralTexts(inferAction(result)); + expect(texts).toHaveLength(1); + expect(texts?.[0]).toContain("files/paths"); + expect(texts?.[0]).toContain("acceptance criteria"); + expect(texts?.[0]).toContain("non-goals"); + expect(texts?.[0]).toContain("risks"); + expect(texts?.[0]).toContain("ordered steps"); + expect(texts?.[0]).not.toContain( + "Write your final report now using ## Summary", + ); + }); + + test("second stub plan turn salvages incomplete-report with a stub-plan Findings prefix", async () => { + const director = new SubAgentDirector( + "system", + [], + undefined, + 30, + Date.now, + false, + true, + ); + const caps = capabilities(); + + await director.decide(inferenceDoneText(STUB_PLAN_ENVELOPE), state, caps); + + const result = actions( + await director.decide(inferenceDoneText(STUB_PLAN_ENVELOPE), state, caps), + ); + expect(result).toContainEqual({ + type: "checkpoint", + message: "subagent-incomplete-report", + }); + const reply = result.find((action) => action.type === "reply"); + expect(reply).toBeDefined(); + if (reply === undefined || reply.type !== "reply") + throw new Error("expected reply action"); + expect(reply.content).toContain("not an attachable plan"); + expect(reply.content).toContain("Plan ready."); + expect(reply.content).toContain("## Summary"); + }); + + test("pass plan fixture with requirePlanSubstance completes", async () => { + const director = new SubAgentDirector( + "system", + [], + undefined, + 30, + Date.now, + false, + true, + ); + const caps = capabilities(); + + const result = actions( + await director.decide(inferenceDoneText(PASS_PLAN_ENVELOPE), state, caps), + ); + expect(result).toContainEqual({ + type: "checkpoint", + message: "subagent-complete", + }); + const reply = result.find((action) => action.type === "reply"); + expect(reply).toBeDefined(); + if (reply === undefined || reply.type !== "reply") + throw new Error("expected reply action"); + expect(reply.content).toBe(PASS_PLAN_ENVELOPE); + }); +}); + describe("SubAgentDirector post-complete terminalization (CL-7068)", () => { test("empty continuation after a valid report reply waits instead of re-inferring", async () => { const director = new SubAgentDirector("system", [], undefined, 1000); @@ -1154,6 +1285,7 @@ describe("SubAgentDirector infer retryPolicy", () => { 30, Date.now, false, + false, retryPolicy, ); const infer = inferAction( diff --git a/src/subagent/nudge-director.ts b/src/subagent/nudge-director.ts index 573711ffb..0a4846354 100644 --- a/src/subagent/nudge-director.ts +++ b/src/subagent/nudge-director.ts @@ -38,6 +38,7 @@ import { lastText, type ForcedStopReason, } from "./stop-policy.js"; +import { hasPlanFindings, hasReportEnvelope } from "./report.js"; const TOOL_FAILURE_RECOVERY_NUDGE = "A tool call failed. Do not repeat the same failed call unchanged. Inspect the error and current state, then change the arguments or approach. If you cannot recover, report the blocker."; @@ -46,6 +47,12 @@ const TOOL_FAILURE_RECOVERY_NUDGE = const INCOMPLETE_REPORT_NUDGE = "Write your final report now using ## Summary, ## Findings, ## Blockers, and ## Paths. Do not narrate status. No more tools unless one lookup is required to cite a line."; +const PLAN_SUBSTANCE_NUDGE = + "Findings is not an attachable plan. Fill files/paths, acceptance criteria, non-goals, risks, and ordered steps — each with a concrete non-placeholder line. Do not rewrite the four report headings."; + +const STUB_PLAN_SALVAGE_PREFIX = + "Stub plan Findings (missing files/paths, acceptance criteria, non-goals, risks, or ordered steps). This is not an attachable plan.\n\n"; + const VERBATIM_TOOL_CALL_NUDGE = "You wrote tool-call markup as assistant text. Invoke the real tool call instead of printing its markup, or write your final report if no tool is needed."; @@ -125,6 +132,8 @@ export class SubAgentDirector extends DefaultDirector { private readonly _systemPrompt: string; /** When true (CritiqueDirector), empty readCounts is not a successful complete. */ private readonly requireEvidence: boolean; + /** When true (counsel / intent=plan), stub plan Findings is not a complete. */ + private readonly requirePlanSubstance: boolean; private turnsCompleted = 0; private thrashState: ThrashState = EMPTY_THRASH_STATE; // Armed for failed-tool recovery so the @@ -223,6 +232,7 @@ export class SubAgentDirector extends DefaultDirector { stallTimeoutMs?: number, now: () => number = Date.now, requireEvidence = false, + requirePlanSubstance = false, retryPolicy: RetryPolicy = createCorbitsRetryPolicy(), ) { super(systemPrompt, toolDefinitions, {}); @@ -236,6 +246,7 @@ export class SubAgentDirector extends DefaultDirector { this.now = now; this.lastActivityAt = now(); this.requireEvidence = requireEvidence; + this.requirePlanSubstance = requirePlanSubstance; this.retryPolicy = retryPolicy; } @@ -327,6 +338,7 @@ export class SubAgentDirector extends DefaultDirector { hasToolCalls, thrashState: this.thrashState, requireEvidence: this.requireEvidence, + requirePlanSubstance: this.requirePlanSubstance, lastAssistantText: this.lastAssistantText, toolLessNarrationCycles: this.toolLessNarrationCycles + 1, }); @@ -372,32 +384,50 @@ export class SubAgentDirector extends DefaultDirector { // Tool-less turn after tools, no report envelope. Must not fall through // to super.decide — DefaultDirector completes any tool-less turn. this.toolLessNarrationCycles += 1; + const stubPlan = + this.requirePlanSubstance && + hasReportEnvelope(this.lastAssistantText) && + !hasPlanFindings(this.lastAssistantText); this.interventions({ id: "incomplete-report", class: "nudge", state: this.interventionState(), - detail: "tool-less turn after tools with no report envelope", + detail: stubPlan + ? "tool-less turn with stub plan Findings" + : "tool-less turn after tools with no report envelope", }); return [ capabilities.checkpoint("subagent-incomplete-report-nudge"), - inferWithSubAgentNudge(capabilities, INCOMPLETE_REPORT_NUDGE), + inferWithSubAgentNudge( + capabilities, + stubPlan ? PLAN_SUBSTANCE_NUDGE : INCOMPLETE_REPORT_NUDGE, + ), ]; } if (stop === "incomplete-report-stop") { this.toolLessNarrationCycles += 1; + const stubPlan = + this.requirePlanSubstance && + hasReportEnvelope(this.lastAssistantText) && + !hasPlanFindings(this.lastAssistantText); this.interventions({ id: "incomplete-report-stop", class: "stop", state: this.interventionState(), - detail: "no report envelope after the wrap-up nudge", + detail: stubPlan + ? "stub plan Findings after the wrap-up nudge" + : "no report envelope after the wrap-up nudge", }); this.onForcedStop("incomplete-report"); this.reportReplied = true; this.flushToolFailureRecoveryAudit(); + const salvageText = stubPlan + ? `${STUB_PLAN_SALVAGE_PREFIX}${this.lastAssistantText}` + : this.lastAssistantText; const terminal: ReactorAction[] = [ capabilities.checkpoint("subagent-incomplete-report"), capabilities.reply( - forcedStopReport("incomplete-report", this.lastAssistantText, { + forcedStopReport("incomplete-report", salvageText, { paths: salvagePathsFromThrash(this.thrashState), }), ), diff --git a/src/subagent/report.ts b/src/subagent/report.ts index 53e9445ac..88ddadbca 100644 --- a/src/subagent/report.ts +++ b/src/subagent/report.ts @@ -128,6 +128,90 @@ export function hasReportEnvelope(text: string): boolean { ); } +const PLAN_FINDING_SECTIONS = [ + { id: "files", match: /files?\s*(?:\/|and|&)?\s*paths?/i }, + { id: "ac", match: /acceptance\s+criteria|\bAC\b/i }, + { id: "nonGoals", match: /non-?goals?/i }, + { id: "risks", match: /\brisks?\b/i }, + { id: "steps", match: /ordered\s+steps|\bsteps\b/i }, +] as const; + +type PlanFindingSectionId = (typeof PLAN_FINDING_SECTIONS)[number]["id"]; + +const PLACEHOLDER_LINE = /^(none\.?|tbd\.?|n\/a\.?|n\.a\.?|\.{2,}|…|-)$/i; + +function stripPlanSectionPrefix(line: string): string { + return line + .replace(/^#{1,6}\s+/, "") + .replace(/^\d+[.)]\s+/, "") + .replace(/^\*\*(.+?)\*\*:?\s*/, "$1 ") + .trim(); +} + +function classifyPlanSectionLine( + line: string, +): { id: PlanFindingSectionId; rest: string } | null { + const stripped = stripPlanSectionPrefix(line); + if (stripped.length === 0) return null; + for (const section of PLAN_FINDING_SECTIONS) { + const match = section.match.exec(stripped); + if (match === null || match.index === undefined) continue; + // Only treat a line as a section start when the label is the line's heading, + // not a later mention in an outline sentence. + if (match.index > 0 && /[,;]/.test(stripped.slice(0, match.index))) { + continue; + } + const after = stripped.slice(match.index + match[0].length); + const separated = after.match(/^\s*[:.\-–—]\s+(\S.*)$/); + return { id: section.id, rest: separated?.[1] ?? "" }; + } + return null; +} + +function isPlaceholderPlanLine(line: string): boolean { + const t = line.replace(/^[-*•]\s+/, "").trim(); + if (t.length === 0) return true; + return PLACEHOLDER_LINE.test(t); +} + +function planSectionHasSubstance(body: string): boolean { + return body.split(/\r?\n/).some((line) => !isPlaceholderPlanLine(line)); +} + +/** + * True iff Findings contains labeled files/paths, acceptance criteria, + * non-goals, risks, and ordered steps, each with a non-placeholder body line. + * Heading presence is `hasReportEnvelope`; this is the plan-lane substance gate. + */ +export function hasPlanFindings(text: string): boolean { + const findings = parseSubAgentReport(text).findings; + if (findings.length === 0) return false; + const lines = findings.split(/\r?\n/); + const starts: { index: number; id: PlanFindingSectionId; rest: string }[] = + []; + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + if (line === undefined) continue; + const classified = classifyPlanSectionLine(line); + if (classified === null) continue; + starts.push({ index: i, ...classified }); + } + const bodies = new Map(); + for (let s = 0; s < starts.length; s++) { + const start = starts[s]; + if (start === undefined) continue; + if (bodies.has(start.id)) continue; + const next = starts[s + 1]; + const end = next?.index ?? lines.length; + const parts = [start.rest, ...lines.slice(start.index + 1, end)]; + bodies.set(start.id, parts.join("\n")); + } + return PLAN_FINDING_SECTIONS.every((section) => { + const body = bodies.get(section.id); + return body !== undefined && planSectionHasSubstance(body); + }); +} + /** Demote ## Summary|Findings|Blockers|Paths lines so nested envelopes stay under Findings. */ export function demoteNestedReportHeadings(text: string): string { // Match parseSubAgentReport: flexible whitespace + case-insensitive section names. diff --git a/src/subagent/run.ts b/src/subagent/run.ts index c0c7a488e..d4de7e155 100644 --- a/src/subagent/run.ts +++ b/src/subagent/run.ts @@ -439,6 +439,17 @@ export function shouldRequireEvidence(input: { return input.directorId === "critic"; } +/** + * Arm plan-substance Findings on counsel or intent=plan. Do not key off + * modelRole === "plan" — gaasbot shares that role and is not a plan author. + */ +export function shouldRequirePlanSubstance(input: { + intent?: TaskIntent; + directorId?: string; +}): boolean { + return input.intent === "plan" || input.directorId === "counsel"; +} + const submitResultDefinition: ToolDefinition = { name: "submit_result", description: @@ -971,6 +982,7 @@ async function runSubAgentInner( modelFamilyPolicy.subAgentStallTimeoutMs, Date.now, shouldRequireEvidence(params), + shouldRequirePlanSubstance(params), createCorbitsRetryPolicy({ providerId: params.provider.providerName, admission: params.admission ?? getProcessAdmissionQueue(), diff --git a/src/subagent/stop-policy.ts b/src/subagent/stop-policy.ts index 4896af933..728cee101 100644 --- a/src/subagent/stop-policy.ts +++ b/src/subagent/stop-policy.ts @@ -9,6 +9,7 @@ import { onTurnBoundary } from "../agent/reactor-events.js"; import { demoteNestedReportHeadings, formatSubAgentReport, + hasPlanFindings, hasReportEnvelope, } from "./report.js"; import type { ThrashState } from "./thrash.js"; @@ -114,6 +115,8 @@ export function evaluateToolLessNarrationSpiral( * When `requireEvidence` is set (CritiqueDirector), an empty `readCounts` * is not complete even with all four headings — same incomplete-report * nudge then salvage, so a wrap-up envelope cannot fake a real review. + * When `requirePlanSubstance` is set (counsel / intent=plan), four headings + * with stub Findings are the same spiral — not a finished plan. */ export function evaluateSubAgentStop(input: { hasToolCalls: boolean; @@ -124,6 +127,12 @@ export function evaluateSubAgentStop(input: { * narration as a finished review. */ requireEvidence?: boolean; + /** + * When true (counsel / intent=plan), a four-heading envelope whose Findings + * lack files/paths, acceptance criteria, non-goals, risks, and ordered steps + * is incomplete-report — not a finished plan. + */ + requirePlanSubstance?: boolean; /** Read/search bookkeeping for the evidence gate above. */ thrashState?: ThrashState; /** @@ -164,6 +173,12 @@ export function evaluateSubAgentStop(input: { ) { return spiralStop(); } + if ( + input.requirePlanSubstance === true && + !hasPlanFindings(input.lastAssistantText) + ) { + return spiralStop(); + } return "complete"; } return null; From 68ea6e0ebd7ab6e914a256718c8846f7761c9d59 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 11 Sep 2026 13:13:04 -0700 Subject: [PATCH 2/2] Match plan section labels to heading starts only Body prose that mentioned steps or risks was treated as a section start, so a five-section plan could lose its real Ordered steps heading and salvage as incomplete. --- src/subagent/index.test.ts | 119 +++++++++++++++++++++++++++++++++++++ src/subagent/report.ts | 10 +--- 2 files changed, 122 insertions(+), 7 deletions(-) diff --git a/src/subagent/index.test.ts b/src/subagent/index.test.ts index 47ca14ec5..a24a1cdfa 100644 --- a/src/subagent/index.test.ts +++ b/src/subagent/index.test.ts @@ -311,6 +311,83 @@ describe("sub-agent stop helpers", () => { "src/subagent/report.ts", ].join("\n"); + const STEPS_IN_AC_BODY_PLAN_ENVELOPE = [ + "## Summary", + "Plan for the salvage gate.", + "", + "## Findings", + "### Files / paths", + "src/subagent/report.ts", + "", + "### Acceptance criteria", + "The worker completes the salvage steps.", + "", + "### Non-goals", + "Do not finish CL-6946.", + "", + "### Risks", + "A headings-only complete would auto-dispatch builder on a stub.", + "", + "### Ordered steps", + "Add hasPlanFindings, then wire evaluateSubAgentStop.", + "", + "## Blockers", + "None.", + "", + "## Paths", + "src/subagent/report.ts", + ].join("\n"); + + const RISKS_IN_AC_BODY_PLAN_ENVELOPE = [ + "## Summary", + "Plan for the salvage gate.", + "", + "## Findings", + "### Files / paths", + "src/subagent/report.ts", + "", + "### Acceptance criteria", + "The worker mitigates residual risks.", + "", + "### Non-goals", + "Do not finish CL-6946.", + "", + "### Risks", + "A headings-only complete would auto-dispatch builder on a stub.", + "", + "### Ordered steps", + "Add hasPlanFindings, then wire evaluateSubAgentStop.", + "", + "## Blockers", + "None.", + "", + "## Paths", + "src/subagent/report.ts", + ].join("\n"); + + const NUMBERED_COUNSEL_PLAN_ENVELOPE = [ + "## Summary", + "Plan for the salvage gate.", + "", + "## Findings", + "1. Files / paths to touch", + " src/subagent/report.ts", + "2. Acceptance criteria", + " The worker completes the salvage steps.", + "3. Non-goals", + " Do not finish CL-6946.", + "4. Risks and open questions", + " A headings-only complete would auto-dispatch builder on a stub.", + "5. Ordered steps", + " Add hasPlanFindings, then wire evaluateSubAgentStop.", + "", + "## Blockers", + "None.", + "", + "## Paths", + "src/subagent/report.ts", + ].join("\n"); + test("evaluateSubAgentStop returns incomplete-report for Summary-only tool-less narration after tools", () => { expect( evaluateSubAgentStop({ @@ -443,6 +520,18 @@ describe("sub-agent stop helpers", () => { expect(hasPlanFindings(PASS_PLAN_ENVELOPE)).toBe(true); }); + test("hasPlanFindings stays true when an earlier section body uses the word steps", () => { + expect(hasPlanFindings(STEPS_IN_AC_BODY_PLAN_ENVELOPE)).toBe(true); + }); + + test("hasPlanFindings stays true when an earlier section body uses the word risks", () => { + expect(hasPlanFindings(RISKS_IN_AC_BODY_PLAN_ENVELOPE)).toBe(true); + }); + + test("hasPlanFindings is true for counsel numbered labels with following-line substance", () => { + expect(hasPlanFindings(NUMBERED_COUNSEL_PLAN_ENVELOPE)).toBe(true); + }); + test("hasReportEnvelope stays heading-presence only on headings-only text", () => { expect(hasReportEnvelope(HEADINGS_ONLY_ENVELOPE)).toBe(true); expect(hasPlanFindings(HEADINGS_ONLY_ENVELOPE)).toBe(false); @@ -500,6 +589,36 @@ describe("sub-agent stop helpers", () => { ).toBe("complete"); }); + test("evaluateSubAgentStop completes a five-section plan with steps in an earlier body", () => { + expect( + evaluateSubAgentStop({ + hasToolCalls: false, + requirePlanSubstance: true, + lastAssistantText: STEPS_IN_AC_BODY_PLAN_ENVELOPE, + }), + ).toBe("complete"); + }); + + test("evaluateSubAgentStop completes a five-section plan with risks in an earlier body", () => { + expect( + evaluateSubAgentStop({ + hasToolCalls: false, + requirePlanSubstance: true, + lastAssistantText: RISKS_IN_AC_BODY_PLAN_ENVELOPE, + }), + ).toBe("complete"); + }); + + test("evaluateSubAgentStop completes counsel numbered labels with following-line substance", () => { + expect( + evaluateSubAgentStop({ + hasToolCalls: false, + requirePlanSubstance: true, + lastAssistantText: NUMBERED_COUNSEL_PLAN_ENVELOPE, + }), + ).toBe("complete"); + }); + test("evaluateSubAgentStop does not stop for many unique reads while still calling tools", () => { let thrash = EMPTY_THRASH_STATE; for (let i = 0; i < 200; i++) { diff --git a/src/subagent/report.ts b/src/subagent/report.ts index 88ddadbca..54bc46635 100644 --- a/src/subagent/report.ts +++ b/src/subagent/report.ts @@ -155,13 +155,9 @@ function classifyPlanSectionLine( if (stripped.length === 0) return null; for (const section of PLAN_FINDING_SECTIONS) { const match = section.match.exec(stripped); - if (match === null || match.index === undefined) continue; - // Only treat a line as a section start when the label is the line's heading, - // not a later mention in an outline sentence. - if (match.index > 0 && /[,;]/.test(stripped.slice(0, match.index))) { - continue; - } - const after = stripped.slice(match.index + match[0].length); + // Labels start the heading or numbered item, not a later word in body prose. + if (match === null || match.index !== 0) continue; + const after = stripped.slice(match[0].length); const separated = after.match(/^\s*[:.\-–—]\s+(\S.*)$/); return { id: section.id, rest: separated?.[1] ?? "" }; }