Skip to content

Commit c4ffba8

Browse files
Stop finished leaves from re-inferring after a report reply (#688)
* Stop finished leaves from re-inferring after a report reply Empty idle-compact and stall continuations were falling through to DefaultDirector.infer after SubAgentDirector already replied with a valid report envelope, which revived the brief about every stall timeout on persist sessions (CL-7068). Fence those empty events once reportReplied is set; only a non-empty parent message re-opens work. * Cover post-report wait paths the fence left untested Salvage, idle-compact meter, and a second empty continuation after a terminal report reply must wait without re-inferring. Prior cases only exercised a single empty ping with no continuation channel.
1 parent af53d5e commit c4ffba8

3 files changed

Lines changed: 164 additions & 5 deletions

File tree

docs/ARCHITECTURE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ The ChatDirector counts consecutive assistant turns that contain tool calls and
130130

131131
#### Sub-agent stall management
132132

133-
`SubAgentDirector` tracks `lastActivityAt`, updated on every real `inference.done` and `tool.done`. Directors are pure `decide(event, ...)` functions with no timer of their own and the reactor has no proactive "idle" event, so a genuinely silent leaf (e.g. parked on a long-running background command with nothing else to do) produces no event for the director to react to. `runSubAgent` (`src/subagent/index.ts`) arms an external interval, at `subAgentStallTimeoutMs`, that pings the same content-less continuation channel the compaction governor uses to re-enter an idle reactor (`requestContinuation`). The director only acts on a ping if the elapsed time since `lastActivityAt` has crossed the timeout — a ping delivered while a tool call is still executing simply queues until that cycle finishes, so "no pending harness-tracked work" falls out of when the check can run at all rather than needing separate bookkeeping. The first stall past the timeout gets one continuation nudge (asking the leaf to check on the background work or report status); a second **consecutive** stall (no activity since that nudge) escalates to the existing salvage path, returning a `stalled` `forcedStopReport` with the same structured shape (summary/findings/blockers) as `turn-budget` and `cancelled`. Any real activity between pings resets the streak, so a leaf that is genuinely working through a slow single turn is never penalized.
133+
`SubAgentDirector` tracks `lastActivityAt`, updated on every real `inference.done` and `tool.done`. Directors are pure `decide(event, ...)` functions with no timer of their own and the reactor has no proactive "idle" event, so a genuinely silent leaf (e.g. parked on a long-running background command with nothing else to do) produces no event for the director to react to. `runSubAgent` (`src/subagent/index.ts`) arms an external interval, at `subAgentStallTimeoutMs`, that pings the same content-less continuation channel the compaction governor uses to re-enter an idle reactor (`requestContinuation`). The director only acts on a ping if the elapsed time since `lastActivityAt` has crossed the timeout — a ping delivered while a tool call is still executing simply queues until that cycle finishes, so "no pending harness-tracked work" falls out of when the check can run at all rather than needing separate bookkeeping. The first stall past the timeout gets one continuation nudge (asking the leaf to check on the background work or report status); a second **consecutive** stall (no activity since that nudge) escalates to the existing salvage path, returning a `stalled` `forcedStopReport` with the same structured shape (summary/findings/blockers) as `turn-budget` and `cancelled`. Any real activity between pings resets the streak, so a leaf that is genuinely working through a slow single turn is never penalized. After the leaf has already replied with a terminal report (complete envelope or salvage), further empty continuations — idle-compact meter sync or stall pings — return `wait` instead of falling through to `DefaultDirector.infer`; only a non-empty parent message (`followup_task` / `send_input`) re-opens the brief.
134134

135135
**Intervention log**: every stop and nudge is appended as one JSONL record to `interventions.jsonl` in the firing leaf's trace dir (`src/subagent/intervention-log.ts`), carrying the trigger's measured value beside the threshold it crossed, the provider/model/family it fired on, and the run state at that moment (turns used vs budget, tool calls, read/edit counts). A refused parent re-dispatch is recorded on the parent side, where no leaf run exists to record it. The parent also appends one `outcome` record per completed dispatch — the salvage kind `classifyBriefSalvage` assigned, or a clean-complete marker, plus the dispatch count — so the log carries dispatch outcomes as well as interventions, and a stop record can later be read alongside what the dispatch it touched actually produced. Writes are fire-and-forget and swallow their own errors — a diagnostic must not be able to fail a run. `scripts/intervention-forensics.ts` aggregates these across local sessions: per-intervention counts by model family, the measured-value distribution against the threshold, two context columns (stops that fired on runs which had already edited files; stops that fired before half the turn budget was spent — neither is a measured false-positive rate, since either is equally consistent with a correct stop or a wrong one), and outcome counts by kind. This exists because every threshold in this tree was set by judgment and four of those judgments were later reverted — a threshold change is expected to cite this data (CL-6938).
136136

src/subagent/nudge-director.test.ts

Lines changed: 127 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ function inferenceDone(
5757
} as unknown as ReactorInboundEvent;
5858
}
5959

