Tool-only auto-pause should nudge, not hard-stop, on turn count alone

docs/ARCHITECTURE.md

@@ -105,17 +105,35 @@ Both directors consume one `ModelFamilyPolicy` object, resolved once per session
 
 | Field | Meaning |
 |---|---|
-| `toolOnlyTurnNudgeAt` | Consecutive tool-only assistant turns (tool calls, no text) before the ChatDirector injects a one-shot wrap-up nudge. |
-| `toolOnlyTurnPauseAt` | Consecutive tool-only turns before the ChatDirector stops issuing infers and surfaces a loud operator-facing pause. |
+| `toolOnlyTurnNudgeAt` | Consecutive tool-only assistant turns (tool calls, no text) before the ChatDirector injects a one-shot wrap-up nudge — a check-in, not a stop. |
 | `wrapUpNudgeText` | Ephemeral nudge text injected at the nudge threshold. |
 | `subAgentStallTimeoutMs` | Wall-clock inactivity, in ms, before a silent sub-agent leaf gets a continuation nudge. |
 | `applyGrokFinishBias` | The existing grok anti-thrash residual (withheld from orchestrators — see `shouldApplyGrokAntiThrash`). |
 
-Defaults are permissive (12 / 20 turn-only thresholds, 5-minute stall timeout) so a busy-but-progressing session — tool turns interleaved with narration — never trips either mechanism. **Grok** is tightened (6 / 10, 90s) — xAI's own CLI ships the same shape of main-session auto-pause ("Goal auto-paused after N consecutive non-completing turns"), and a directly observed 14-turn pure-tool-call grok session that the operator had to cancel by hand motivated the lower thresholds. **Kimi (Moonshot)** detection ships now (`isKimiLeafProvider`) so callers can already branch on the family, but its thresholds are provisional — pinned to the permissive default with a why-comment in the policy module — pending eval characterization of Kimi's tool-only and stall behavior.
+Defaults (`src/agent/model-family-policy.ts:47`): nudge at 25 consecutive tool-only turns, 5-minute stall timeout. The hard pause is no longer a `ModelFamilyPolicy` field — it runs the same period-detection thrash check for every family (see below). Nudge-at-25 replaced an earlier count-only design (nudge at 12, hard-pause at 20 by count alone, grok tightened to 6/10) that conflated any tool-only turn with no-progress — a Grok session hard-paused at 10 turns while making real progress through Linear lookups and code reads (CL-4839's original loop protection was aimed at runaway list-crawl thrash, not busy-but-progressing tool use). A grep/jq pass over real session traces under `~/.corbits/projects/*/*/context/turns.jsonl` (54 sessions with any tool-only run) found healthy tool-only streaks topping out at 13 turns (p90 12, p99 13) — 25 sits comfortably above that. **Grok** shares the default nudge threshold (its own 6/10 pair was the miscalibration this fixed) but keeps its shorter sub-agent stall timeout (90s) and `applyGrokFinishBias` residual, both independently motivated. **Kimi (Moonshot)** detection ships now (`isKimiLeafProvider`) so callers can already branch on the family, but its thresholds are provisional — pinned to the permissive default with a why-comment in the policy module — pending eval characterization of Kimi's tool-only and stall behavior.
 
 #### Main-session loop protection
 
-The ChatDirector counts consecutive assistant turns that contain tool calls and no text (`toolOnlyStreak`), reset by any turn with text and by every fresh operator message. A dismissed `ask_operator` counts as a no-progress, tool-only turn — the decline path does not reset the streak. At `toolOnlyTurnNudgeAt` the director arms a one-shot ephemeral wrap-up nudge; at `toolOnlyTurnPauseAt` it stops issuing infers entirely and replies with a loud, operator-facing pause message ("Auto-paused after N consecutive tool-only turns... Send a message to resume"), using the same `capabilities.reply()` channel the workflow-stall message already uses to reach the TUI — no new director-to-UI channel was needed. Because a turn with pending `tool_call` blocks must be followed by tool results before anything else (a bare nudge turn on top of pending tool calls is a provider-invalid conversation), both the nudge and the pause are applied by rewriting the `infer` action that follows once those pending tools have resolved — the same one-shot rewrite shape as the sub-agent report-forced wiring below. This loop-protection rewrite runs with the **highest precedence** among the terminal/continuation rewrites in `decideInner`: it is checked before the workflow-idle, open-task, and goal-governor continuation nudges, since those exist to keep a session moving — exactly the behavior the pause guards against. Resuming is just the operator sending a new message, which resets the streak and un-pauses through the same reset path as the other nudge budgets.
+The ChatDirector counts consecutive assistant turns that contain tool calls and no text (`toolOnlyStreak`), reset by any turn with text and by every fresh operator message. A dismissed `ask_operator` counts as a no-progress, tool-only turn — the decline path does not reset the streak. Two independent triggers ride on that streak: at `toolOnlyTurnNudgeAt` the director arms a one-shot ephemeral wrap-up nudge, regardless of what the tool calls were — a long streak of varied, productive tool calls runs straight through it every time.
+
+The hard pause is a separate signal that does **not** depend on the nudge having fired first. The director appends each tool-only turn's fingerprint (`fingerprintToolCalls`, `src/subagent/stop-policy.ts:108`) to a rolling history (`toolFingerprintHistory`, `src/agent/director.ts:356`, capped at `TOOL_FINGERPRINT_HISTORY_CAP` — `src/subagent/stop-policy.ts:184` — so a very long streak doesn't grow the buffer or per-turn scan unbounded) and runs `detectToolFingerprintThrash` (`src/subagent/stop-policy.ts:168`) over it on every turn.
+
+`detectToolFingerprintThrash` is exact-period detection, not a consecutive-identical check: it finds the shortest period `p` such that the tail of the fingerprint history is `p` repeated at least a required number of times (`detectSequencePeriod`, `src/util/period-detection.ts:61` — the same shape as the character-stream repetition detector in `src/tui-opentui/stall-watchdog.ts`'s `detectRepetition`, which now delegates to the same generic helper). This catches three shapes uniformly, where the previous consecutive-identical check only ever caught the first:
+
+- **period 1** — the same tool call every turn (`A,A,A,...`).
+- **period 2** — an alternating pair (`A,B,A,B,...`). The previous implementation compared each turn only to the one immediately before it, so this pattern never triggered at any length.
+- **period ≥3** — a rotating cycle (`A,B,C,A,B,C,...`).
+
+The repeat floor differs by period (`src/subagent/stop-policy.ts:138-157`): period 1 requires 5 repeats (`IDENTICAL_REPEAT_MIN`) — a short run of identical calls is legitimate (rerunning a flaky test, polling a build), and review on CL-5611 found the previous 4-repeat pause false-positived on exactly that. Any cycle of period ≥2 requires only 3 repeats (`CYCLE_REPEAT_MIN`) — there is no plausible legitimate reason to re-issue a fixed rotation of *different* tool calls with identical arguments, so it fires fast (an alternating pair pauses at 6 turns; a 3-call cycle at 9). Both floors are set well above the *measured* healthy ceiling: a local forensic scan (`scripts/tool-fingerprint-forensics.ts`, 328 sessions with a tool-only run, 559 tool-only runs — **this dataset informs the period-detection repeat floors above, not the backstop threshold below, which uses a separate measurement**) found zero occurrences of any repeating cycle for any period the scan checks — periods 1 through 6 (`MAX_PERIOD_SCANNED`); the scan does not check periods 7-8, so `TOOL_FINGERPRINT_MAX_PERIOD` (`src/subagent/stop-policy.ts:138`) has no forensic backing above period 6, only headroom — stronger than CL-5611's original "zero 3+ identical" finding for the periods it does cover. The 5-repeat period-1 floor itself is not independently measured (the forensic dataset contains no repeats to calibrate against); it is inferred headroom for the polling case, chosen only to sit above the previously-false-positived value of 4.
+
+Once `detectToolFingerprintThrash` reports `repeating: true`, the director stops issuing infers entirely and replies with a loud, operator-facing pause message ("Auto-paused: the model repeated the same tool call N times in a row..." for period 1, or "...repeated a P-call cycle N times in a row..." for a longer cycle, both ending "without making progress. Send a message to resume."), using the same `capabilities.reply()` channel the workflow-stall message already uses to reach the TUI. A streak of length 200+ with a different tool call every turn never pauses. Because a turn with pending `tool_call` blocks must be followed by tool results before anything else (a bare nudge turn on top of pending tool calls is a provider-invalid conversation), both the nudge and the pause are applied by rewriting the `infer` action that follows once those pending tools have resolved (`applyToolOnlyLoopProtection`, `src/agent/director.ts:447`) — the same one-shot rewrite shape as the sub-agent report-forced wiring below. This loop-protection rewrite runs with the **highest precedence** among the terminal/continuation rewrites in `decideInner`: it is checked before the workflow-idle, open-task, and goal-governor continuation nudges, since those exist to keep a session moving — exactly the behavior the pause guards against. Resuming is just the operator sending a new message, which resets the streak, the fingerprint history, and un-pauses through the same reset path as the other nudge budgets.
+
+**Backstop: nudge, then escalate — not an immediate pause.** Period detection has a structural blind spot: any period above `TOOL_FINGERPRINT_MAX_PERIOD`, or a "phase-broken" cycle that inserts a varying element between otherwise-repeating windows (e.g. `A,B,A,B,UNIQUE,A,B,A,B,UNIQUE,...`), never settles into an exact repeating tail and so never fires the fast path — at any streak length. Earlier versions of this backstop each had their own escape, all the same shape: the reset condition was satisfiable by something the model or the system itself could trigger. Round 4 fixed the narration escape (a raw tool-only streak that reset on any narrated turn, so a model inserting one word every ~55 turns kept resetting the counter) by separating two questions that had been sharing one reset rule — but its fix reset `turnsSinceUserMessage` on *any* `message.received` event, which is also satisfied by the synthetic content-less messages the runner sends itself after compaction (`buildCompactionContinuationMessage` in `src/tui/runner.ts`, `src/exec/runner.ts`, `src/subagent/run.ts`) — and compaction fires more often during long tool-only loops, i.e. exactly when the backstop should be counting.
+Round 5 fixes the reset condition's shape instead of patching another instance: `turnsSinceUserMessage` now resets only when the inbound message carries `OPERATOR_ORIGINATED_FLAG` (`src/agent/message-provenance.ts`), a flag set only at the genuine human-input submit sites — the TUI's prompt-submit path (`userInboundMessage`, `src/tui/runner.ts`) and exec's initial-task send (`operatorTaskMessage`, `src/exec/runner.ts`). Nothing else sets it, so a message.received event from a synthetic or system-originated send (compaction continuation, retry, future director continuation) is system-originated by default and cannot accidentally qualify — the failure mode inverts from "silently forgets to exclude a sender" to "must explicitly claim to be a human." "Is the model cycling?" (`toolFingerprintHistory` / `lastThrashCheck`) is unaffected by this and is still cleared by any narrated turn — narration remains legitimate evidence the model is not stuck in a tight loop; only the "how long since the operator last saw a real checkpoint?" side (`turnsSinceUserMessage`, `src/agent/director.ts`) requires the operator flag. `detectTurnsSinceUserMessageBackstop` (`src/subagent/stop-policy.ts`) is the secondary/final-net check driven by this counter, evaluated only when period detection has not already reported `repeating: true` on that same turn — so it can never preempt the fast path, only catch what the fast path misses (periods above `TOOL_FINGERPRINT_MAX_PERIOD`, and phase-broken cycles).
+
+**This backstop's threshold (100) is a judgment call, not a measured value.** turns-since-last-genuine-operator-message was never separately measured — an earlier revision of this doc cited a scan of it with a stated methodology and specific percentiles; no corresponding script or output exists anywhere in the tree, and the citation was internally inconsistent about the session/run counts besides. That claim is retracted. The only real measurement available is `scripts/tool-fingerprint-forensics.ts`, which measures a related but different quantity — consecutive tool-only-turn streaks, reset by narration — p50 3, p90 8, p99 16, max 28 across 328 local sessions with a tool-only run. It doesn't directly justify 100 (narration doesn't reset this counter, so the distributions aren't comparable), but it's the only forensic data point on hand, and 100 sits comfortably above every percentile of it.
+
+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.
 
 #### Sub-agent stall management
 

scripts/tool-fingerprint-forensics.ts

@@ -0,0 +1,169 @@
+// Forensic scan over local session traces (~/.corbits/projects/**/context/turns.jsonl)
+// used to re-derive the tool-fingerprint period-detection thresholds in
+// src/subagent/stop-policy.ts (detectToolFingerprintThrash). For every
+// maximal tool-only run (consecutive assistant turns with tool calls and no
+// text) in every local session, finds the largest number of exact repeats
+// observed for each candidate period 1-6, plus run-length percentiles —
+// mirroring the CL-5611 analysis (54 sessions, healthy streaks topping out
+// at 13 turns, zero sessions repeating a fingerprint 3+ times consecutively)
+// but extended to check every period, not just period 1.
+//
+// Run: bun run scripts/tool-fingerprint-forensics.ts
+//
+// Does not print or retain any turn content — only aggregate counts — so it
+// is safe to run without pulling trace data into an LLM context window.
+
+import { readdirSync, statSync, readFileSync } from "node:fs";
+import { join } from "node:path";
+import { homedir } from "node:os";
+
+function stableJson(value: unknown): string {
+  if (value === null || typeof value !== "object") return JSON.stringify(value);
+  if (Array.isArray(value)) return `[${value.map(stableJson).join(",")}]`;
+  const obj = value as Record<string, unknown>;
+  const keys = Object.keys(obj).sort();
+  return `{${keys.map((k) => `${JSON.stringify(k)}:${stableJson(obj[k])}`).join(",")}}`;
+}
+
+function fingerprintToolCalls(content: ReadonlyArray<Record<string, unknown>>): string | null {
+  const parts: string[] = [];
+  for (const block of content) {
+    if (block.type !== "tool_call") continue;
+    const name = typeof block.name === "string" ? block.name : "";
+    let args: unknown = block.arguments ?? {};
+    if (typeof args === "string") {
+      try {
+        args = JSON.parse(args) as unknown;
+      } catch {
+        // keep raw string
+      }
+    }
+    parts.push(`${name}:${stableJson(args)}`);
+  }
+  if (parts.length === 0) return null;
+  parts.sort();
+  return parts.join("|");
+}
+
+function findAll(dir: string, name: string, out: string[]): void {
+  let entries: string[];
+  try {
+    entries = readdirSync(dir);
+  } catch {
+    return;
+  }
+  for (const entry of entries) {
+    const path = join(dir, entry);
+    let info: ReturnType<typeof statSync>;
+    try {
+      info = statSync(path);
+    } catch {
+      continue;
+    }
+    if (info.isDirectory()) findAll(path, name, out);
+    else if (entry === name) out.push(path);
+  }
+}
+
+function periodicSuffixLength(seq: readonly string[], period: number): number {
+  let i = seq.length - 1;
+  let j = i - period;
+  let matched = 0;
+  while (j >= 0 && seq[i] === seq[j]) {
+    matched++;
+    i--;
+    j--;
+  }
+  return matched + period;
+}
+
+function maxRepeatsForPeriod(seq: readonly string[], period: number): number {
+  return Math.floor(periodicSuffixLength(seq, period) / period);
+}
+
+const root = join(homedir(), ".corbits", "projects");
+const files: string[] = [];
+findAll(root, "turns.jsonl", files);
+
+const MAX_PERIOD_SCANNED = 6;
+const periodBest: Record<number, number> = {};
+let sessionsWithToolOnlyRun = 0;
+const runLengths: number[] = [];
+
+for (const file of files) {
+  let lines: string[];
+  try {
+    lines = readFileSync(file, "utf8").split("\n").filter((l) => l.trim().length > 0);
+  } catch {
+    continue;
+  }
+
+  const fingerprints: (string | null)[] = [];
+  for (const line of lines) {
+    let turn: { role?: string; content?: unknown } | undefined;
+    try {
+      turn = JSON.parse(line) as { role?: string; content?: unknown };
+    } catch {
+      continue;
+    }
+    if (turn.role !== "assistant" || !Array.isArray(turn.content)) continue;
+    const content = turn.content as ReadonlyArray<Record<string, unknown>>;
+    const hasToolCalls = content.some((b) => b.type === "tool_call");
+    const hasText = content.some(
+      (b) => b.type === "text" && typeof b.text === "string" && b.text.length > 0,
+    );
+    fingerprints.push(hasToolCalls && !hasText ? fingerprintToolCalls(content) : null);
+  }
+
+  const runs: string[][] = [];
+  let run: string[] = [];
+  for (const fp of fingerprints) {
+    if (fp === null) {
+      if (run.length > 0) runs.push(run);
+      run = [];
+    } else {
+      run.push(fp);
+    }
+  }
+  if (run.length > 0) runs.push(run);
+  if (runs.length > 0) sessionsWithToolOnlyRun++;
+
+  for (const r of runs) {
+    runLengths.push(r.length);
+    for (let end = 1; end <= r.length; end++) {
+      const prefix = r.slice(0, end);
+      for (let period = 1; period <= MAX_PERIOD_SCANNED; period++) {
+        if (prefix.length < period) continue;
+        const reps = maxRepeatsForPeriod(prefix, period);
+        if (reps > (periodBest[period] ?? 0)) periodBest[period] = reps;
+      }
+    }
+  }
+}
+
+runLengths.sort((a, b) => a - b);
+function percentile(p: number): number {
+  if (runLengths.length === 0) return 0;
+  const idx = Math.min(runLengths.length - 1, Math.floor((p / 100) * runLengths.length));
+  return runLengths[idx] as number;
+}
+
+console.log(
+  JSON.stringify(
+    {
+      sessionFilesScanned: files.length,
+      sessionsWithToolOnlyRun,
+      totalToolOnlyRuns: runLengths.length,
+      runLengthP50: percentile(50),
+      runLengthP90: percentile(90),
+      runLengthP99: percentile(99),
+      runLengthMax: runLengths[runLengths.length - 1] ?? 0,
+      // Largest number of exact repeats observed anywhere, for each period.
+      // A value of 1 means "no repeat beyond the base occurrence was ever
+      // observed" at that period.
+      maxRepeatsByPeriod: periodBest,
+    },
+    null,
+    2,
+  ),
+);