Skip to content

Commit dabb74d

Browse files
Keep cancelled worker Findings and Paths on salvage (#668)
* 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. * Fix prettier formatting in cancelled-worker salvage
1 parent 6efacf8 commit dabb74d

8 files changed

Lines changed: 298 additions & 46 deletions

File tree

src/subagent/index.test.ts

Lines changed: 65 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ import {
2121
classifyBriefSalvage,
2222
EMPTY_THRASH_STATE,
2323
nextThrashState,
24+
salvagePathsFromThrash,
25+
evaluateToolLessNarrationSpiral,
26+
MAX_TOOLLESS_NARRATION_CYCLES,
2427
partialTextFromEvent,
2528
preferCompletedSubAgentReply,
2629
resolveSubAgentCatchOutcome,
@@ -184,6 +187,30 @@ describe("sub-agent stop helpers", () => {
184187
).toBe("incomplete-report-stop");
185188
});
186189

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

398439
test("forcedStopReport renders a Stopped line for display; classification uses the typed reason", () => {
399-
expect(forcedStopReport("cancelled", "partial", "Session closed")).toMatch(
440+
expect(forcedStopReport("cancelled", "partial", { detail: "Session closed" })).toMatch(
400441
/^Stopped: cancelled Session closed\n/,
401442
);
402443
expect(forcedStopReport("cancelled", "partial")).toMatch(/^Stopped: cancelled\n/);
403-
expect(forcedStopReport("deadline", "x", "30s elapsed")).toMatch(
444+
expect(forcedStopReport("deadline", "x", { detail: "30s elapsed" })).toMatch(
404445
/^Stopped: deadline 30s elapsed\n/,
405446
);
406447
// Nested Stopped: under Findings is display-only; classify via typed reason.
407448
const nested = forcedStopReport(
408449
"deadline",
409-
forcedStopReport("cancelled", "inner", "inner reason"),
450+
forcedStopReport("cancelled", "inner", { detail: "inner reason" }),
410451
);
411452
expect(nested).toMatch(/^Stopped: deadline\n/);
412453
expect(nested).toContain("Stopped: cancelled — inner reason");
413454
});
414455

456+
test("salvagePathsFromThrash prefers edited paths then collapses chunked reads", () => {
457+
const state = nextThrashState(EMPTY_THRASH_STATE, [
458+
{
459+
type: "tool_call",
460+
name: "read_file",
461+
arguments: { path: "src/a.ts", offset: 0, limit: 10 },
462+
},
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+
415474
test("createSubAgentRunController aborts on an explicit deadline and reports deadlineHit", async () => {
416475
const ctl = createSubAgentRunController(undefined, 20);
417476
expect(ctl.signal.aborted).toBe(false);

src/subagent/index.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,12 @@ 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 {
25+
EMPTY_THRASH_STATE,
26+
nextThrashState,
27+
salvagePathsFromThrash,
28+
type ThrashState,
29+
} from "./thrash.js";
2530
export {
2631
appendActivitySummary,
2732
buildDispatchBrief,
@@ -36,17 +41,21 @@ export {
3641
} from "./report.js";
3742
export {
3843
SUBAGENT_DEADLINE_MARGIN_MS,
44+
MAX_TOOLLESS_NARRATION_CYCLES,
3945
appendSubAgentParentHints,
4046
evaluateSubAgentStop,
47+
evaluateToolLessNarrationSpiral,
4148
forcedStopReport,
4249
partialTextFromEvent,
4350
preferCompletedSubAgentReply,
4451
resolveSubAgentCatchOutcome,
4552
resolveSubAgentDeadlineMs,
4653
type ForcedStopReason,
54+
type ForcedStopReportOptions,
4755
type SubAgentCatchOutcome,
4856
type SubAgentParentHintOptions,
4957
type SubAgentStopReason,
58+
type ToolLessNarrationSpiral,
5059
} from "./stop-policy.js";
5160

5261
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: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,12 @@ 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 {
19+
EMPTY_THRASH_STATE,
20+
nextThrashState,
21+
salvagePathsFromThrash,
22+
type ThrashState,
23+
} from "./thrash.js";
1924
import { NOOP_INTERVENTION_SINK, type InterventionSink } from "./intervention-log.js";
2025
import {
2126
evaluateSubAgentStop,
@@ -86,8 +91,9 @@ export class SubAgentDirector extends DefaultDirector {
8691
// already completed.
8792
private lastConsumedNudgeText: string | null = null;
8893
// 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;
94+
// narration without the envelope salvages as incomplete-report
95+
// (MAX_TOOLLESS_NARRATION_CYCLES = 2).
96+
private toolLessNarrationCycles = 0;
9197

9298
// Stall management: a leaf that goes quiet (e.g. parked on a long-running
9399
// background command with nothing else to do) produces no inbound events
@@ -213,7 +219,7 @@ export class SubAgentDirector extends DefaultDirector {
213219
thrashState: this.thrashState,
214220
requireEvidence: this.requireEvidence,
215221
lastAssistantText: this.lastAssistantText,
216-
incompleteReportNudgeFired: this.incompleteReportNudgeFired,
222+
toolLessNarrationCycles: this.toolLessNarrationCycles + 1,
217223
});
218224

219225
if (stop === "complete") {
@@ -229,7 +235,7 @@ export class SubAgentDirector extends DefaultDirector {
229235
if (stop === "incomplete-report") {
230236
// Tool-less turn after tools, no report envelope. Must not fall through
231237
// to super.decide — DefaultDirector completes any tool-less turn.
232-
this.incompleteReportNudgeFired = true;
238+
this.toolLessNarrationCycles += 1;
233239
this.interventions({
234240
id: "incomplete-report",
235241
class: "nudge",
@@ -242,6 +248,7 @@ export class SubAgentDirector extends DefaultDirector {
242248
];
243249
}
244250
if (stop === "incomplete-report-stop") {
251+
this.toolLessNarrationCycles += 1;
245252
this.interventions({
246253
id: "incomplete-report-stop",
247254
class: "stop",
@@ -251,7 +258,11 @@ export class SubAgentDirector extends DefaultDirector {
251258
this.onForcedStop("incomplete-report");
252259
const terminal: ReactorAction[] = [
253260
capabilities.checkpoint("subagent-incomplete-report"),
254-
capabilities.reply(forcedStopReport("incomplete-report", this.lastAssistantText)),
261+
capabilities.reply(
262+
forcedStopReport("incomplete-report", this.lastAssistantText, {
263+
paths: salvagePathsFromThrash(this.thrashState),
264+
}),
265+
),
255266
];
256267
this.compaction.noteIdleTurn(event, terminal);
257268
const compacted = this.compaction.interceptActions(event, terminal, capabilities);
@@ -330,11 +341,10 @@ export class SubAgentDirector extends DefaultDirector {
330341
const terminal: ReactorAction[] = [
331342
capabilities.checkpoint("subagent-stalled"),
332343
capabilities.reply(
333-
forcedStopReport(
334-
"stalled",
335-
this.lastAssistantText,
336-
`no activity for ${Math.round(elapsed / 1000)}s after stall nudge`,
337-
),
344+
forcedStopReport("stalled", this.lastAssistantText, {
345+
detail: `no activity for ${Math.round(elapsed / 1000)}s after stall nudge`,
346+
paths: salvagePathsFromThrash(this.thrashState),
347+
}),
338348
),
339349
];
340350
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 critic director. Greybeard is also
273290
* intent=review and may spawn-only then envelope; that is not a fake
@@ -772,6 +789,12 @@ export async function runSubAgent(params: RunSubAgentParams): Promise<RunSubAgen
772789
// transcript (which would interleave sub-agent text with the parent turn).
773790
const toolNamesUsed: string[] = [];
774791
let lastPartialText = "";
792+
// Accumulate assistant prose across turns (capped) so cancel/deadline
793+
// salvage Findings keep substantive mid-run text, not only the final cycle.
794+
const TURN_PROSE_CAP = 12_000;
795+
let accumulatedProse = "";
796+
// Thrash paths from tool.start so mid-tool cancel still lists files touched.
797+
let thrashState = EMPTY_THRASH_STATE;
775798
// Watch the streamed text of the in-flight cycle so a salvage on
776799
// cancel/deadline has the cycle's tail as its payload, even though no
777800
// turn boundary has completed yet to carry it.
@@ -782,9 +805,27 @@ export async function runSubAgent(params: RunSubAgentParams): Promise<RunSubAgen
782805
toolNamesUsed.push(name);
783806
params.onProgress?.({ description: params.description, toolName: name });
784807
}
808+
if (event.type === "tool.start") {
809+
const call = (event as { data?: { call?: { name?: unknown; arguments?: unknown } } }).data
810+
?.call;
811+
if (typeof call?.name === "string" && call.name.length > 0) {
812+
thrashState = nextThrashState(thrashState, [
813+
{ type: "tool_call", name: call.name, arguments: call.arguments },
814+
]);
815+
}
816+
}
785817
cycleRecorder.handleEvent(event);
786818
const partial = partialTextFromEvent(event);
787-
if (partial !== null) lastPartialText = partial;
819+
if (partial !== null) {
820+
lastPartialText = partial;
821+
const trimmed = partial.trim();
822+
if (trimmed.length > 0) {
823+
const joined =
824+
accumulatedProse.length === 0 ? trimmed : `${accumulatedProse}\n\n${trimmed}`;
825+
accumulatedProse =
826+
joined.length <= TURN_PROSE_CAP ? joined : joined.slice(-TURN_PROSE_CAP);
827+
}
828+
}
788829
params.onEvent?.(event);
789830
};
790831
streamPromise = consumeStream(agent.stream(), streamSink);
@@ -927,11 +968,13 @@ export async function runSubAgent(params: RunSubAgentParams): Promise<RunSubAgen
927968
if (interruptController.signal.aborted && !runController.signal.aborted) {
928969
interruptedKeepAlive = true;
929970
const abortedCycleText = await cycleRecorder.dispose("cancelled", { drain: streamPromise });
930-
const tail =
931-
lastPartialText.trim().length > 0 ? lastPartialText : abortedCycleText.slice(-2000);
971+
const tail = salvageFindingsText(accumulatedProse, lastPartialText, abortedCycleText);
932972
return {
933973
report: appendActivitySummary(
934-
forcedStopReport("cancelled", tail, "interrupted by interrupt_agent"),
974+
forcedStopReport("cancelled", tail, {
975+
detail: "interrupted by interrupt_agent",
976+
paths: salvagePathsFromThrash(thrashState),
977+
}),
935978
toolNamesUsed,
936979
),
937980
stopReason: "cancelled",
@@ -953,15 +996,17 @@ export async function runSubAgent(params: RunSubAgentParams): Promise<RunSubAgen
953996
// Deadline always salvages (even with zero output). Cancel after any
954997
// tools or assistant prose salvages so the parent keeps partial work;
955998
// pre-progress cancel still surfaces as a bare AbortError.
956-
const hadProgress = toolNamesUsed.length > 0 || lastPartialText.trim().length > 0;
999+
const hadProgress =
1000+
toolNamesUsed.length > 0 ||
1001+
lastPartialText.trim().length > 0 ||
1002+
accumulatedProse.trim().length > 0;
9571003
const outcome = resolveSubAgentCatchOutcome({
9581004
deadlineHit: runController.deadlineHit(),
9591005
hadProgress,
9601006
});
9611007
if (outcome !== "rethrow") {
9621008
const reason = outcome === "salvage-deadline" ? "deadline" : "cancelled";
963-
const tail =
964-
lastPartialText.trim().length > 0 ? lastPartialText : abortedCycleText.slice(-2000);
1009+
const tail = salvageFindingsText(accumulatedProse, lastPartialText, abortedCycleText);
9651010
const detail =
9661011
reason === "deadline" && resolvedDeadlineMs !== undefined
9671012
? `${resolvedDeadlineMs}ms elapsed`
@@ -973,7 +1018,13 @@ export async function runSubAgent(params: RunSubAgentParams): Promise<RunSubAgen
9731018
...(detail !== undefined ? { detail } : {}),
9741019
});
9751020
return {
976-
report: appendActivitySummary(forcedStopReport(reason, tail, detail), toolNamesUsed),
1021+
report: appendActivitySummary(
1022+
forcedStopReport(reason, tail, {
1023+
...(detail !== undefined ? { detail } : {}),
1024+
paths: salvagePathsFromThrash(thrashState),
1025+
}),
1026+
toolNamesUsed,
1027+
),
9771028
stopReason: reason,
9781029
};
9791030
}

0 commit comments

Comments
 (0)