60-
function inferenceDoneText(text: string): ReactorInboundEvent {
60+
function inferenceDoneText(text: string, inputTokens = 0): ReactorInboundEvent {
6161
return {
6262
type: "inference.done",
6363
turn: {
@@ -66,7 +66,7 @@ function inferenceDoneText(text: string): ReactorInboundEvent {
6666
timestamp: 0,
6767
content: [{ type: "text", text }],
6868
},
69-
usage: { input: 0, output: 1, cacheRead: 0, cacheWrite: 0, thinking: 0 },
69+
usage: { input: inputTokens, output: 1, cacheRead: 0, cacheWrite: 0, thinking: 0 },
7070
source: { model: "test-model" },
7171
} as unknown as ReactorInboundEvent;
7272
}
@@ -427,3 +427,128 @@ describe("SubAgentDirector incomplete-report wiring", () => {
427427
expect(result.some((action) => action.type === "reply")).toBe(false);
428428
});
429429
});
430+
431+
describe("SubAgentDirector post-complete terminalization (CL-7068)", () => {
432+
test("empty continuation after a valid report reply waits instead of re-inferring", async () => {
433+
const director = new SubAgentDirector("system", [], undefined, 1000);
434+
const caps = capabilities();
435+
436+
await director.decide(inferenceDone(["read-1"]), state, caps);
437+
await director.decide(toolDone("read-1"), state, caps);
438+
const complete = actions(
439+
await director.decide(inferenceDoneText(REPORT_ENVELOPE), state, caps),
440+
);
441+
expect(complete).toContainEqual({ type: "checkpoint", message: "subagent-complete" });
442+
expect(complete.some((action) => action.type === "reply")).toBe(true);
443+
444+
const afterEmpty = actions(await director.decide(messageReceived(""), state, caps));
445+
expect(afterEmpty.some((action) => action.type === "infer")).toBe(false);
446+
expect(afterEmpty.some((action) => action.type === "reply")).toBe(false);
447+
expect(afterEmpty).toContainEqual({ type: "wait" });
448+
});
449+
450+
test("stall empty-ping after a report reply does not revive inference", async () => {
451+
let now = 0;
452+
const director = new SubAgentDirector("system", [], undefined, 1000, () => now);
453+
const caps = capabilities();
454+
455+
await director.decide(inferenceDone(["read-1"]), state, caps);
456+
await director.decide(toolDone("read-1"), state, caps);
457+
await director.decide(inferenceDoneText(REPORT_ENVELOPE), state, caps);
458+
459+
now += 1500;
460+
const afterStall = actions(await director.decide(messageReceived(""), state, caps));
461+
expect(afterStall.some((action) => action.type === "infer")).toBe(false);
462+
expect(afterStall).toContainEqual({ type: "wait" });
463+
expect(afterStall.some((action) => action.type === "checkpoint")).toBe(false);
464+
});
465+
466+
test("a non-empty parent follow-up re-opens inference after a report reply", async () => {
467+
const director = new SubAgentDirector("system", [], undefined, 1000);
468+
const caps = capabilities();
469+
470+
await director.decide(inferenceDone(["read-1"]), state, caps);
471+
await director.decide(toolDone("read-1"), state, caps);
472+
await director.decide(inferenceDoneText(REPORT_ENVELOPE), state, caps);
473+
474+
const followup = actions(
475+
await director.decide(messageReceived("Please also check auth.ts"), state, caps),
476+
);
477+
expect(followup.some((action) => action.type === "infer")).toBe(true);
478+
expect(followup.some((action) => action.type === "wait")).toBe(false);
479+
});
480+
481+
test("empty continuation after incomplete-report-stop salvage waits instead of re-inferring", async () => {
482+
const director = new SubAgentDirector("system", [], undefined, 1000);
483+
const caps = capabilities();
484+
485+
await director.decide(inferenceDone(["read-1"]), state, caps);
486+
await director.decide(toolDone("read-1"), state, caps);
487+
await director.decide(inferenceDoneText("Still looking at the files..."), state, caps);
488+
const salvage = actions(
489+
await director.decide(inferenceDoneText("Still narrating, no envelope."), state, caps),
490+
);
491+
expect(salvage).toContainEqual({ type: "checkpoint", message: "subagent-incomplete-report" });
492+
expect(salvage.some((action) => action.type === "reply")).toBe(true);
493+
494+
const afterEmpty = actions(await director.decide(messageReceived(""), state, caps));
495+
expect(afterEmpty.some((action) => action.type === "infer")).toBe(false);
496+
expect(afterEmpty.some((action) => action.type === "reply")).toBe(false);
497+
expect(afterEmpty).toContainEqual({ type: "wait" });
498+
});
499+
500+
test("idle-compact meter path after a report reply waits instead of re-inferring", async () => {
501+
let continuations = 0;
502+
const director = new SubAgentDirector(
503+
"system",
504+
[],
505+
() => {
506+
continuations++;
507+
},
508+
1000,
509+
);
510+
const caps = capabilities();
511+
512+
// Under-threshold tooling so tool.done does not compact before the report.
513+
await director.decide(inferenceDone(["read-1"]), longState, caps);
514+
await director.decide(toolDone("read-1"), longState, caps);
515+
516+
const complete = actions(
517+
await director.decide(inferenceDoneText(REPORT_ENVELOPE, 999_999), longState, caps),
518+
);
519+
expect(complete).toContainEqual({ type: "checkpoint", message: "subagent-complete" });
520+
expect(complete.some((action) => action.type === "reply")).toBe(true);
521+
// noteIdleTurn arms a continuation so the idle-compact path can run.
522+
expect(continuations).toBe(1);
523+
524+
const compact = actions(await director.decide(messageReceived(""), longState, caps));
525+
expect(compact).toEqual([
526+
{ type: "compact", compactor: "pruning-compactor", reason: "context-threshold" },
527+
]);
528+
expect(continuations).toBe(2);
529+
530+
// Post-compact empty re-entry is meter-only; reportReplied keeps it waiting.
531+
const afterMeter = actions(await director.decide(messageReceived(""), longState, caps));
532+
expect(afterMeter.some((action) => action.type === "infer")).toBe(false);
533+
expect(afterMeter.some((action) => action.type === "reply")).toBe(false);
534+
expect(afterMeter).toContainEqual({ type: "wait" });
535+
});
536+
537+
test("repeated empty continuations after a report reply keep waiting", async () => {
538+
const director = new SubAgentDirector("system", [], undefined, 1000);
539+
const caps = capabilities();
540+
541+
await director.decide(inferenceDone(["read-1"]), state, caps);
542+
await director.decide(toolDone("read-1"), state, caps);
543+
await director.decide(inferenceDoneText(REPORT_ENVELOPE), state, caps);
544+
545+
const firstEmpty = actions(await director.decide(messageReceived(""), state, caps));
546+
expect(firstEmpty.some((action) => action.type === "infer")).toBe(false);
547+
expect(firstEmpty).toContainEqual({ type: "wait" });
548+
549+
const secondEmpty = actions(await director.decide(messageReceived(""), state, caps));
550+
expect(secondEmpty.some((action) => action.type === "infer")).toBe(false);
551+
expect(secondEmpty.some((action) => action.type === "reply")).toBe(false);
552+
expect(secondEmpty).toContainEqual({ type: "wait" });
553+
});
554+
});

