Skip to content

Commit 4dd0d7c

Browse files
Give stall recovery a full timeout grace after the first nudge (#887)
* Nudge once when assistants print tool-call markup as text Models sometimes emit <tool_call><function=...> wrappers as assistant text instead of real tool_call blocks. Catch that narrow shape, give one corrective nudge per no-real-tool epoch without counting it as incomplete-report narration, then fall through to the existing report policy. Thinking blocks and arbitrary XML stay out of scope; the epoch resets only on genuine tool activity or a parent follow-up. * Let a complete report envelope win over quoted tool-call markup * Give stall recovery a full timeout grace after the first nudge Queued empty stall pings that drain right after the first nudge used to count as a second consecutive stall and stop the leaf immediately. Record stallNudgeAt on the nudge, wait through pings inside stallTimeoutMs without treating them as activity or restarting grace, and stop only once the grace elapses with no tool.done or turn reset. * Cover same-tick stall pings and document the grace window Queued empty continuations with no clock advance must nudge then wait, not salvage. Architecture now describes stallNudgeAt instead of a consecutive-ping streak. * Align product stall copy with the post-nudge grace window * Document the post-nudge stall grace in the changelog
1 parent 91d2105 commit 4dd0d7c

5 files changed

Lines changed: 142 additions & 13 deletions

File tree

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,12 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename
1313

1414
## [Unreleased]
1515

16+
### Fixed
17+
18+
- A stalled worker now gets a full `stallTimeoutMs` grace after the first
19+
continuation nudge before salvage — stall pings queued inside that window
20+
wait instead of counting toward escalation.
21+
1622
### Removed
1723

1824
- `--force` is no longer accepted. It had no runtime effect; resume and the

docs/ARCHITECTURE.md

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

141141
#### Sub-agent stall management
142142

143-
`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 worker (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 worker 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 worker that is genuinely working through a slow single turn is never penalized. After the worker 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 (`resume_agent` / `send_input`) re-opens the brief.
143+
`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 worker (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 records `stallNudgeAt` and issues one continuation nudge (asking the worker to check on the background work or report status). Later empty pings inside `subAgentStallTimeoutMs` of that instant wait without stopping or treating the ping as activity; salvage fires only once a ping arrives after that grace with still no `tool.done` / turn-boundary reset. That grace is what keeps two queued interval ticks from salvaging hundreds of milliseconds after the nudge. Any real activity clears `stallNudgeAt`, so a worker that is genuinely working through a slow single turn is never penalized. After the worker 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 (`resume_agent` / `send_input`) re-opens the brief.
144144

145145
**Intervention log**: every stop and nudge is appended as one JSONL record to `interventions.jsonl` in the firing worker'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 worker 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).
146146

docs/PRODUCT.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ Corbits Code fans work out to short-lived **fleet agents** — workers with thei
172172
- **Tasks** are checklist items owned by one agent via `manage_tasks`.
173173
- **Fleet agents** are spawned with `spawn_agent`. On the TUI primary, mailbox mail arrives as inbound when a worker finishes or fails — do not poll `wait_agents`. Nested orchestrators still collect with `wait_agents`. Workers ask the parent with `ask_director`. That parks a question while the worker stays `running`. Nested `wait_agents` returns `awaiting_director` with a question payload — that is not terminal. The parent answers with `send_input` (`target` = the worker's session id). When the parent TUI is not blocked in `wait_agents`, a parked question arrives as a synthetic idle-send wake. Escalate to the human only with `ask_operator`.
174174

175-
Dispatch uses a structured brief (context / goal / optional goals seed) and returns a structured report. The TUI Agents strip and fleet board show who is running; live tool progress updates the status bar without dumping the child transcript into the parent chat. There is no turn budget. A tool-less final turn completes only with the four-heading report envelope. Printed `<tool_call>` markup in assistant text gets one corrective nudge to issue a real tool call and does not count as the wrap-up; without the envelope, one incomplete-report nudge is given and a second tool-less turn without the envelope salvages as `incomplete-report-stop`. A silent worker (no activity for `stallTimeoutMs`, opt-in) gets one continuation nudge, then salvages as `stalled` if a second consecutive check finds no activity. An opt-in `deadlineMs`, or an operator cancel, can also end a run early. Each of these returns a salvage report so a runaway or idle child cannot quietly burn a large token budget or look done after prose alone.
175+
Dispatch uses a structured brief (context / goal / optional goals seed) and returns a structured report. The TUI Agents strip and fleet board show who is running; live tool progress updates the status bar without dumping the child transcript into the parent chat. There is no turn budget. A tool-less final turn completes only with the four-heading report envelope. Printed `<tool_call>` markup in assistant text gets one corrective nudge to issue a real tool call and does not count as the wrap-up; without the envelope, one incomplete-report nudge is given and a second tool-less turn without the envelope salvages as `incomplete-report-stop`. A silent worker (no activity for `stallTimeoutMs`, opt-in) gets one continuation nudge, then salvages as `stalled` only after a full `stallTimeoutMs` grace with still no activity — queued checks inside that window wait, they do not salvage. An opt-in `deadlineMs`, or an operator cancel, can also end a run early. Each of these returns a salvage report so a runaway or idle child cannot quietly burn a large token budget or look done after prose alone.
176176

177177
## Roadmap (planned, not yet shipped)
178178

src/subagent/nudge-director.test.ts

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1014,6 +1014,117 @@ describe("SubAgentDirector post-complete terminalization (CL-7068)", () => {
10141014
});
10151015
});
10161016

1017+
describe("SubAgentDirector stall nudge grace", () => {
1018+
test("two queued empty pings in the same tick nudge then wait, not stop", async () => {
1019+
let now = 3_000_000;
1020+
const director = new SubAgentDirector(
1021+
"system",
1022+
[],
1023+
undefined,
1024+
1_000,
1025+
() => now,
1026+
);
1027+
const caps = capabilities();
1028+
1029+
await director.decide(inferenceDoneText("working"), state, caps);
1030+
1031+
now += 1_000;
1032+
const first = actions(
1033+
await director.decide(messageReceived(""), state, caps),
1034+
);
1035+
expect(first).toContainEqual({
1036+
type: "checkpoint",
1037+
message: "subagent-stall-nudge",
1038+
});
1039+
expect(first.some((action) => action.type === "reply")).toBe(false);
1040+
1041+
const second = actions(
1042+
await director.decide(messageReceived(""), state, caps),
1043+
);
1044+
expect(second).toEqual([{ type: "wait" }]);
1045+
});
1046+
1047+
test("queued pings inside grace wait; stop only after grace with no activity", async () => {
1048+
let now = 1_000_000;
1049+
const director = new SubAgentDirector(
1050+
"system",
1051+
[],
1052+
undefined,
1053+
1_000,
1054+
() => now,
1055+
);
1056+
const caps = capabilities();
1057+
1058+
await director.decide(inferenceDoneText("working"), state, caps);
1059+
1060+
now += 1_000;
1061+
const first = actions(
1062+
await director.decide(messageReceived(""), state, caps),
1063+
);
1064+
expect(first).toContainEqual({
1065+
type: "checkpoint",
1066+
message: "subagent-stall-nudge",
1067+
});
1068+
1069+
now += 200;
1070+
const midGrace = actions(
1071+
await director.decide(messageReceived(""), state, caps),
1072+
);
1073+
expect(midGrace).toEqual([{ type: "wait" }]);
1074+
1075+
now += 200;
1076+
const stillGrace = actions(
1077+
await director.decide(messageReceived(""), state, caps),
1078+
);
1079+
expect(stillGrace).toEqual([{ type: "wait" }]);
1080+
1081+
now += 600;
1082+
const stopped = actions(
1083+
await director.decide(messageReceived(""), state, caps),
1084+
);
1085+
expect(stopped).toContainEqual({
1086+
type: "checkpoint",
1087+
message: "subagent-stalled",
1088+
});
1089+
expect(stopped.some((action) => action.type === "reply")).toBe(true);
1090+
});
1091+
1092+
test("tool.done during grace clears stallNudgeAt so a later silence nudges again", async () => {
1093+
let now = 2_000_000;
1094+
const director = new SubAgentDirector(
1095+
"system",
1096+
[],
1097+
undefined,
1098+
1_000,
1099+
() => now,
1100+
);
1101+
const caps = capabilities();
1102+
1103+
await director.decide(inferenceDone(["read-1"]), state, caps);
1104+
now += 1_000;
1105+
const first = actions(
1106+
await director.decide(messageReceived(""), state, caps),
1107+
);
1108+
expect(first).toContainEqual({
1109+
type: "checkpoint",
1110+
message: "subagent-stall-nudge",
1111+
});
1112+
1113+
now += 100;
1114+
await director.decide(toolDone("read-1"), state, caps);
1115+
1116+
now += 1_000;
1117+
const afterActivity = actions(
1118+
await director.decide(messageReceived(""), state, caps),
1119+
);
1120+
expect(afterActivity).toContainEqual({
1121+
type: "checkpoint",
1122+
message: "subagent-stall-nudge",
1123+
});
1124+
expect(afterActivity.some((action) => action.type === "reply")).toBe(false);
1125+
});
1126+
});
1127+
10171128
function stubAdmission(
10181129
notes: { provider: string; until: number }[],
10191130
): AdmissionQueue {

src/subagent/nudge-director.ts

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,11 @@ export class SubAgentDirector extends DefaultDirector {
176176
private readonly stallTimeoutMs: number | undefined;
177177
private readonly now: () => number;
178178
private lastActivityAt: number;
179-
private consecutiveStalls = 0;
179+
// Wall clock when the first stall nudge was issued. Later empty pings inside
180+
// stallTimeoutMs of this instant wait without stopping or restarting grace;
181+
// stop only after the grace elapses with no activity. Cleared on real
182+
// tool.done / turn-boundary activity.
183+
private stallNudgeAt: number | undefined;
180184
private lastAssistantText = "";
181185
// Every stop and nudge is recorded with its measured value beside its
182186
// threshold, so a later threshold change can cite data instead of judgment
@@ -303,7 +307,7 @@ export class SubAgentDirector extends DefaultDirector {
303307
if (onTurnBoundary(event)) {
304308
this.lastConsumedNudgeText = null;
305309
this.lastActivityAt = this.now();
306-
this.consecutiveStalls = 0;
310+
this.stallNudgeAt = undefined;
307311
this.compaction.noteInferenceDone(event, state.turns);
308312
this.turnsCompleted++;
309313
const content = event.turn.content as readonly {
@@ -410,7 +414,7 @@ export class SubAgentDirector extends DefaultDirector {
410414
}
411415
if (event.type === "tool.done") {
412416
this.lastActivityAt = this.now();
413-
this.consecutiveStalls = 0;
417+
this.stallNudgeAt = undefined;
414418
if (event.result.isError === true) {
415419
// Failed-tool recovery guidance. Arm once; coalesce consecutive failure
416420
// audits until applyPendingNudge flushes a single counted record.
@@ -438,12 +442,12 @@ export class SubAgentDirector extends DefaultDirector {
438442
* "no pending harness-tracked work" falls out of when this method can run
439443
* at all rather than needing separate bookkeeping.
440444
*
441-
* First stall past the timeout: one continuation nudge, asking the leaf to
442-
* report status or keep going. A second consecutive stall (no activity
443-
* since the nudge) escalates to the existing salvage path, same shape as
444-
* the turn-boundary checks above. Returns null when this event is not
445-
* a stall check the director should act on (let it fall through as an
446-
* ordinary continuation).
445+
* First silence past the timeout: one continuation nudge, and record
446+
* stallNudgeAt. Queued pings that arrive inside the stallTimeoutMs grace
447+
* after that nudge neither stop nor restart the grace (and do not count as
448+
* activity). Stop only when a ping arrives after the grace with still no
449+
* activity. Returns null when this event is not a stall check the director
450+
* should act on (let it fall through as an ordinary continuation).
447451
*/
448452
private checkStallPing(
449453
event: ReactorInboundEvent,
@@ -456,8 +460,8 @@ export class SubAgentDirector extends DefaultDirector {
456460
const elapsed = this.now() - this.lastActivityAt;
457461
if (elapsed < this.stallTimeoutMs) return null;
458462

459-
this.consecutiveStalls++;
460-
if (this.consecutiveStalls === 1) {
463+
if (this.stallNudgeAt === undefined) {
464+
this.stallNudgeAt = this.now();
461465
this.interventions({
462466
id: "stall-nudge",
463467
class: "nudge",
@@ -473,6 +477,14 @@ export class SubAgentDirector extends DefaultDirector {
473477
inferWithSubAgentNudge(capabilities, SUBAGENT_STALL_NUDGE),
474478
];
475479
}
480+
481+
const sinceNudge = this.now() - this.stallNudgeAt;
482+
if (sinceNudge < this.stallTimeoutMs) {
483+
// Still inside the post-nudge grace. Wait without faking activity or
484+
// restarting the grace clock.
485+
return [capabilities.wait()];
486+
}
487+
476488
this.interventions({
477489
id: "stalled",
478490
class: "stop",

0 commit comments

Comments
 (0)