Give successful leaf task completions backstop progress credit (#503)

* Give successful leaf task completions backstop progress credit

* Bound leaf-progress backstop resets between operator messages

Author: TheGreatAxios

docs/ARCHITECTURE.md

@@ -148,6 +148,8 @@ Round 5 fixes the reset condition's shape instead of patching another instance:
 
 Because the operator explicitly wants long autonomous runs to keep going, reaching the backstop threshold (`TURNS_SINCE_USER_MESSAGE_BACKSTOP`, 100) does not pause on its own — it fires a one-shot nudge asking the model for a progress summary, the same ephemeral-turn rewrite mechanism as the check-in nudge. Only if that nudge goes unheeded — `turnsSinceUserMessage` advances a further full `TURNS_SINCE_USER_MESSAGE_BACKSTOP` turns with still no user message and no thrash detected — does the director hard-pause, with a distinct message ("Auto-paused: went N turns without a message from the operator, and a progress-summary nudge went unanswered for a further N turns...") tagged `toolOnlyPauseReason: "backstop"` to distinguish it from a thrash pause in logs and messages. A genuine cycle (thrash) still preempts this escalation at any point and pauses immediately, since that is a fast, unambiguous no-progress signal on its own.
 
+**Fleet-heavy work does not falsely trip this (CL-5893).** A primary that is productively blocked on many concurrent/sequential `task` dispatches racks up `turnsSinceUserMessage` at one tool.done→infer cycle per leaf, with no operator message in between — a successful leaf completion (`tool.done` for a `task` call, not a tool error, a string result body, and no salvage-classifiable envelope in the report) re-arms the interval exactly like a fresh operator message would (resetting `turnsSinceUserMessage` and clearing any pending backstop nudge) without being treated as one, so a productive multi-dispatch streak gets meaningfully more room before the backstop can fire. A failed or salvaged leaf completion earns no such credit, so true no-progress tool-only churn still nudges then pauses as above. This reset is bounded, not unlimited: it is capped at `MAX_LEAF_PROGRESS_BACKSTOP_RESETS` (5) leaf-credited resets between genuine operator messages, so an unbroken run of trivial always-succeeding leaf tasks still exhausts the cap and lets the ordinary nudge/pause escalation force an operator checkpoint.
+
 #### 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 `no-progress` / `turn-budget` / `thrash` / `never-acted` / `never-edited`. Any real activity between pings resets the streak, so a leaf that is genuinely working through a slow single turn is never penalized.

src/agent/director.test.ts

@@ -89,6 +89,36 @@ function toolDoneEvent(callId: string): ReactorInboundEvent {
   } as unknown as ReactorInboundEvent;
 }
 
+// A parent turn dispatching a leaf `task` call — varied arguments per id so
+// the fingerprint changes turn to turn (mirrors toolOnlyTurn's shape, but
+// with the tool name pendingTaskCallIds actually tracks).
+function taskTurn(id: string): ReactorInboundEvent {
+  return {
+    type: "inference.done",
+    turn: {
+      role: "assistant",
+      model: "test",
+      timestamp: 0,
+      content: [{ type: "tool_call", id, name: "task", arguments: { prompt: `do ${id}` } }],
+    },
+    usage: { input: 0, output: 0 },
+    source: "test",
+  } as unknown as ReactorInboundEvent;
+}
+
+// A task tool.done result. Defaults to a plain successful completion (no
+// tool error, no salvage-classifiable envelope in the body) — CL-5893's
+// "successful leaf tool.done" progress signal.
+function taskDoneEvent(
+  callId: string,
+  options: { isError?: boolean; content?: string } = {},
+): ReactorInboundEvent {
+  return {
+    type: "tool.done",
+    result: { callId, isError: options.isError ?? false, content: options.content ?? "ok" },
+  } as unknown as ReactorInboundEvent;
+}
+
 // A genuine operator submit — carries OPERATOR_ORIGINATED_FLAG, matching what
 // userInboundMessage() builds at the real TUI/exec prompt-submit sites.
 function messageReceived(content = "hello"): ReactorInboundEvent {
@@ -785,4 +815,194 @@ describe("ChatDirector tool-only loop protection", () => {
     );
     expect(later.some((a) => a.type === "infer")).toBe(true);
   });