src/subagent/nudge-director.ts

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,18 @@ function withEphemeralNudge(
6969
return { ...(options ?? {}), ephemeralTurns: [ephemeralNudgeTurn(text)] };
7070
}
7171

72+
function isEmptyContinuation(event: ReactorInboundEvent): boolean {
73+
if (event.type !== "message.received") return false;
74+
const content = event.message.content;
75+
return typeof content === "string" && content.length === 0;
76+
}
77+
78+
function isNonEmptyParentMessage(event: ReactorInboundEvent): boolean {
79+
if (event.type !== "message.received") return false;
80+
const content = event.message.content;
81+
return typeof content === "string" && content.length > 0;
82+
}
83+
7284
export class SubAgentDirector extends DefaultDirector {
7385
private readonly compaction: CompactionGovernor;
7486
/** When true (CritiqueDirector), empty readCounts is not a successful complete. */
@@ -95,6 +107,13 @@ export class SubAgentDirector extends DefaultDirector {
95107
// (MAX_TOOLLESS_NARRATION_CYCLES = 2).
96108
private toolLessNarrationCycles = 0;
97109

110+
// Once this leaf has replied with a terminal report (complete envelope or
111+
// salvage), empty continuations from idle-compact / stall must not fall
112+
// through to DefaultDirector.infer — that re-opens the brief without a new
113+
// parent message (CL-7068). Cleared only by a non-empty parent message
114+
// (followup_task / send_input).
115+
private reportReplied = false;
116+
98117
// Stall management: a leaf that goes quiet (e.g. parked on a long-running
99118
// background command with nothing else to do) produces no inbound events
100119
// for the director to react to. The reactor has no proactive "idle" event
@@ -166,14 +185,20 @@ export class SubAgentDirector extends DefaultDirector {
166185
state: ReactorState,
167186
capabilities: ReactorCapabilities,
168187
): Promise<ReactorAction | ReactorAction[]> {
188+
// A real parent follow-up re-opens the brief; empty continuations do not.
189+
if (isNonEmptyParentMessage(event)) {
190+
this.reportReplied = false;
191+
}
192+
169193
const afterCompact = this.compaction.resumeAfterCompact(event);
170194
if (afterCompact !== null) {
171195
// Compacted history is the live occupancy until the next provider-
172196
// reported inference.done; paint from the estimate in the meantime.
173197
this.compaction.notePostCompact(state.turns ?? []);
174198
// Idle empty compact only needed the decide re-entry to sync the meter;
175-
// stay idle rather than starting an unprompted inference.
176-
if (afterCompact === "meter") return capabilities.wait();
199+
// stay idle rather than starting an unprompted inference. Same for any
200+
// post-compact resume after this leaf already replied its report.
201+
if (afterCompact === "meter" || this.reportReplied) return capabilities.wait();
177202
return this.applyPendingNudge([capabilities.infer()], capabilities);
178203
}
179204
const idleCompact = this.compaction.interceptIdleContinuation(event, capabilities);
@@ -189,6 +214,12 @@ export class SubAgentDirector extends DefaultDirector {
189214
return recovery;
190215
}
191216

217+
// After a terminal report reply, empty idle-compact / stall pings must
218+
// not reach DefaultDirector (which always infers on message.received).
219+
if (this.reportReplied && isEmptyContinuation(event)) {
220+
return capabilities.wait();
221+
}
222+
192223
const stallOutcome = this.checkStallPing(event, capabilities);
193224
if (stallOutcome !== null) return stallOutcome;
194225

@@ -223,6 +254,7 @@ export class SubAgentDirector extends DefaultDirector {
223254
});
224255

225256
if (stop === "complete") {
257+
this.reportReplied = true;
226258
const terminal: ReactorAction[] = [
227259
capabilities.checkpoint("subagent-complete"),
228260
capabilities.reply(lastText(content)),
@@ -256,6 +288,7 @@ export class SubAgentDirector extends DefaultDirector {
256288
detail: "no report envelope after the wrap-up nudge",
257289
});
258290
this.onForcedStop("incomplete-report");
291+
this.reportReplied = true;
259292
const terminal: ReactorAction[] = [
260293
capabilities.checkpoint("subagent-incomplete-report"),
261294
capabilities.reply(
@@ -338,6 +371,7 @@ export class SubAgentDirector extends DefaultDirector {
338371
detail: `no activity for ${Math.round(elapsed / 1000)}s after stall nudge`,
339372
});
340373
this.onForcedStop("stalled");
374+
this.reportReplied = true;
341375
const terminal: ReactorAction[] = [
342376
capabilities.checkpoint("subagent-stalled"),
343377
capabilities.reply(

0 commit comments

Comments
 (0)