Merge pull request #919 from corbitsdev/cl-7756-salvage-plancounsel-stub-findings-as-incomplete-not-a

Salvage stub plan findings as an incomplete report

Author: TheGreatAxios

CHANGELOG.md

@@ -13,6 +13,13 @@ 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 —
@@ -24,6 +31,7 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename
   ask is dropped. Do not poll `list_agents`.
 
 
+
 ## [0.3.21] - 2026-09-11
 
 ### Added

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 `<tool_call>` 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 `<tool_call>` 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`)

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. Once a parked ask is surfaced, `list_agents` fails closed until `send_input` answers or the ask is dropped — do not poll `list_agents`. 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 `<tool_call>` 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 `<tool_call>` 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)
 

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", () => {

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.
 

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");
   });

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.

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)", () => {

src/subagent/agent-fleet.test.ts

@@ -617,6 +617,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");