+
+  // CL-5893: the primary is productively blocked on a long stream of task
+  // dispatches — each successful leaf completion is progress the operator
+  // will see, so it must re-arm the backstop interval regardless of how many
+  // parent turns (tool.done -> infer cycles) that takes in total.
+  describe("CL-5893: successful leaf task completions re-arm the backstop", () => {
+    test("a back-to-back streak of successful task completions is bounded — the cap exhausts and the nudge/pause escalation eventually fires", async () => {
+      const director = createChatDirector("system", [], {
+        onTasksChange: () => {},
+        provider: providerlessPolicy,
+      });
+      const capabilities = makeCapabilities();
+
+      // Every success here lands one turn after the last reset, so the
+      // MAX_LEAF_PROGRESS_BACKSTOP_RESETS credits are consumed almost
+      // immediately (the worst case for the bound — a genuinely spaced-out
+      // fleet gets far more turns before exhausting the same cap). Once
+      // exhausted, successes stop resetting the interval and the ordinary
+      // nudge (at the 100-turn threshold) then pause (a further 100 turns
+      // unheeded) fire on schedule.
+      let nudgedAt: number | null = null;
+      let pausedAt: number | null = null;
+      for (let i = 0; i < 300 && pausedAt === null; i++) {
+        const id = `task-ok-${i}`;
+        await director.decide(taskTurn(id), mockState, capabilities);
+        const result = actionsArray(await director.decide(taskDoneEvent(id), mockState, capabilities));
+        if (result.some((a) => a.type === "reply" && a.content.includes("Auto-paused"))) {
+          pausedAt = i;
+        } else if (
+          nudgedAt === null &&
+          result.some((a) => a.type === "infer" && ephemeralText(a)?.includes("progress summary"))
+        ) {
+          nudgedAt = i;
+        }
+      }
+
+      expect(nudgedAt).not.toBeNull();
+      expect(pausedAt).not.toBeNull();
+      // A runaway trivial-success loop still pauses — it just gets the cap's
+      // worth of extra headroom first, well past the plain 100-turn
+      // threshold, before the escalation is forced.
+      expect(pausedAt as number).toBeGreaterThan(150);
+    });
+
+    test("an operator message re-arms the full leaf-progress cap", async () => {
+      const director = createChatDirector("system", [], {
+        onTasksChange: () => {},
+        provider: providerlessPolicy,
+      });
+      const capabilities = makeCapabilities();
+
+      // Exhaust the cap with MAX_LEAF_PROGRESS_BACKSTOP_RESETS (5) successes.
+      for (let i = 0; i < 5; i++) {
+        const id = `task-ok-a-${i}`;
+        await director.decide(taskTurn(id), mockState, capabilities);
+        await director.decide(taskDoneEvent(id), mockState, capabilities);
+      }
+
+      await director.decide(messageReceived(), mockState, capabilities);
+
+      // If the cap were not re-armed by the operator message, all 100 of
+      // these would get zero credit and turnsSinceUserMessage would climb
+      // straight to the 100-turn nudge threshold by the last iteration. With
+      // the cap re-armed, the first 5 are credited again (holding the
+      // interval near zero) and the remaining 95 only climb to 95 — no
+      // nudge or pause.
+      let sawPauseOrNudge = false;
+      for (let i = 0; i < 100; i++) {
+        const id = `task-ok-b-${i}`;
+        await director.decide(taskTurn(id), mockState, capabilities);
+        const result = actionsArray(await director.decide(taskDoneEvent(id), mockState, capabilities));
+        if (
+          result.some((a) => a.type === "reply" && a.content.includes("Auto-paused")) ||
+          result.some((a) => a.type === "infer" && ephemeralText(a)?.includes("progress summary"))
+        ) {
+          sawPauseOrNudge = true;
+        }
+      }
+      expect(sawPauseOrNudge).toBe(false);
+    });
+
+    test("non-string tool result content gets no backstop credit — the backstop still nudges then pauses", async () => {
+      const director = createChatDirector("system", [], {
+        onTasksChange: () => {},
+        provider: providerlessPolicy,
+      });
+      const capabilities = makeCapabilities();
+
+      let nudged = false;
+      let paused = false;
+      for (let i = 0; i < 200 && !paused; i++) {
+        const id = `task-nonstring-${i}`;
+        await director.decide(taskTurn(id), mockState, capabilities);
+        const result = actionsArray(
+          await director.decide(
+            { type: "tool.done", result: { callId: id, isError: false, content: undefined } } as unknown as ReactorInboundEvent,
+            mockState,
+            capabilities,
+          ),
+        );
+        if (result.some((a) => a.type === "reply" && a.content.includes("Auto-paused"))) {
+          paused = true;
+        } else if (result.some((a) => a.type === "infer" && ephemeralText(a)?.includes("progress summary"))) {
+          nudged = true;
+        }
+      }
+      expect(nudged).toBe(true);
+      expect(paused).toBe(true);
+    });
+
+    test("periodic successful task completions amid other tool-only turns keep resetting the backstop", async () => {
+      const director = createChatDirector("system", [], {
+        onTasksChange: () => {},
+        provider: providerlessPolicy,
+      });
+      const capabilities = makeCapabilities();
+
+      let sawPauseOrNudge = false;
+      for (let round = 0; round < 5; round++) {
+        // 80 varied tool-only turns per round — below the 100 threshold on
+        // their own, and would accumulate past it across rounds without a
+        // reset.
+        const actions = await runToolOnlyStreak(director, capabilities, 80);
+        if (
+          actions.some((a) => a.type === "reply" && a.content.includes("Auto-paused")) ||
+          actions.some((a) => a.type === "infer" && ephemeralText(a)?.includes("progress summary"))
+        ) {
+          sawPauseOrNudge = true;
+        }
+        // A successful task completion lands at the end of the round and
+        // must reset the interval before the next round starts.
+        const id = `task-round-${round}`;
+        await director.decide(taskTurn(id), mockState, capabilities);
+        await director.decide(taskDoneEvent(id), mockState, capabilities);
+      }
+      expect(sawPauseOrNudge).toBe(false);
+    });
+
+    test("failed task completions get no progress credit — the backstop still nudges then pauses", async () => {
+      const director = createChatDirector("system", [], {
+        onTasksChange: () => {},
+        provider: providerlessPolicy,
+      });
+      const capabilities = makeCapabilities();
+
+      let nudged = false;
+      let paused = false;
+      for (let i = 0; i < 200 && !paused; i++) {
+        const id = `task-fail-${i}`;
+        await director.decide(taskTurn(id), mockState, capabilities);
+        const result = actionsArray(
+          await director.decide(
+            taskDoneEvent(id, { isError: true, content: "boom" }),
+            mockState,
+            capabilities,
+          ),
+        );
+        if (result.some((a) => a.type === "reply" && a.content.includes("Auto-paused"))) {
+          paused = true;
+        } else if (result.some((a) => a.type === "infer" && ephemeralText(a)?.includes("progress summary"))) {
+          nudged = true;
+        }
+      }
+      expect(nudged).toBe(true);
+      expect(paused).toBe(true);
+    });
+
+    test("a task completion without a tool error but carrying a salvage envelope is not counted as progress", async () => {
+      const director = createChatDirector("system", [], {
+        onTasksChange: () => {},
+        provider: providerlessPolicy,
+      });
+      const capabilities = makeCapabilities();
+
+      let paused = false;
+      for (let i = 0; i < 200 && !paused; i++) {
+        const id = `task-salvage-${i}`;
+        await director.decide(taskTurn(id), mockState, capabilities);
+        const result = actionsArray(
+          await director.decide(
+            taskDoneEvent(id, { content: forcedStopReport("no-progress", "x") }),
+            mockState,
+            capabilities,
+          ),
+        );
+        if (result.some((a) => a.type === "reply" && a.content.includes("Auto-paused"))) paused = true;
+      }
+      expect(paused).toBe(true);
+    });
+  });
 });

