Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/ARCHITECTURE.md
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ The ChatDirector counts consecutive assistant turns that contain tool calls and

#### Sub-agent stall management

`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.
`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.

**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).

Expand Down
129 changes: 127 additions & 2 deletions src/subagent/nudge-director.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ function inferenceDone(
} as unknown as ReactorInboundEvent;
}

function inferenceDoneText(text: string): ReactorInboundEvent {
function inferenceDoneText(text: string, inputTokens = 0): ReactorInboundEvent {
return {
type: "inference.done",
turn: {
Expand All @@ -66,7 +66,7 @@ function inferenceDoneText(text: string): ReactorInboundEvent {
timestamp: 0,
content: [{ type: "text", text }],
},
usage: { input: 0, output: 1, cacheRead: 0, cacheWrite: 0, thinking: 0 },
usage: { input: inputTokens, output: 1, cacheRead: 0, cacheWrite: 0, thinking: 0 },
source: { model: "test-model" },
} as unknown as ReactorInboundEvent;
}
Expand Down Expand Up @@ -427,3 +427,128 @@ describe("SubAgentDirector incomplete-report wiring", () => {
expect(result.some((action) => action.type === "reply")).toBe(false);
});
});

describe("SubAgentDirector post-complete terminalization (CL-7068)", () => {
test("empty continuation after a valid report reply waits instead of re-inferring", async () => {
const director = new SubAgentDirector("system", [], undefined, 1000);
const caps = capabilities();

await director.decide(inferenceDone(["read-1"]), state, caps);
await director.decide(toolDone("read-1"), state, caps);
const complete = actions(
await director.decide(inferenceDoneText(REPORT_ENVELOPE), state, caps),
);
expect(complete).toContainEqual({ type: "checkpoint", message: "subagent-complete" });
expect(complete.some((action) => action.type === "reply")).toBe(true);

const afterEmpty = actions(await director.decide(messageReceived(""), state, caps));
expect(afterEmpty.some((action) => action.type === "infer")).toBe(false);
expect(afterEmpty.some((action) => action.type === "reply")).toBe(false);
expect(afterEmpty).toContainEqual({ type: "wait" });
});

test("stall empty-ping after a report reply does not revive inference", async () => {
let now = 0;
const director = new SubAgentDirector("system", [], undefined, 1000, () => now);
const caps = capabilities();

await director.decide(inferenceDone(["read-1"]), state, caps);
await director.decide(toolDone("read-1"), state, caps);
await director.decide(inferenceDoneText(REPORT_ENVELOPE), state, caps);

now += 1500;
const afterStall = actions(await director.decide(messageReceived(""), state, caps));
expect(afterStall.some((action) => action.type === "infer")).toBe(false);
expect(afterStall).toContainEqual({ type: "wait" });
expect(afterStall.some((action) => action.type === "checkpoint")).toBe(false);
});

test("a non-empty parent follow-up re-opens inference after a report reply", async () => {
const director = new SubAgentDirector("system", [], undefined, 1000);
const caps = capabilities();

await director.decide(inferenceDone(["read-1"]), state, caps);
await director.decide(toolDone("read-1"), state, caps);
await director.decide(inferenceDoneText(REPORT_ENVELOPE), state, caps);

const followup = actions(
await director.decide(messageReceived("Please also check auth.ts"), state, caps),
);
expect(followup.some((action) => action.type === "infer")).toBe(true);
expect(followup.some((action) => action.type === "wait")).toBe(false);
});

test("empty continuation after incomplete-report-stop salvage waits instead of re-inferring", async () => {
const director = new SubAgentDirector("system", [], undefined, 1000);
const caps = capabilities();

await director.decide(inferenceDone(["read-1"]), state, caps);
await director.decide(toolDone("read-1"), state, caps);
await director.decide(inferenceDoneText("Still looking at the files..."), state, caps);
const salvage = actions(
await director.decide(inferenceDoneText("Still narrating, no envelope."), state, caps),
);
expect(salvage).toContainEqual({ type: "checkpoint", message: "subagent-incomplete-report" });
expect(salvage.some((action) => action.type === "reply")).toBe(true);

const afterEmpty = actions(await director.decide(messageReceived(""), state, caps));
expect(afterEmpty.some((action) => action.type === "infer")).toBe(false);
expect(afterEmpty.some((action) => action.type === "reply")).toBe(false);
expect(afterEmpty).toContainEqual({ type: "wait" });
});

test("idle-compact meter path after a report reply waits instead of re-inferring", async () => {
let continuations = 0;
const director = new SubAgentDirector(
"system",
[],
() => {
continuations++;
},
1000,
);
const caps = capabilities();

// Under-threshold tooling so tool.done does not compact before the report.
await director.decide(inferenceDone(["read-1"]), longState, caps);
await director.decide(toolDone("read-1"), longState, caps);

const complete = actions(
await director.decide(inferenceDoneText(REPORT_ENVELOPE, 999_999), longState, caps),
);
expect(complete).toContainEqual({ type: "checkpoint", message: "subagent-complete" });
expect(complete.some((action) => action.type === "reply")).toBe(true);
// noteIdleTurn arms a continuation so the idle-compact path can run.
expect(continuations).toBe(1);

const compact = actions(await director.decide(messageReceived(""), longState, caps));
expect(compact).toEqual([
{ type: "compact", compactor: "pruning-compactor", reason: "context-threshold" },
]);
expect(continuations).toBe(2);

// Post-compact empty re-entry is meter-only; reportReplied keeps it waiting.
const afterMeter = actions(await director.decide(messageReceived(""), longState, caps));
expect(afterMeter.some((action) => action.type === "infer")).toBe(false);
expect(afterMeter.some((action) => action.type === "reply")).toBe(false);
expect(afterMeter).toContainEqual({ type: "wait" });
});

test("repeated empty continuations after a report reply keep waiting", async () => {
const director = new SubAgentDirector("system", [], undefined, 1000);
const caps = capabilities();

await director.decide(inferenceDone(["read-1"]), state, caps);
await director.decide(toolDone("read-1"), state, caps);
await director.decide(inferenceDoneText(REPORT_ENVELOPE), state, caps);

const firstEmpty = actions(await director.decide(messageReceived(""), state, caps));
expect(firstEmpty.some((action) => action.type === "infer")).toBe(false);
expect(firstEmpty).toContainEqual({ type: "wait" });

const secondEmpty = actions(await director.decide(messageReceived(""), state, caps));
expect(secondEmpty.some((action) => action.type === "infer")).toBe(false);
expect(secondEmpty.some((action) => action.type === "reply")).toBe(false);
expect(secondEmpty).toContainEqual({ type: "wait" });
});
});
38 changes: 36 additions & 2 deletions src/subagent/nudge-director.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,18 @@ function withEphemeralNudge(
return { ...(options ?? {}), ephemeralTurns: [ephemeralNudgeTurn(text)] };
}

