diff --git a/src/subagent/index.test.ts b/src/subagent/index.test.ts index 02bd84f51..bc23403d6 100644 --- a/src/subagent/index.test.ts +++ b/src/subagent/index.test.ts @@ -21,6 +21,9 @@ import { classifyBriefSalvage, EMPTY_THRASH_STATE, nextThrashState, + salvagePathsFromThrash, + evaluateToolLessNarrationSpiral, + MAX_TOOLLESS_NARRATION_CYCLES, partialTextFromEvent, preferCompletedSubAgentReply, resolveSubAgentCatchOutcome, @@ -184,6 +187,30 @@ describe("sub-agent stop helpers", () => { ).toBe("incomplete-report-stop"); }); + test("evaluateToolLessNarrationSpiral nudges once then stops at the cycle cap", () => { + expect(evaluateToolLessNarrationSpiral(1)).toBe("nudge"); + expect(evaluateToolLessNarrationSpiral(MAX_TOOLLESS_NARRATION_CYCLES)).toBe("stop"); + expect(evaluateToolLessNarrationSpiral(MAX_TOOLLESS_NARRATION_CYCLES + 1)).toBe("stop"); + }); + + test("evaluateSubAgentStop spiral uses toolLessNarrationCycles over the deprecated flag", () => { + expect( + evaluateSubAgentStop({ + hasToolCalls: false, + lastAssistantText: SUMMARY_ONLY_NARRATION, + toolLessNarrationCycles: 1, + incompleteReportNudgeFired: true, + }), + ).toBe("incomplete-report"); + expect( + evaluateSubAgentStop({ + hasToolCalls: false, + lastAssistantText: SUMMARY_ONLY_NARRATION, + toolLessNarrationCycles: 2, + }), + ).toBe("incomplete-report-stop"); + }); + test("evaluateSubAgentStop returns complete for tool-less after tools with all four headings", () => { expect( evaluateSubAgentStop({ @@ -390,28 +417,60 @@ describe("sub-agent stop helpers", () => { expect(deadlineWithHint).toContain("wall-clock deadline"); expect(deadlineWithHint).toContain("deadline reached"); // Only fires for a deadline report, not for other forced-stop reasons. - expect( - appendSubAgentParentHints(forcedStopReport("cancelled", "x"), "cancelled"), - ).not.toContain("wall-clock deadline"); + const cancelledWithHint = appendSubAgentParentHints( + forcedStopReport("cancelled", "x"), + "cancelled", + ); + expect(cancelledWithHint).not.toContain("wall-clock deadline"); + expect(cancelledWithHint).toContain("was cancelled before finishing"); + expect(cancelledWithHint).toContain("Findings and Paths"); + + // Paths section carries thrash salvage; empty prose with paths still informs Findings. + const withPaths = forcedStopReport("cancelled", "", { + paths: ["src/a.ts", "src/b.ts"], + }); + const withPathsParsed = parseSubAgentReport(withPaths); + expect(withPathsParsed.paths).toContain("src/a.ts"); + expect(withPathsParsed.paths).toContain("src/b.ts"); + expect(withPathsParsed.findings).toContain("Files touched before stop"); + expect(withPathsParsed.findings).toContain("src/a.ts"); }); test("forcedStopReport renders a Stopped line for display; classification uses the typed reason", () => { - expect(forcedStopReport("cancelled", "partial", "Session closed")).toMatch( + expect(forcedStopReport("cancelled", "partial", { detail: "Session closed" })).toMatch( /^Stopped: cancelled — Session closed\n/, ); expect(forcedStopReport("cancelled", "partial")).toMatch(/^Stopped: cancelled\n/); - expect(forcedStopReport("deadline", "x", "30s elapsed")).toMatch( + expect(forcedStopReport("deadline", "x", { detail: "30s elapsed" })).toMatch( /^Stopped: deadline — 30s elapsed\n/, ); // Nested Stopped: under Findings is display-only; classify via typed reason. const nested = forcedStopReport( "deadline", - forcedStopReport("cancelled", "inner", "inner reason"), + forcedStopReport("cancelled", "inner", { detail: "inner reason" }), ); expect(nested).toMatch(/^Stopped: deadline\n/); expect(nested).toContain("Stopped: cancelled — inner reason"); }); + test("salvagePathsFromThrash prefers edited paths then collapses chunked reads", () => { + const state = nextThrashState(EMPTY_THRASH_STATE, [ + { + type: "tool_call", + name: "read_file", + arguments: { path: "src/a.ts", offset: 0, limit: 10 }, + }, + { + type: "tool_call", + name: "edit_file", + arguments: { path: "src/b.ts", old_string: "a", new_string: "b" }, + }, + { type: "tool_call", name: "read_file", arguments: { path: "src/a.ts" } }, + ]); + expect(salvagePathsFromThrash(state)).toEqual(["src/b.ts", "src/a.ts"]); + expect(salvagePathsFromThrash(state, 1)).toEqual(["src/b.ts"]); + }); + test("createSubAgentRunController aborts on an explicit deadline and reports deadlineHit", async () => { const ctl = createSubAgentRunController(undefined, 20); expect(ctl.signal.aborted).toBe(false); diff --git a/src/subagent/index.ts b/src/subagent/index.ts index 73a6f32a6..dba195f3c 100644 --- a/src/subagent/index.ts +++ b/src/subagent/index.ts @@ -21,7 +21,12 @@ export { type FleetObservation, type FleetWatch, } from "./fleet-report.js"; -export { EMPTY_THRASH_STATE, nextThrashState, type ThrashState } from "./thrash.js"; +export { + EMPTY_THRASH_STATE, + nextThrashState, + salvagePathsFromThrash, + type ThrashState, +} from "./thrash.js"; export { appendActivitySummary, buildDispatchBrief, @@ -36,17 +41,21 @@ export { } from "./report.js"; export { SUBAGENT_DEADLINE_MARGIN_MS, + MAX_TOOLLESS_NARRATION_CYCLES, appendSubAgentParentHints, evaluateSubAgentStop, + evaluateToolLessNarrationSpiral, forcedStopReport, partialTextFromEvent, preferCompletedSubAgentReply, resolveSubAgentCatchOutcome, resolveSubAgentDeadlineMs, type ForcedStopReason, + type ForcedStopReportOptions, type SubAgentCatchOutcome, type SubAgentParentHintOptions, type SubAgentStopReason, + type ToolLessNarrationSpiral, } from "./stop-policy.js"; export { diff --git a/src/subagent/nudge-director.test.ts b/src/subagent/nudge-director.test.ts index 5d405ffb6..a3c78e4b8 100644 --- a/src/subagent/nudge-director.test.ts +++ b/src/subagent/nudge-director.test.ts @@ -392,6 +392,8 @@ describe("SubAgentDirector incomplete-report wiring", () => { if (reply === undefined || reply.type !== "reply") throw new Error("expected reply action"); expect(reply.content).toContain("narrated instead of writing a report envelope"); expect(reply.content).toContain("Still narrating, no envelope."); + expect(reply.content).toContain("## Paths"); + expect(reply.content).toContain("read-1.ts"); }); test("tool-less turn with the four headings completes normally", async () => { diff --git a/src/subagent/nudge-director.ts b/src/subagent/nudge-director.ts index add920355..5e8df6a91 100644 --- a/src/subagent/nudge-director.ts +++ b/src/subagent/nudge-director.ts @@ -15,7 +15,12 @@ import type { } from "@intx/types/runtime"; import { createCompactionGovernor, type CompactionGovernor } from "../agent/compaction.js"; import { onTurnBoundary } from "../agent/reactor-events.js"; -import { EMPTY_THRASH_STATE, nextThrashState, type ThrashState } from "./thrash.js"; +import { + EMPTY_THRASH_STATE, + nextThrashState, + salvagePathsFromThrash, + type ThrashState, +} from "./thrash.js"; import { NOOP_INTERVENTION_SINK, type InterventionSink } from "./intervention-log.js"; import { evaluateSubAgentStop, @@ -86,8 +91,9 @@ export class SubAgentDirector extends DefaultDirector { // already completed. private lastConsumedNudgeText: string | null = null; // Soft incomplete-report wrap-up is one-shot per run; a second tool-less - // narration without the envelope salvages as incomplete-report. - private incompleteReportNudgeFired = false; + // narration without the envelope salvages as incomplete-report + // (MAX_TOOLLESS_NARRATION_CYCLES = 2). + private toolLessNarrationCycles = 0; // Stall management: a leaf that goes quiet (e.g. parked on a long-running // background command with nothing else to do) produces no inbound events @@ -213,7 +219,7 @@ export class SubAgentDirector extends DefaultDirector { thrashState: this.thrashState, requireEvidence: this.requireEvidence, lastAssistantText: this.lastAssistantText, - incompleteReportNudgeFired: this.incompleteReportNudgeFired, + toolLessNarrationCycles: this.toolLessNarrationCycles + 1, }); if (stop === "complete") { @@ -229,7 +235,7 @@ export class SubAgentDirector extends DefaultDirector { if (stop === "incomplete-report") { // Tool-less turn after tools, no report envelope. Must not fall through // to super.decide — DefaultDirector completes any tool-less turn. - this.incompleteReportNudgeFired = true; + this.toolLessNarrationCycles += 1; this.interventions({ id: "incomplete-report", class: "nudge", @@ -242,6 +248,7 @@ export class SubAgentDirector extends DefaultDirector { ]; } if (stop === "incomplete-report-stop") { + this.toolLessNarrationCycles += 1; this.interventions({ id: "incomplete-report-stop", class: "stop", @@ -251,7 +258,11 @@ export class SubAgentDirector extends DefaultDirector { this.onForcedStop("incomplete-report"); const terminal: ReactorAction[] = [ capabilities.checkpoint("subagent-incomplete-report"), - capabilities.reply(forcedStopReport("incomplete-report", this.lastAssistantText)), + capabilities.reply( + forcedStopReport("incomplete-report", this.lastAssistantText, { + paths: salvagePathsFromThrash(this.thrashState), + }), + ), ]; this.compaction.noteIdleTurn(event, terminal); const compacted = this.compaction.interceptActions(event, terminal, capabilities); @@ -330,11 +341,10 @@ export class SubAgentDirector extends DefaultDirector { const terminal: ReactorAction[] = [ capabilities.checkpoint("subagent-stalled"), capabilities.reply( - forcedStopReport( - "stalled", - this.lastAssistantText, - `no activity for ${Math.round(elapsed / 1000)}s after stall nudge`, - ), + forcedStopReport("stalled", this.lastAssistantText, { + detail: `no activity for ${Math.round(elapsed / 1000)}s after stall nudge`, + paths: salvagePathsFromThrash(this.thrashState), + }), ), ]; return terminal; diff --git a/src/subagent/run.ts b/src/subagent/run.ts index 8c7cdd467..a5b4544e0 100644 --- a/src/subagent/run.ts +++ b/src/subagent/run.ts @@ -90,6 +90,7 @@ import { resolveSubAgentDeadlineMs, type ForcedStopReason, } from "./stop-policy.js"; +import { EMPTY_THRASH_STATE, nextThrashState, salvagePathsFromThrash } from "./thrash.js"; import { SubAgentDirector } from "./nudge-director.js"; import { assertTierMayMountFleetVerb } from "./authority.js"; import { createReadAgentTraceTool } from "./trace-tool.js"; @@ -268,6 +269,22 @@ function abortReasonText(signal: AbortSignal): string | undefined { return undefined; } +/** + * Findings payload for cancel/deadline salvage. Prefer multi-turn accumulated + * prose; fall back to the last turn-boundary text, then the in-flight cycle tail. + */ +function salvageFindingsText( + accumulatedProse: string, + lastPartialText: string, + abortedCycleText: string, +): string { + const prior = accumulatedProse.trim(); + if (prior.length > 0) return prior; + const last = lastPartialText.trim(); + if (last.length > 0) return last; + return abortedCycleText.slice(-2000); +} + /** * Arm requireEvidence only for the critic director. Greybeard is also * intent=review and may spawn-only then envelope; that is not a fake @@ -772,6 +789,12 @@ export async function runSubAgent(params: RunSubAgentParams): Promise 0) { + thrashState = nextThrashState(thrashState, [ + { type: "tool_call", name: call.name, arguments: call.arguments }, + ]); + } + } cycleRecorder.handleEvent(event); const partial = partialTextFromEvent(event); - if (partial !== null) lastPartialText = partial; + if (partial !== null) { + lastPartialText = partial; + const trimmed = partial.trim(); + if (trimmed.length > 0) { + const joined = + accumulatedProse.length === 0 ? trimmed : `${accumulatedProse}\n\n${trimmed}`; + accumulatedProse = + joined.length <= TURN_PROSE_CAP ? joined : joined.slice(-TURN_PROSE_CAP); + } + } params.onEvent?.(event); }; streamPromise = consumeStream(agent.stream(), streamSink); @@ -927,11 +968,13 @@ export async function runSubAgent(params: RunSubAgentParams): Promise 0 ? lastPartialText : abortedCycleText.slice(-2000); + const tail = salvageFindingsText(accumulatedProse, lastPartialText, abortedCycleText); return { report: appendActivitySummary( - forcedStopReport("cancelled", tail, "interrupted by interrupt_agent"), + forcedStopReport("cancelled", tail, { + detail: "interrupted by interrupt_agent", + paths: salvagePathsFromThrash(thrashState), + }), toolNamesUsed, ), stopReason: "cancelled", @@ -953,15 +996,17 @@ export async function runSubAgent(params: RunSubAgentParams): Promise 0 || lastPartialText.trim().length > 0; + const hadProgress = + toolNamesUsed.length > 0 || + lastPartialText.trim().length > 0 || + accumulatedProse.trim().length > 0; const outcome = resolveSubAgentCatchOutcome({ deadlineHit: runController.deadlineHit(), hadProgress, }); if (outcome !== "rethrow") { const reason = outcome === "salvage-deadline" ? "deadline" : "cancelled"; - const tail = - lastPartialText.trim().length > 0 ? lastPartialText : abortedCycleText.slice(-2000); + const tail = salvageFindingsText(accumulatedProse, lastPartialText, abortedCycleText); const detail = reason === "deadline" && resolvedDeadlineMs !== undefined ? `${resolvedDeadlineMs}ms elapsed` @@ -973,7 +1018,13 @@ export async function runSubAgent(params: RunSubAgentParams): Promise= MAX_TOOLLESS_NARRATION_CYCLES ? "stop" : "nudge"; +} + /** * Pure stop decision for leaf workers. Null means keep running tools. * @@ -79,7 +96,7 @@ export type SubAgentStopReason = "complete" | "incomplete-report" | "incomplete- * that never called a tool at all) completes only when the assistant text has * a four-heading envelope (Summary, Findings, Blockers, Paths). Missing * envelope nudges once (`incomplete-report`) then salvages - * (`incomplete-report-stop`). + * (`incomplete-report-stop`) via 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. @@ -100,24 +117,36 @@ export function evaluateSubAgentStop(input: { * (Summary/Findings/Blockers/Paths) nudges once then salvages. */ lastAssistantText: string; - /** True after the one-shot incomplete-report wrap-up nudge has been injected. */ + /** + * 1-based count of consecutive tool-less narration turns so far (including + * the current one). When omitted, falls back to `incompleteReportNudgeFired` + * for older call sites (false → cycle 1, true → cycle 2). + */ + toolLessNarrationCycles?: number; + /** + * @deprecated Prefer `toolLessNarrationCycles`. True after the one-shot + * incomplete-report wrap-up nudge has been injected. + */ incompleteReportNudgeFired?: boolean; }): SubAgentStopReason | null { + const spiralCycles = + input.toolLessNarrationCycles ?? (input.incompleteReportNudgeFired === true ? 2 : 1); + const spiralStop = (): SubAgentStopReason => + evaluateToolLessNarrationSpiral(spiralCycles) === "stop" + ? "incomplete-report-stop" + : "incomplete-report"; + // A tool-less turn is complete only with a report envelope. CritiqueDirector // additionally requires at least one read/search (hasEvidence). if (!input.hasToolCalls) { if (!hasReportEnvelope(input.lastAssistantText)) { - return input.incompleteReportNudgeFired === true - ? "incomplete-report-stop" - : "incomplete-report"; + return spiralStop(); } if ( input.requireEvidence === true && (input.thrashState === undefined || input.thrashState.readCounts.size === 0) ) { - return input.incompleteReportNudgeFired === true - ? "incomplete-report-stop" - : "incomplete-report"; + return spiralStop(); } return "complete"; } @@ -154,6 +183,14 @@ export function partialTextFromEvent(event: ReactorEmittedEvent): string | null export type ForcedStopReason = "cancelled" | "deadline" | "stalled" | "incomplete-report"; +/** Optional detail / Paths payload for a forced-stop salvage envelope. */ +export interface ForcedStopReportOptions { + /** Path-specific specifics (cancel reason) rendered on the `Stopped:` line. */ + detail?: string; + /** Edited/read paths for the Paths section (string or list; capped by caller). */ + paths?: string | readonly string[]; +} + // Exact Summary text rendered for each forced-stop reason. Human-facing only — // forcedStopReport is the sole reader; the parent classifies outcomes from the // structured ForcedStopReason value itself (see run.ts/task-tool.ts), never by @@ -166,19 +203,29 @@ const FORCED_STOP_SUMMARIES: Record = { "incomplete-report": "Stopped: worker narrated instead of writing a report envelope.", }; +function normalizeSalvagePaths(paths: ForcedStopReportOptions["paths"]): string { + if (paths === undefined) return ""; + if (typeof paths === "string") return paths.trim(); + return paths + .map((p) => p.trim()) + .filter((p) => p.length > 0) + .join("\n"); +} + /** * Build the parent-facing report when a leaf is force-stopped. There is no * further inference, so this must already be a full envelope — not an - * instruction asking the finished worker to summarize. `detail` is the - * path-specific specifics (cancel reason) rendered verbatim on the report's - * `Stopped:` line so the parent and the TUI see the cause, not just that the - * worker stopped. + * instruction asking the finished worker to summarize. Options carry the + * cancel `detail` (Stopped line) and salvage `paths` (Paths section). Findings + * keep demoted partial prose, or a files-touched stub when only paths remain. */ export function forcedStopReport( reason: ForcedStopReason, partialText: string, - detail?: string, + options: ForcedStopReportOptions = {}, ): string { + const detail = options.detail; + const pathText = normalizeSalvagePaths(options.paths); const summary = FORCED_STOP_SUMMARIES[reason]; const blockers = reason === "cancelled" @@ -191,15 +238,18 @@ export function forcedStopReport( // Demote nested report-section headings so runSubAgent's parse/format pass // cannot clobber this outer Summary/Blockers with an agent-shaped envelope // stuffed into Findings (cancel after a structured partial). + const trimmed = partialText.trim(); const findings = - partialText.trim().length > 0 - ? demoteNestedReportHeadings(partialText.trim()) - : "(no partial findings on the final turn)"; + trimmed.length > 0 + ? demoteNestedReportHeadings(trimmed) + : pathText.length > 0 + ? `Files touched before stop:\n${pathText}` + : "(no partial findings on the final turn)"; return formatSubAgentReport({ summary, findings, blockers, - paths: "", + paths: pathText, stopped: detail !== undefined && detail.length > 0 ? `${reason} — ${detail}` : reason, }); } @@ -207,6 +257,9 @@ export function forcedStopReport( const DEADLINE_PARENT_HINT = "[Sub-agent hit an explicit wall-clock deadline before finishing. Continue from Findings rather than redoing completed work; re-dispatch with continuation context and a longer deadline only if more wall-clock time is warranted.]"; +const CANCELLED_PARENT_HINT = + "[Sub-agent was cancelled before finishing. Continue from Findings and Paths rather than redoing completed work; re-dispatch only if remaining work is still needed.]"; + /** Options for parent-hint stacking (session re-dispatch ledger state). */ export interface SubAgentParentHintOptions { /** @@ -219,8 +272,8 @@ export interface SubAgentParentHintOptions { /** * Prepend the parent-facing salvage hint for `reason`, chosen from the * structured ForcedStopReason the run reported directly — never by parsing - * `report`'s prose. Reasons with no dedicated hint (cancelled, stalled, - * incomplete-report, or a normal complete) pass `report` through unchanged. + * `report`'s prose. Reasons with no dedicated hint (stalled, incomplete-report, + * or a normal complete) pass `report` through unchanged. */ export function appendSubAgentParentHints( report: string, @@ -230,6 +283,8 @@ export function appendSubAgentParentHints( switch (reason) { case "deadline": return `${DEADLINE_PARENT_HINT}\n\n${report}`; + case "cancelled": + return `${CANCELLED_PARENT_HINT}\n\n${report}`; default: return report; } diff --git a/src/subagent/thrash.test.ts b/src/subagent/thrash.test.ts index fba4496e9..02118603e 100644 --- a/src/subagent/thrash.test.ts +++ b/src/subagent/thrash.test.ts @@ -2,6 +2,7 @@ import { describe, expect, test } from "bun:test"; import { EMPTY_THRASH_STATE, nextThrashState, + salvagePathsFromThrash, type ThrashState, type ThrashToolCallBlock, } from "./thrash.js"; @@ -106,4 +107,16 @@ describe("thrash pure module", () => { const state = applyAll([grep("needle"), grep("needle")]); expect(state.readCounts.get("grep::needle::src")).toBe(2); }); + + test("salvagePathsFromThrash lists edited first, then read paths, capped", () => { + const state = applyAll([ + read("src/read.ts"), + edit("src/edit.ts"), + read("src/read.ts", { offset: 0, limit: 20 }), + grep("needle"), + ]); + expect(salvagePathsFromThrash(state)).toEqual(["src/edit.ts", "src/read.ts", "src"]); + expect(salvagePathsFromThrash(state, 0)).toEqual([]); + expect(salvagePathsFromThrash(EMPTY_THRASH_STATE)).toEqual([]); + }); }); diff --git a/src/subagent/thrash.ts b/src/subagent/thrash.ts index 6aaa06df7..8f725fa19 100644 --- a/src/subagent/thrash.ts +++ b/src/subagent/thrash.ts @@ -5,7 +5,9 @@ * prohibits shell file work, but a prompt violation deserves a correction, * not a verdict that the work never happened. `editedPaths` (from typed write * tools only) is diagnostics for interventions.jsonl; no stop decision - * depends on it. + * depends on it. Cancel/incomplete salvage also lists these paths via + * salvagePathsFromThrash so the parent keeps file evidence when the leaf + * is force-stopped. */ import { isProductMutationTool, productMutationPaths } from "../agent/product-mutation-tools.js"; @@ -25,6 +27,9 @@ export const EMPTY_THRASH_STATE: ThrashState = { totalToolCalls: 0, }; +/** Cap on Paths lines rendered into a forced-stop salvage report. */ +export const SALVAGE_PATHS_CAP = 40; + /** Content block shape compatible with fingerprintToolCalls / inference turns. */ export interface ThrashToolCallBlock { type: string; @@ -141,3 +146,51 @@ export function nextThrashState( totalToolCalls, }; } + +/** + * Recover a filesystem path from a thrash readCounts key. Chunked reads + * (`path::offset:limit`) collapse to `path`; search keys contribute their + * scoped path segment when present; bare `shell:program` keys are skipped. + */ +function pathFromReadKey(key: string): string | null { + if (key.startsWith("shell:")) return null; + if (key.startsWith("grep::") || key.startsWith("search_files::")) { + const parts = key.split("::"); + const scoped = parts[2]; + return scoped !== undefined && scoped.length > 0 ? scoped : null; + } + const chunkSep = key.indexOf("::"); + if (chunkSep === -1) return key.length > 0 ? key : null; + const path = key.slice(0, chunkSep); + return path.length > 0 ? path : null; +} + +/** + * Edited then read paths for a forced-stop Paths section. Deduped, edited + * first, capped so a thrashing leaf cannot flood the parent report. + */ +export function salvagePathsFromThrash( + state: ThrashState, + cap: number = SALVAGE_PATHS_CAP, +): string[] { + const limit = Math.max(0, Math.floor(cap)); + if (limit === 0) return []; + const out: string[] = []; + const seen = new Set(); + const push = (path: string): void => { + if (out.length >= limit) return; + if (seen.has(path)) return; + seen.add(path); + out.push(path); + }; + for (const edited of state.editedPaths) { + if (edited.length > 0) push(edited); + if (out.length >= limit) return out; + } + for (const key of state.readCounts.keys()) { + const path = pathFromReadKey(key); + if (path !== null) push(path); + if (out.length >= limit) return out; + } + return out; +}