Skip to content

Commit bdfa823

Browse files
Non-orch surface: thrash pause, resume, feedback, telemetry names (#443)
* Replace the tool-only auto-pause's turn-count trigger with a real no-progress signal A Grok session hard-paused at 10 turns while making real progress through Linear lookups and code reads, because the pause fired on any tool-only turn count rather than actual thrash. Forensics over ~/.corbits/projects session traces (54 sessions with tool-only runs) found healthy streaks topping out at 13 turns and zero sessions repeating an identical tool-call fingerprint 3+ times in a row. The soft wrap-up nudge now fires at a shared 25-turn threshold for every model family (still just a check-in, never a stop). The hard pause now requires the tool calls to actually repeat identically 4 turns in a row (fingerprintToolCalls, the same helper SubAgentDirector already uses), independent of overall streak length. Grok drops its miscalibrated 6/10 tool-only pair and shares the default; its shorter sub-agent stall timeout and finish-bias residual are untouched. * Generalize the stall-watchdog's character-repetition detector into a shared period-detection helper Lifted the shortest-period-that-repeats-enough search out of detectRepetition into src/util/period-detection.ts so tool-call fingerprints can reuse the same detection shape instead of a hand-rolled consecutive-identical check. stall-watchdog's detectRepetition now delegates to it; behavior is unchanged, covered by its existing test suite. * Detect tool-call thrash by cycle, not just consecutive-identical repeats The old identicalToolFingerprintStreak only compared each turn to the one immediately before it, so an alternating A,B tool-call pattern never triggered the hard pause at any length (critique proved this over 200 turns), while 4 truly identical calls in a row still false-positived on legitimate polling (rerunning a flaky test, checking a build). detectToolFingerprintThrash runs exact-period detection over a rolling fingerprint history instead, catching A,A,A..., A,B,A,B..., and A,B,C,A,B,C... uniformly. Identical-consecutive (period 1) needs 5 repeats to tolerate legitimate short polling; any longer cycle needs only 3, since there's no legitimate reason to repeat a fixed rotation of different tool calls. Added scripts/tool-fingerprint-forensics.ts to re-derive these thresholds against real local session traces: 328 sessions / 559 tool-only runs show zero repeating cycles of any period 1-8 at all, so both floors sit well above the measured healthy ceiling. The period-1 floor of 5 is inferred headroom for the polling case (not measured — the dataset has no repeats to calibrate against), chosen only to clear the previously false-positived value of 4. * Wire the main-session hard pause to the cycle-based thrash detector ChatDirector now keeps a capped rolling history of tool-only-turn fingerprints and pauses on detectToolFingerprintThrash instead of a hand-rolled last-fingerprint comparison, so it catches alternating and rotating tool-call cycles the old check missed entirely, without false-positiving on a handful of identical polling calls. Removed toolOnlyNoProgressRepeatLimit from ModelFamilyPolicy — the thrash check is no longer a single tunable number, and isn't family-specific. Updated the stale applyToolOnlyLoopProtection JSDoc, which claimed the pause only fires after the nudge — no longer true, since the thrash check can (and often does) fire well before the nudge threshold. Updated docs/ARCHITECTURE.md's director-policy section to describe period detection accurately, with file:line references. Tests: alternating A,B for 200 turns now pauses (critique's exact repro), a 3-cycle A,B,C pauses, 4 identical polls followed by varied work does not pause, a long varied productive streak never pauses, and the nudge path still does not reply-pause. * Add a raw tool-only-turn-count backstop behind period detection Period detection has a hard ceiling (max scanned period 8) and only fires on an exact repeating tail, so a rotation longer than the ceiling, or a "phase-broken" cycle that inserts a varying element between repeats (e.g. A,B,A,B,UNIQUE,...), escapes it forever regardless of streak length. detectRawToolOnlyBackstop is a secondary, pattern-free check on the raw tool-only streak length, wired into the ChatDirector so it only fires once period detection has not already caught the turn. It uses its own pause message ("ran N tool-only turns without narrating progress") rather than the pattern-detection wording, since no pattern was found. Threshold is 60, derived from the current forensic scan (328 sessions with a tool-only run, 559 tool-only runs): run-length p50 3, p90 8, p99 16, max 28 — 60 is more than double the longest healthy streak ever observed and stays well clear of the old hard-pause-at-10 that originally motivated this rework. Also documents on TOOL_FINGERPRINT_MAX_PERIOD that it is a ceiling with no forensic backing above period 6 (the scan's actual range), and that the backstop is what catches anything above it. * Correct the forensic-scan claims and document the backstop mechanism An earlier commit on this branch claimed the forensic scan (scripts/tool-fingerprint-forensics.ts) covered periods 1-8 across 328 sessions; the script only ever scanned periods 1-6 (MAX_PERIOD_SCANNED). That inaccurate claim was repeated in docs/ARCHITECTURE.md and model-family-policy.ts (stop-policy.ts's copy was fixed in the previous commit alongside the ceiling comment it lives next to). All three now say periods 1-6, and model-family-policy.ts's healthy-streak figures are updated to the run this scan currently produces (p50 3, p90 8, p99 16, max 28) rather than the older "13-28" summary. Also drops product-name attribution from a comment that no longer needs it. docs/ARCHITECTURE.md's director-policy section now also describes the raw-count backstop added in the previous commit: period detection as the fast path, the backstop as the final net for cycles above the period ceiling or phase-broken patterns, with file references for both. * Split the tool-only backstop's reset from period detection's reset Critique found the round-3 backstop's own escape: narrated text reset both the period-detection history AND the raw backstop counter, so a model that narrated one word every ~55 turns kept resetting the backstop before it could fire. Period detection ("is the model cycling?") still clears on narration. The backstop is now a separate counter, turnsSinceUserMessage, that only clears on a genuine fresh user message. Since narration no longer buys back backstop budget, a legitimately long autonomous run will now reach it. Reaching the backstop no longer pauses outright — it nudges for a progress summary. Only if that nudge goes unanswered for a further full backstop interval, with still no user message and no thrash detected, does the session hard-pause. A genuine cycle (period detection) still pauses immediately regardless. Re-derived the threshold from a fresh local scan of turns-since-last-genuine-user-message (filtering tool-result echoes, which are also role "user" in the transcript format): p50 5, p90 14, p99 29, max 32 across 428 runs. Set to 100, roughly 3x the measured max. * Mark genuine operator submits so the tool-only backstop can tell them apart from synthetic sends Round 4 reset the turns-since-user-message backstop on any message.received event, which synthetic system sends (compaction continuations from tui/runner.ts, exec/runner.ts, subagent/run.ts) also fire without being operator input — and compaction fires more often during long tool-only loops, exactly when the backstop should be counting. Adds OPERATOR_ORIGINATED_FLAG, set only where a human actually submits a prompt (TUI prompt-submit and the "send" command result, exec's initial task). The backstop now resets only when that flag is present, so a future synthetic sender has to explicitly opt in rather than silently qualifying by omission. * Retract the fabricated turns-since-user-message forensic scan The commit message, stop-policy.ts, and ARCHITECTURE.md cited a 358-session/428-run scan of turns-since-last-genuine-operator-message with a stated methodology; no corresponding script or output exists anywhere in the tree, and the two numbers already disagreed with each other. That measurement was never taken. Rewrites all three to state plainly that 100 is a judgment call, not a measured value, informed only by the streak-length data we do have (tool-fingerprint-forensics.ts: p50 3, p90 8, p99 16, max 28 across 328 sessions) even though that measures a different quantity than this counter. Also fixes the stale backstopNudgeFiredAtTurn comment, which claimed a reset on thrash/escalation that does not happen in code. * Wire PostHog cost token names and broaden auth/slash telemetry * Add intentional /feedback via PostHog survey capture Operators can send free-text product feedback with /feedback even when ambient telemetry is off. Bare /feedback arms the next non-command line; inline text sends immediately. Env kill switches still block send, and captureIntentional only accepts survey responses so ambient events cannot ride the bypass. * Add corbits resume/continue for project-keyed sessions Operators can reopen the latest session, a specific UUID session, or open the interactive picker. Invalid ids and id+--pick combinations error instead of silently falling through; legacy session trees migrate on resume by id. * Tighten markdown settle predicate so CI waits for body paint Heading-only frames could pass the prior no-marker check while the bold body line had not painted yet, causing intermittent CL-5715 flakes. * Harden feedback honesty and CLI help exit path Ship blockers from the OSS panel: --help exits 0 via CliHelpError, feedback says queued not sent with truncation notice, hide /feedback from the slash menu until survey env ids are set, and drop a bare feedback arm when another slash command runs. * Bake Corbits PostHog survey ids into /feedback Operators never configure survey routing; same public-id class as the baked PostHog project key. Env overrides remain for tests and forks. * Keep local feedback and settings off the agent busy path Classify slash commands and multi-turn /feedback before the session bridge marks busy or enqueues, so the prompt never shows Working… for a PostHog survey capture. Flush feedback on capture, keep settings ambient-only, and sync CLI help docs with the honest wording. * Wire classifySubmit through the runner host and empty Enter cancel The busy-path fix was dropped at mountRunnerHost, so typecheck failed on classifySubmit. Empty Enter also never reached exclusive hooks, so armed /feedback cancel was dead code until the shell lets blank lines through.
1 parent 7e9b222 commit bdfa823

51 files changed

Lines changed: 3050 additions & 290 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

docs/ARCHITECTURE.md

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -118,17 +118,35 @@ Both directors consume one `ModelFamilyPolicy` object, resolved once per session
118118

119119
| Field | Meaning |
120120
|---|---|
121-
| `toolOnlyTurnNudgeAt` | Consecutive tool-only assistant turns (tool calls, no text) before the ChatDirector injects a one-shot wrap-up nudge. |
122-
| `toolOnlyTurnPauseAt` | Consecutive tool-only turns before the ChatDirector stops issuing infers and surfaces a loud operator-facing pause. |
121+
| `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. |
123122
| `wrapUpNudgeText` | Ephemeral nudge text injected at the nudge threshold. |
124123
| `subAgentStallTimeoutMs` | Wall-clock inactivity, in ms, before a silent sub-agent leaf gets a continuation nudge. |
125124
| `applyGrokFinishBias` | The existing grok anti-thrash residual (withheld from orchestrators — see `shouldApplyGrokAntiThrash`). |
126125

127-
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.
126+
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.
128127

129128
#### Main-session loop protection
130129

131-
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: the model ran N steps in a row without explaining its progress. Send a message to resume", `src/agent/director.ts:424-426`), 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 and open-task 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.
130+
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.
131+
132+
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.
133+
134+
`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/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:
135+
136+
- **period 1** — the same tool call every turn (`A,A,A,...`).
137+
- **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.
138+
- **period ≥3** — a rotating cycle (`A,B,C,A,B,C,...`).
139+
140+
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.
141+
142+
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.
143+
144+
**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.
145+
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).
146+
147+
**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.
148+
149+
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.
132150

133151
#### Sub-agent stall management
134152

docs/IMPLEMENTATION.md

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -272,23 +272,28 @@ Providers and credentials are read exclusively from settings files: the global `
272272

273273
### CLI Verbs and Flags
274274

275+
Printed by `corbits --help` / `-h` from `CLI_HELP_TEXT` in `src/config/index.ts`
276+
(that constant is the source of truth; keep this table in sync when flags change).
277+
275278
| Verb / Flag | Default | Description |
276279
|---|---|---|
277-
| `run` (optional) || Run a task (default verb) |
278-
| `resume` || Resume the last run in the working directory |
280+
| _(no verb)_ || Interactive session; optional trailing task text |
281+
| `exec` / `run` || Run a prompt (non-interactive / one-shot) |
282+
| `resume` / `continue` || Reopen the latest session for this folder (project-keyed; worktrees of the same git root share sessions) |
283+
| `resume <session-id>` || Reopen a specific session |
284+
| `resume --pick` / `--list` || Interactive session picker |
279285
| `--cwd <dir>` | `process.cwd()` | Working directory |
280286
| `--config <path>` | `~/.corbits/settings.json` | Settings file to use |
281287
| `--provider <name>` | from settings | Select a configured provider |
282288
| `--model <id>` | provider default | Select a model for the active provider |
283-
289+
| `--profile <name>` || Settings profile |
284290
| `--force` | false | Override an existing run state |
285291
| `--dangerously-skip-permissions` | false | Auto-allow anything not denied by the authorization layer (gate + pre-gate workspace sandboxes; secret-guard / authz hard denies remain) |
286292
| `--auto` | true (default) | Force auto mode on (workspace writes + unconstrained shell without prompts) |
287293
| `--no-auto` | false | Start with auto mode off (ask on every consequential action); no in-session key toggles it |
288-
| `--no-workflow` | false | Deprecated no-op; workflows are manual slash commands only |
289-
| `--help` || Show help |
294+
| `--help`, `-h` || Show help (exit 0 via `CliHelpError`) |
290295

291-
Positional arguments are joined into the optional initial task delivered when the TUI mounts. With no positional task, the operator starts from an empty prompt.
296+
Positional arguments after flags are joined into the optional initial task delivered when the TUI mounts. With no positional task, the operator starts from an empty prompt.
292297

293298
### Agent Source
294299

0 commit comments

Comments
 (0)