function isEmptyContinuation(event: ReactorInboundEvent): boolean {
if (event.type !== "message.received") return false;
const content = event.message.content;
return typeof content === "string" && content.length === 0;
}

function isNonEmptyParentMessage(event: ReactorInboundEvent): boolean {
if (event.type !== "message.received") return false;
const content = event.message.content;
return typeof content === "string" && content.length > 0;
}

export class SubAgentDirector extends DefaultDirector {
private readonly compaction: CompactionGovernor;
/** When true (CritiqueDirector), empty readCounts is not a successful complete. */
Expand All @@ -95,6 +107,13 @@ export class SubAgentDirector extends DefaultDirector {
// (MAX_TOOLLESS_NARRATION_CYCLES = 2).
private toolLessNarrationCycles = 0;

// Once this leaf has replied with a terminal report (complete envelope or
// salvage), empty continuations from idle-compact / stall must not fall
// through to DefaultDirector.infer — that re-opens the brief without a new
// parent message (CL-7068). Cleared only by a non-empty parent message
// (followup_task / send_input).
private reportReplied = false;

// 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
// for the director to react to. The reactor has no proactive "idle" event
Expand Down Expand Up @@ -166,14 +185,20 @@ export class SubAgentDirector extends DefaultDirector {
state: ReactorState,
capabilities: ReactorCapabilities,
): Promise<ReactorAction | ReactorAction[]> {
// A real parent follow-up re-opens the brief; empty continuations do not.
if (isNonEmptyParentMessage(event)) {
this.reportReplied = false;
}

const afterCompact = this.compaction.resumeAfterCompact(event);
if (afterCompact !== null) {
// Compacted history is the live occupancy until the next provider-
// reported inference.done; paint from the estimate in the meantime.
this.compaction.notePostCompact(state.turns ?? []);
// Idle empty compact only needed the decide re-entry to sync the meter;
// stay idle rather than starting an unprompted inference.
if (afterCompact === "meter") return capabilities.wait();
// stay idle rather than starting an unprompted inference. Same for any
// post-compact resume after this leaf already replied its report.
if (afterCompact === "meter" || this.reportReplied) return capabilities.wait();
return this.applyPendingNudge([capabilities.infer()], capabilities);
}
const idleCompact = this.compaction.interceptIdleContinuation(event, capabilities);
Expand All @@ -189,6 +214,12 @@ export class SubAgentDirector extends DefaultDirector {
return recovery;
}

// After a terminal report reply, empty idle-compact / stall pings must
// not reach DefaultDirector (which always infers on message.received).
if (this.reportReplied && isEmptyContinuation(event)) {
return capabilities.wait();
}

const stallOutcome = this.checkStallPing(event, capabilities);
if (stallOutcome !== null) return stallOutcome;

Expand Down Expand Up @@ -223,6 +254,7 @@ export class SubAgentDirector extends DefaultDirector {
});

if (stop === "complete") {
this.reportReplied = true;
const terminal: ReactorAction[] = [
capabilities.checkpoint("subagent-complete"),
capabilities.reply(lastText(content)),
Expand Down Expand Up @@ -256,6 +288,7 @@ export class SubAgentDirector extends DefaultDirector {
detail: "no report envelope after the wrap-up nudge",
});
this.onForcedStop("incomplete-report");
this.reportReplied = true;
const terminal: ReactorAction[] = [
capabilities.checkpoint("subagent-incomplete-report"),
capabilities.reply(
Expand Down Expand Up @@ -338,6 +371,7 @@ export class SubAgentDirector extends DefaultDirector {
detail: `no activity for ${Math.round(elapsed / 1000)}s after stall nudge`,
});
this.onForcedStop("stalled");
this.reportReplied = true;
const terminal: ReactorAction[] = [
capabilities.checkpoint("subagent-stalled"),
capabilities.reply(
Expand Down
Loading