Skip to content

Commit 4f4fe60

Browse files
committed
fix(subagent): digest mailbox reports instead of pasting them whole
1 parent 08b4137 commit 4f4fe60

7 files changed

Lines changed: 476 additions & 53 deletions

File tree

‎docs/ARCHITECTURE.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,12 +148,12 @@ A worker deny-on-ask registers a harness-owned denied-call envelope (`src/permis
148148

149149
Two directors, selected by role:
150150

151-
- **ChatDirector** (interactive, `src/agent/director.ts`) — Extends `DefaultDirector` with task list tracking, workflow nudges, LSP auto-activation, and multi-turn chat semantics. It never terminates the session: operator declines are surfaced as replies and the reactor stays alive for the next message. Yielding while a live fleet is running is allowed (idle-with-fleet); the open-task nudge does not rewrite that wait/reply, and the workflow idle rail does not declare the parent stuck while helpers run. The TUI idle-with-fleet seed alone is not occupancy: an empty fleet still auto-continues slash workflows. Occupancy, mailbox, and operator inbounds reset the workflow idle counter. When a worker finishes or fails while the parent is idle (including idle-with-fleet), occupancy delivers mailbox mail as system inbound so Skywalker starts a new turn without polling `wait_agents`. When the fleet goes dry with tasks still todo/doing, the TUI runtime re-enters the parent with collected worker reports rather than settling idle.
151+
- **ChatDirector** (interactive, `src/agent/director.ts`) — Extends `DefaultDirector` with task list tracking, workflow nudges, LSP auto-activation, and multi-turn chat semantics. It never terminates the session: operator declines are surfaced as replies and the reactor stays alive for the next message. Yielding while a live fleet is running is allowed (idle-with-fleet); the open-task nudge does not rewrite that wait/reply, and the workflow idle rail does not declare the parent stuck while helpers run. The TUI idle-with-fleet seed alone is not occupancy: an empty fleet still auto-continues slash workflows. Occupancy, mailbox, and operator inbounds reset the workflow idle counter. When a worker finishes or fails while the parent is idle (including idle-with-fleet), occupancy delivers mailbox mail as system inbound so Skywalker starts a new turn without polling `wait_agents`. Mailbox mail is a Summary + Blockers digest with a blob pointer to the full report, not the full report JSON. When the fleet goes dry with tasks still todo/doing, the TUI runtime re-enters the parent with collected worker reports rather than settling idle. Already-collected IDs in that continuation are id/status/description only.
152152

153153
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.
154154

155155
- **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** (once; later tool-less turns wait). 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. After real tool work, wrap-up Findings that are not placeholder or outline-only complete instead of being salvaged as a stub 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.
156-
`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 collect through mailbox mail the same way. A mounted `wait_agents` (exec primary) collects worker reports directly instead. 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, except a recoverable/continuable child failure (`continuable: true`) MAY spawn one successor with the same brief; 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.
156+
`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 with a Summary + Blockers digest and a blob pointer to the full report. Nested orchestrators collect through mailbox mail the same way. A mounted `wait_agents` (exec primary) collects worker reports directly instead. 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, except a recoverable/continuable child failure (`continuable: true`) MAY spawn one successor with the same brief; 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.
157157

158158
#### Model-family policy (`src/agent/model-family-policy.ts`)
159159

‎docs/IMPLEMENTATION.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ Listings are list-free, dumps are dump-locked: a bounded `ls`/`tree` prints name
212212

213213
- **Alt+Enter** queues a follow-up (kind `"queue"`) delivered only on **session-idle** — parent-idle **and** no live fleet lanes (`run` goes idle). Session-idle Alt+Enter is a no-op. **Ctrl+C** stops the run.
214214

215-
Idle-with-fleet is shipped: after a non-blocking `spawn_agent` dispatch the parent turn can settle while workers keep running. The runner emits a `fleet` event carrying the live-lane count (same liveness rule as the progress strip: interrupted leftovers are not live); the bridge holds the run busy on that count, so mid-hold Enter upgrades to a new primary turn (sent immediately) instead of queueing a steer, follow-ups keep waiting for true session-idle, and any steer left pending at the hold's engagement delivers immediately — the parent it was steering has already stopped. The workflow idle rail honors live helper occupancy, not the TUI idle-with-fleet seed: a live fleet is not a stuck parent, an empty fleet still auto-continues slash workflows, and occupancy / mailbox / operator inbounds reset the workflow idle counter. While the hold is up and the parent is not processing, occupancy flushes mailbox mail (`driveMailboxMail` + `buildMailboxMailMessage`) on store subscribe and idle-with-fleet settle — one child done while siblings run is enough. Skip that shot when a fleet-dry open-task continuation is latched. Both wakes wait for send to settle as accepted before taking reports; a pending Promise is not a continuation, and a failed or uncertain send idles the parent. Exec-primary `wait_agents` mounts with no yield predicate (`createWaitAgentsTool({ sessions, fleetRecords })`) and blocks to ready/timeout/abort; parent interrupt/abort cancels that parked waiter via the tool AbortSignal (`timed_out`, workers stay running) rather than a second waiter table. `interrupt_agent` on a child unblocks wait as interrupted through the mailbox overlay. `shouldYieldWait` remains a supported-but-unwired tool option (no production mount passes it).
215+
Idle-with-fleet is shipped: after a non-blocking `spawn_agent` dispatch the parent turn can settle while workers keep running. The runner emits a `fleet` event carrying the live-lane count (same liveness rule as the progress strip: interrupted leftovers are not live); the bridge holds the run busy on that count, so mid-hold Enter upgrades to a new primary turn (sent immediately) instead of queueing a steer, follow-ups keep waiting for true session-idle, and any steer left pending at the hold's engagement delivers immediately — the parent it was steering has already stopped. The workflow idle rail honors live helper occupancy, not the TUI idle-with-fleet seed: a live fleet is not a stuck parent, an empty fleet still auto-continues slash workflows, and occupancy / mailbox / operator inbounds reset the workflow idle counter. While the hold is up and the parent is not processing, occupancy flushes mailbox mail (`driveMailboxMail` + `buildMailboxMailMessage`) on store subscribe and idle-with-fleet settle — one child done while siblings run is enough. The mail payload is a Summary + Blockers digest with a blob pointer, not the full report JSON; fleet-dry continuation lists already-collected IDs as id/status/description only and does not re-deliver an ID mailbox mail is already sending. Skip that shot when a fleet-dry open-task continuation is latched. Both wakes wait for send to settle as accepted before taking reports; a pending Promise is not a continuation, and a failed or uncertain send idles the parent. Exec-primary `wait_agents` mounts with no yield predicate (`createWaitAgentsTool({ sessions, fleetRecords })`) and blocks to ready/timeout/abort; parent interrupt/abort cancels that parked waiter via the tool AbortSignal (`timed_out`, workers stay running) rather than a second waiter table. `interrupt_agent` on a child unblocks wait as interrupted through the mailbox overlay. `shouldYieldWait` remains a supported-but-unwired tool option (no production mount passes it).
216216

217217
`src/tui/stream-event-map.ts` maps reactor events onto the bridge's inbound events, and `src/tui/turn-state.ts` tracks the turn's status. `src/tui/turns-to-blocks.ts` hydrates a resumed session's stored turns into the same content blocks.
218218

‎docs/TUI.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -723,7 +723,9 @@ to idle — unless todo/doing tasks remain, in which case a system
723723
continuation starts before the fleet-0 event so the run stays busy and
724724
follow-ups wait one more turn. Occupancy wakes consume mailbox reports only
725725
after send settles as accepted; a failed or uncertain send idles the parent
726-
so a later flush can retry. The mail and that fleet-dry continuation
726+
so a later flush can retry. Mailbox mail is a Summary + Blockers digest with
727+
a blob pointer to the full report. Fleet-dry continuation lists
728+
already-collected IDs as id/status/description only. The mail and that fleet-dry continuation
727729
are runtime-to-agent traffic — the fleet board owns worker status — so
728730
neither paints a transcript row, and neither rehydrates as one.
729731

‎src/subagent/fleet-dry-drive.test.ts‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
shouldDriveOpenTasks,
1313
type FleetDryMailboxRecord,
1414
} from "./fleet-dry-drive.js";
15+
import { driveMailboxMail } from "./mailbox-mail-drive.js";
1516
import { createSubAgentSessionStore } from "./session-store.js";
1617
import {
1718
ACCEPTED_DELIVERY,
@@ -497,6 +498,107 @@ describe("driveOpenTasksAfterFleetDry", () => {
497498
expect(records.get("w1")?.collected).toBe(true);
498499
});
499500

501+
test("already-collected IDs are id, status, and description only", async () => {
502+
const records = recordsOf({
503+
prior: {
504+
status: "done",
505+
report: "SECRET_PRIOR_REPORT",
506+
description: "already mailed",
507+
collected: true,
508+
},
509+
fresh: {
510+
status: "done",
511+
report: "fresh report",
512+
description: "new lane",
513+
},
514+
});
515+
const sent: string[] = [];
516+
const driven = await driveOpenTasksAfterFleetDry({
517+
previousRunning: 1,
518+
running: 0,
519+
openTasks: [openTask],
520+
...driveFixture(records, {
521+
send: (prompt) => {
522+
sent.push(prompt);
523+
return ACCEPTED_DELIVERY;
524+
},
525+
}),
526+
});
527+
expect(driven).toBe(true);
528+
const parsed = reportsJSONFromPrompt(sent[0] ?? "") as {
529+
agent_id: string;
530+
status: string;
531+
description?: string;
532+
report?: string;
533+
}[];
534+
expect(parsed).toEqual([
535+
{
536+
agent_id: "fresh",
537+
status: "done",
538+
description: "new lane",
539+
report: "fresh report",
540+
},
541+
{
542+
agent_id: "prior",
543+
status: "done",
544+
description: "already mailed",
545+
},
546+
]);
547+
expect(sent[0]).not.toContain("SECRET_PRIOR_REPORT");
548+
expect(records.get("fresh")?.collected).toBe(true);
549+
expect(records.get("prior")?.collected).toBe(true);
550+
});
551+
552+
test("omits IDs mailbox mail is already delivering in the same window", async () => {
553+
const records = recordsOf({
554+
w1: {
555+
status: "done",
556+
report: "SECRET_MAILBOX_BODY",
557+
description: "lane",
558+
},
559+
});
560+
const mailbox = collectingMailbox(records);
561+
let resolveSend: ((result: typeof ACCEPTED_DELIVERY) => void) | undefined;
562+
let sendStarted: (() => void) | undefined;
563+
const sendSeen = new Promise<void>((resolve) => {
564+
sendStarted = resolve;
565+
});
566+
const mailboxDrive = driveMailboxMail({
567+
parentProcessing: false,
568+
mailbox,
569+
lanes: [],
570+
beginSystemContinuation: () => undefined,
571+
send: () => {
572+
sendStarted?.();
573+
return new Promise<typeof ACCEPTED_DELIVERY>((resolve) => {
574+
resolveSend = resolve;
575+
});
576+
},
577+
});
578+
await sendSeen;
579+
const drySent: string[] = [];
580+
const dryDriven = await driveOpenTasksAfterFleetDry({
581+
previousRunning: 1,
582+
running: 0,
583+
openTasks: [openTask],
584+
parentProcessing: false,
585+
mailbox,
586+
lanes: [],
587+
beginSystemContinuation: () => undefined,
588+
send: (prompt) => {
589+
drySent.push(prompt);
590+
return ACCEPTED_DELIVERY;
591+
},
592+
});
593+
expect(dryDriven).toBe(true);
594+
expect(reportsJSONFromPrompt(drySent[0] ?? "")).toEqual([]);
595+
expect(drySent[0]).not.toContain("SECRET_MAILBOX_BODY");
596+
expect(records.get("w1")?.collected).not.toBe(true);
597+
resolveSend?.(ACCEPTED_DELIVERY);
598+
expect(await mailboxDrive).toBe(true);
599+
expect(records.get("w1")?.collected).toBe(true);
600+
});
601+
500602
test("dry+open continuation JSON includes a spill URI for oversized reports", async () => {
501603
const original = `head-${"x".repeat(FLEET_DRY_REPORT_CHARS)}TAIL-MARKER`;
502604
const records = recordsOf({

0 commit comments

Comments
 (0)