Skip to content

Commit f82d437

Browse files
committed
Keep cancelled worker Findings and Paths on salvage
Force-stopped leaves were returning empty Paths and often only the final-turn scrap as Findings, so parents re-did completed file work. Accumulate mid-run prose and thrash paths into the catch/interrupt salvage envelope, and give cancelled runs the same parent hint shape as deadlines.
1 parent 02a3f85 commit f82d437

8 files changed

Lines changed: 286 additions & 48 deletions

File tree

src/subagent/index.test.ts

Lines changed: 63 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ import {
2222
classifyBriefSalvage,
2323
EMPTY_THRASH_STATE,
2424
nextThrashState,
25+
salvagePathsFromThrash,
26+
evaluateToolLessNarrationSpiral,
27+
MAX_TOOLLESS_NARRATION_CYCLES,
2528
partialTextFromEvent,
2629
preferCompletedSubAgentReply,
2730
resolveSubAgentCatchOutcome,
@@ -185,6 +188,30 @@ describe("sub-agent stop helpers", () => {
185188
).toBe("incomplete-report-stop");
186189
});
187190

191+
test("evaluateToolLessNarrationSpiral nudges once then stops at the cycle cap", () => {
192+
expect(evaluateToolLessNarrationSpiral(1)).toBe("nudge");
193+
expect(evaluateToolLessNarrationSpiral(MAX_TOOLLESS_NARRATION_CYCLES)).toBe("stop");
194+
expect(evaluateToolLessNarrationSpiral(MAX_TOOLLESS_NARRATION_CYCLES + 1)).toBe("stop");
195+
});
196+
197+
test("evaluateSubAgentStop spiral uses toolLessNarrationCycles over the deprecated flag", () => {
198+
expect(
199+
evaluateSubAgentStop({
200+
hasToolCalls: false,
201+
lastAssistantText: SUMMARY_ONLY_NARRATION,
202+
toolLessNarrationCycles: 1,
203+
incompleteReportNudgeFired: true,
204+
}),
205+
).toBe("incomplete-report");
206+
expect(
207+
evaluateSubAgentStop({
208+
hasToolCalls: false,
209+
lastAssistantText: SUMMARY_ONLY_NARRATION,
210+
toolLessNarrationCycles: 2,
211+
}),
212+
).toBe("incomplete-report-stop");
213+
});
214+
188215
test("evaluateSubAgentStop returns complete for tool-less after tools with all four headings", () => {
189216
expect(
190217
evaluateSubAgentStop({
@@ -391,31 +418,59 @@ describe("sub-agent stop helpers", () => {
391418
expect(deadlineWithHint).toContain("wall-clock deadline");
392419
expect(deadlineWithHint).toContain("deadline reached");
393420
// Only fires for a deadline report, not for other forced-stop reasons.
394-
expect(
395-
appendSubAgentParentHints(forcedStopReport("cancelled", "x"), "cancelled"),
396-
).not.toContain("wall-clock deadline");
421+
const cancelledWithHint = appendSubAgentParentHints(
422+
forcedStopReport("cancelled", "x"),
423+
"cancelled",
424+
);
425+
expect(cancelledWithHint).not.toContain("wall-clock deadline");
426+
expect(cancelledWithHint).toContain("was cancelled before finishing");
427+
expect(cancelledWithHint).toContain("Findings and Paths");
428+
429+
// Paths section carries thrash salvage; empty prose with paths still informs Findings.
430+
const withPaths = forcedStopReport("cancelled", "", {
431+
paths: ["src/a.ts", "src/b.ts"],
432+
});
433+
const withPathsParsed = parseSubAgentReport(withPaths);
434+
expect(withPathsParsed.paths).toContain("src/a.ts");
435+
expect(withPathsParsed.paths).toContain("src/b.ts");
436+
expect(withPathsParsed.findings).toContain("Files touched before stop");
437+
expect(withPathsParsed.findings).toContain("src/a.ts");
397438
});
398439

399440
test("forcedStopReport carries a machine-readable Stopped line the parent sees verbatim", () => {
400-
const cancelled = forcedStopReport("cancelled", "partial", "Session closed");
441+
const cancelled = forcedStopReport("cancelled", "partial", { detail: "Session closed" });
401442
expect(stopReasonFromReport(cancelled)).toBe("cancelled — Session closed");
402443
// Without a detail the line is the bare reason token.
403444
expect(stopReasonFromReport(forcedStopReport("cancelled", "partial"))).toBe("cancelled");
404-
expect(stopReasonFromReport(forcedStopReport("deadline", "x", "30s elapsed"))).toBe(
405-
"deadline30s elapsed",
406-
);
445+
expect(
446+
stopReasonFromReport(forcedStopReport("deadline", "x", { detail: "30s elapsed" })),
447+
).toBe("deadline — 30s elapsed");
407448

408449
// A nested forced-stop quoted in Findings must not leak its Stopped line
409450
// as the outer report's reason.
410451
const nested = forcedStopReport(
411452
"deadline",
412-
forcedStopReport("cancelled", "inner", "inner reason"),
453+
forcedStopReport("cancelled", "inner", { detail: "inner reason" }),
413454
);
414455
expect(stopReasonFromReport(nested)).toBe("deadline");
415456
// A clean report has no Stopped line.
416457
expect(stopReasonFromReport("## Summary\nDone.\n\n## Findings\nx")).toBe(null);
417458
});
418459

460+
test("salvagePathsFromThrash prefers edited paths then collapses chunked reads", () => {
461+
const state = nextThrashState(EMPTY_THRASH_STATE, [
462+
{ type: "tool_call", name: "read_file", arguments: { path: "src/a.ts", offset: 0, limit: 10 } },
463+
{
464+
type: "tool_call",
465+
name: "edit_file",
466+
arguments: { path: "src/b.ts", old_string: "a", new_string: "b" },
467+
},
468+
{ type: "tool_call", name: "read_file", arguments: { path: "src/a.ts" } },
469+
]);
470+
expect(salvagePathsFromThrash(state)).toEqual(["src/b.ts", "src/a.ts"]);
471+
expect(salvagePathsFromThrash(state, 1)).toEqual(["src/b.ts"]);
472+
});
473+
419474
test("createSubAgentRunController aborts on an explicit deadline and reports deadlineHit", async () => {
420475
const ctl = createSubAgentRunController(undefined, 20);
421476
expect(ctl.signal.aborted).toBe(false);

src/subagent/index.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ export {
2121
type FleetObservation,
2222
type FleetWatch,
2323
} from "./fleet-report.js";
24-
export { EMPTY_THRASH_STATE, nextThrashState, type ThrashState } from "./thrash.js";
24+
export { EMPTY_THRASH_STATE, nextThrashState, salvagePathsFromThrash, type ThrashState } from "./thrash.js";
2525
export {
2626
appendActivitySummary,
2727
buildDispatchBrief,
@@ -37,17 +37,21 @@ export {
3737
} from "./report.js";
3838
export {
3939
SUBAGENT_DEADLINE_MARGIN_MS,
40+
MAX_TOOLLESS_NARRATION_CYCLES,
4041
appendSubAgentParentHints,
4142
evaluateSubAgentStop,
43+
evaluateToolLessNarrationSpiral,
4244
forcedStopReport,
4345
partialTextFromEvent,
4446
preferCompletedSubAgentReply,
4547
resolveSubAgentCatchOutcome,
4648
resolveSubAgentDeadlineMs,
4749
type ForcedStopReason,
50+
type ForcedStopReportOptions,
4851
type SubAgentCatchOutcome,
4952
type SubAgentParentHintOptions,
5053
type SubAgentStopReason,
54+
type ToolLessNarrationSpiral,
5155
} from "./stop-policy.js";
5256

5357
export {

src/subagent/nudge-director.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,8 @@ describe("SubAgentDirector incomplete-report wiring", () => {
392392
if (reply === undefined || reply.type !== "reply") throw new Error("expected reply action");
393393
expect(reply.content).toContain("narrated instead of writing a report envelope");
394394
expect(reply.content).toContain("Still narrating, no envelope.");
395+
expect(reply.content).toContain("## Paths");
396+
expect(reply.content).toContain("read-1.ts");
395397
});
396398

397399
test("tool-less turn with the four headings completes normally", async () => {

src/subagent/nudge-director.ts

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import type {
1515
} from "@intx/types/runtime";
1616
import { createCompactionGovernor, type CompactionGovernor } from "../agent/compaction.js";
1717
import { onTurnBoundary } from "../agent/reactor-events.js";
18-
import { EMPTY_THRASH_STATE, nextThrashState, type ThrashState } from "./thrash.js";
18+
import { EMPTY_THRASH_STATE, nextThrashState, salvagePathsFromThrash, type ThrashState } from "./thrash.js";
1919
import { NOOP_INTERVENTION_SINK, type InterventionSink } from "./intervention-log.js";
2020
import {
2121
evaluateSubAgentStop,
@@ -86,8 +86,9 @@ export class SubAgentDirector extends DefaultDirector {
8686
// already completed.
8787
private lastConsumedNudgeText: string | null = null;
8888
// Soft incomplete-report wrap-up is one-shot per run; a second tool-less
89-
// narration without the envelope salvages as incomplete-report.
90-
private incompleteReportNudgeFired = false;
89+
// narration without the envelope salvages as incomplete-report
90+
// (MAX_TOOLLESS_NARRATION_CYCLES = 2).
91+
private toolLessNarrationCycles = 0;
9192

9293
// Stall management: a leaf that goes quiet (e.g. parked on a long-running
9394
// background command with nothing else to do) produces no inbound events
@@ -213,7 +214,7 @@ export class SubAgentDirector extends DefaultDirector {
213214
thrashState: this.thrashState,
214215
requireEvidence: this.requireEvidence,
215216
lastAssistantText: this.lastAssistantText,
216-
incompleteReportNudgeFired: this.incompleteReportNudgeFired,
217+
toolLessNarrationCycles: this.toolLessNarrationCycles + 1,
217218
});
218219

219220
if (stop === "complete") {
@@ -229,7 +230,7 @@ export class SubAgentDirector extends DefaultDirector {
229230
if (stop === "incomplete-report") {
230231
// Tool-less turn after tools, no report envelope. Must not fall through
231232
// to super.decide — DefaultDirector completes any tool-less turn.
232-
this.incompleteReportNudgeFired = true;
233+
this.toolLessNarrationCycles += 1;
233234
this.interventions({
234235
id: "incomplete-report",
235236
class: "nudge",
@@ -242,6 +243,7 @@ export class SubAgentDirector extends DefaultDirector {
242243
];
243244
}
244245
if (stop === "incomplete-report-stop") {
246+
this.toolLessNarrationCycles += 1;
245247
this.interventions({
246248
id: "incomplete-report-stop",
247249
class: "stop",
@@ -251,7 +253,11 @@ export class SubAgentDirector extends DefaultDirector {
251253
this.onForcedStop("incomplete-report");
252254
const terminal: ReactorAction[] = [
253255
capabilities.checkpoint("subagent-incomplete-report"),
254-
capabilities.reply(forcedStopReport("incomplete-report", this.lastAssistantText)),
256+
capabilities.reply(
257+
forcedStopReport("incomplete-report", this.lastAssistantText, {
258+
paths: salvagePathsFromThrash(this.thrashState),
259+
}),
260+
),
255261
];
256262
this.compaction.noteIdleTurn(event, terminal);
257263
const compacted = this.compaction.interceptActions(event, terminal, capabilities);
@@ -330,11 +336,10 @@ export class SubAgentDirector extends DefaultDirector {
330336
const terminal: ReactorAction[] = [
331337
capabilities.checkpoint("subagent-stalled"),
332338
capabilities.reply(
333-
forcedStopReport(
334-
"stalled",
335-
this.lastAssistantText,
336-
`no activity for ${Math.round(elapsed / 1000)}s after stall nudge`,
337-
),
339+
forcedStopReport("stalled", this.lastAssistantText, {
340+
detail: `no activity for ${Math.round(elapsed / 1000)}s after stall nudge`,
341+
paths: salvagePathsFromThrash(this.thrashState),
342+
}),
338343
),
339344
];
340345
return terminal;

src/subagent/run.ts

Lines changed: 59 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ import {
9090
resolveSubAgentDeadlineMs,
9191
type ForcedStopReason,
9292
} from "./stop-policy.js";
93+
import { EMPTY_THRASH_STATE, nextThrashState, salvagePathsFromThrash } from "./thrash.js";
9394
import { SubAgentDirector } from "./nudge-director.js";
9495
import { assertTierMayMountFleetVerb } from "./authority.js";
9596
import { createReadAgentTraceTool } from "./trace-tool.js";
@@ -268,6 +269,22 @@ function abortReasonText(signal: AbortSignal): string | undefined {
268269
return undefined;
269270
}
270271

272+
/**
273+
* Findings payload for cancel/deadline salvage. Prefer multi-turn accumulated
274+
* prose; fall back to the last turn-boundary text, then the in-flight cycle tail.
275+
*/
276+
function salvageFindingsText(
277+
accumulatedProse: string,
278+
lastPartialText: string,
279+
abortedCycleText: string,
280+
): string {
281+
const prior = accumulatedProse.trim();
282+
if (prior.length > 0) return prior;
283+
const last = lastPartialText.trim();
284+
if (last.length > 0) return last;
285+
return abortedCycleText.slice(-2000);
286+
}
287+
271288
/**
272289
* Arm requireEvidence only for the critique director. Greybeard is also
273290
* intent=review and may spawn-only then envelope; that is not a fake
@@ -770,6 +787,12 @@ export async function runSubAgent(params: RunSubAgentParams): Promise<RunSubAgen
770787
// transcript (which would interleave sub-agent text with the parent turn).
771788
const toolNamesUsed: string[] = [];
772789
let lastPartialText = "";
790+
// Accumulate assistant prose across turns (capped) so cancel/deadline
791+
// salvage Findings keep substantive mid-run text, not only the final cycle.
792+
const TURN_PROSE_CAP = 12_000;
793+
let accumulatedProse = "";
794+
// Thrash paths from tool.start so mid-tool cancel still lists files touched.
795+
let thrashState = EMPTY_THRASH_STATE;
773796
// Watch the streamed text of the in-flight cycle so a salvage on
774797
// cancel/deadline has the cycle's tail as its payload, even though no
775798
// turn boundary has completed yet to carry it.
@@ -780,9 +803,27 @@ export async function runSubAgent(params: RunSubAgentParams): Promise<RunSubAgen
780803
toolNamesUsed.push(name);
781804
params.onProgress?.({ description: params.description, toolName: name });
782805
}
806+
if (event.type === "tool.start") {
807+
const call = (event as { data?: { call?: { name?: unknown; arguments?: unknown } } }).data
808+
?.call;
809+
if (typeof call?.name === "string" && call.name.length > 0) {
810+
thrashState = nextThrashState(thrashState, [
811+
{ type: "tool_call", name: call.name, arguments: call.arguments },
812+
]);
813+
}
814+
}
783815
cycleRecorder.handleEvent(event);
784816
const partial = partialTextFromEvent(event);
785-
if (partial !== null) lastPartialText = partial;
817+
if (partial !== null) {
818+
lastPartialText = partial;
819+
const trimmed = partial.trim();
820+
if (trimmed.length > 0) {
821+
const joined =
822+
accumulatedProse.length === 0 ? trimmed : `${accumulatedProse}\n\n${trimmed}`;
823+
accumulatedProse =
824+
joined.length <= TURN_PROSE_CAP ? joined : joined.slice(-TURN_PROSE_CAP);
825+
}
826+
}
786827
params.onEvent?.(event);
787828
};
788829
streamPromise = consumeStream(agent.stream(), streamSink);
@@ -925,11 +966,13 @@ export async function runSubAgent(params: RunSubAgentParams): Promise<RunSubAgen
925966
if (interruptController.signal.aborted && !runController.signal.aborted) {
926967
interruptedKeepAlive = true;
927968
const abortedCycleText = await cycleRecorder.dispose("cancelled", { drain: streamPromise });
928-
const tail =
929-
lastPartialText.trim().length > 0 ? lastPartialText : abortedCycleText.slice(-2000);
969+
const tail = salvageFindingsText(accumulatedProse, lastPartialText, abortedCycleText);
930970
return {
931971
report: appendActivitySummary(
932-
forcedStopReport("cancelled", tail, "interrupted by interrupt_agent"),
972+
forcedStopReport("cancelled", tail, {
973+
detail: "interrupted by interrupt_agent",
974+
paths: salvagePathsFromThrash(thrashState),
975+
}),
933976
toolNamesUsed,
934977
),
935978
stopReason: "cancelled",
@@ -951,15 +994,17 @@ export async function runSubAgent(params: RunSubAgentParams): Promise<RunSubAgen
951994
// Deadline always salvages (even with zero output). Cancel after any
952995
// tools or assistant prose salvages so the parent keeps partial work;
953996
// pre-progress cancel still surfaces as a bare AbortError.
954-
const hadProgress = toolNamesUsed.length > 0 || lastPartialText.trim().length > 0;
997+
const hadProgress =
998+
toolNamesUsed.length > 0 ||
999+
lastPartialText.trim().length > 0 ||
1000+
accumulatedProse.trim().length > 0;
9551001
const outcome = resolveSubAgentCatchOutcome({
9561002
deadlineHit: runController.deadlineHit(),
9571003
hadProgress,
9581004
});
9591005
if (outcome !== "rethrow") {
9601006
const reason = outcome === "salvage-deadline" ? "deadline" : "cancelled";
961-
const tail =
962-
lastPartialText.trim().length > 0 ? lastPartialText : abortedCycleText.slice(-2000);
1007+
const tail = salvageFindingsText(accumulatedProse, lastPartialText, abortedCycleText);
9631008
const detail =
9641009
reason === "deadline" && resolvedDeadlineMs !== undefined
9651010
? `${resolvedDeadlineMs}ms elapsed`
@@ -971,7 +1016,13 @@ export async function runSubAgent(params: RunSubAgentParams): Promise<RunSubAgen
9711016
...(detail !== undefined ? { detail } : {}),
9721017
});
9731018
return {
974-
report: appendActivitySummary(forcedStopReport(reason, tail, detail), toolNamesUsed),
1019+
report: appendActivitySummary(
1020+
forcedStopReport(reason, tail, {
1021+
...(detail !== undefined ? { detail } : {}),
1022+
paths: salvagePathsFromThrash(thrashState),
1023+
}),
1024+
toolNamesUsed,
1025+
),
9751026
stopReason: reason,
9761027
};
9771028
}

0 commit comments

Comments
 (0)