src/agent/director.ts

@@ -27,6 +27,7 @@ import {
   detectToolFingerprintThrash,
   detectTurnsSinceUserMessageBackstop,
   TURNS_SINCE_USER_MESSAGE_BACKSTOP,
+  MAX_LEAF_PROGRESS_BACKSTOP_RESETS,
   TOOL_FINGERPRINT_HISTORY_CAP,
   type ToolFingerprintThrashCheck,
 } from "../subagent/stop-policy.js";
@@ -413,18 +414,32 @@ class ChatDirectorImpl extends DefaultDirector {
   // synthetic system sends (compaction continuations, retries, future
   // director continuations) fire that event too without being operator
   // input (round-5 fix; see message-provenance.ts for the flag's invariant).
+  // CL-5893: also reset (without being treated as an operator message) by a
+  // successful leaf task tool.done — see the pendingTaskCallIds handling
+  // below — so a parent productively blocked on long-running task calls does
+  // not hard-pause purely from turn volume; a true no-progress tool-only
+  // loop with no successful completions is unaffected.
   // Increments on every turn boundary, tool-only or narrated alike.
   private turnsSinceUserMessage = 0;
   // Set to the turnsSinceUserMessage value at which the backstop nudge fired,
   // so the escalation check can require a full further backstop interval to
   // elapse (still with no user message and no period-detected thrash) before
-  // hard-pausing. Reset to null only on an operator-originated message; it
+  // hard-pausing. Reset to null on an operator-originated message or a
+  // successful leaf task completion (CL-5893); it
   // is NOT reset when thrash detection or the escalation pause fires —
   // pausedForToolOnly and toolOnlyPauseReason are recomputed fresh every
   // turn instead, so a stale non-null value here is harmless once a pause
   // is in effect (the next operator message clears both together).
   private backstopNudgeFiredAtTurn: number | null = null;
   private pendingBackstopNudge = false;
+  // CL-5893: how many times a successful leaf task completion has re-armed
+  // the backstop since the last genuine operator message. Capped at
+  // MAX_LEAF_PROGRESS_BACKSTOP_RESETS so an unbroken run of trivial
+  // always-succeeding leaf tasks cannot reset the backstop forever — once
+  // exhausted, leaf successes stop resetting the interval and the ordinary
+  // nudge/pause escalation proceeds. Reset to 0 only alongside the other
+  // operator-message resets below, never by the leaf-success path itself.
+  private leafProgressBackstopResets = 0;
   // Which mechanism triggered pausedForToolOnly — the period-detection fast
   // path (a recognized cycle) or the backstop escalation (nudge went
   // unheeded for a further full interval with no user message). Drives the
@@ -673,6 +688,7 @@ class ChatDirectorImpl extends DefaultDirector {
         this.turnsSinceUserMessage = 0;
         this.backstopNudgeFiredAtTurn = null;
         this.pendingBackstopNudge = false;
+        this.leafProgressBackstopResets = 0;
         this.salvageNudgeFired = false;
         this.pendingSalvageNudge = null;
         this.pendingTaskCallIds.clear();
@@ -851,6 +867,39 @@ class ChatDirectorImpl extends DefaultDirector {
         this.salvageNudgeFired = true;
         this.pendingSalvageNudge = PRIMARY_SALVAGE_NUDGE;
       }
+      // CL-5893: a parent productively blocked on long-running task calls
+      // racks up turnsSinceUserMessage one tool.done->infer cycle at a time
+      // per leaf, and could hard-pause on fleet-heavy work despite never
+      // actually stalling. A successful leaf completion — no tool error, and
+      // no salvage class at all (not even a soft one like turn-budget or
+      // deadline) — is real progress the operator will see reflected in the
+      // transcript, so it re-arms the backstop interval exactly like a fresh
+      // operator message would, without being treated as one: it does not
+      // touch toolOnlyStreak/toolFingerprintHistory (those track cycling,
+      // which a completed task says nothing about) or salvageNudgeFired.
+      // True no-progress (tool-only churn with no successful leaf completions)
+      // still nudges then pauses exactly as before.
+      //
+      // Bounded (round 2): this reset is capped at
+      // MAX_LEAF_PROGRESS_BACKSTOP_RESETS per operator message so an
+      // unbroken loop of trivial always-succeeding leaf tasks cannot reset
+      // the backstop forever — once the cap is exhausted, leaf successes
+      // stop resetting the interval and the nudge/pause escalation
+      // eventually forces an operator checkpoint. Credit also requires the
+      // tool result content to actually be a string: non-string content is
+      // coerced to "" above only for salvage classification (an empty body
+      // classifies as success), which must not also buy backstop credit.
+      if (
+        !event.result.isError &&
+        salvage === null &&
+        typeof event.result.content === "string" &&
+        this.leafProgressBackstopResets < MAX_LEAF_PROGRESS_BACKSTOP_RESETS
+      ) {
+        this.turnsSinceUserMessage = 0;
+        this.backstopNudgeFiredAtTurn = null;
+        this.pendingBackstopNudge = false;
+        this.leafProgressBackstopResets++;
+      }
     }
 
     if (event.type === "tool.done" && this.workflowCalls.has(event.result.callId)) {