From 51957e2ed4487aa8d3e9904df349974ab62c9fb3 Mon Sep 17 00:00:00 2001 From: Michael Grundberg Date: Sun, 6 Sep 2026 17:41:01 +0200 Subject: [PATCH] fix(session): let each CLI declare how its own pane shows work MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Codex session reported `isWorking: false` for its entire life, including mid-turn. Codeman has four paths that mark a session working, and all four were inert for Codex: - The spinner fast path tests eight braille frames, and Codex animates none. - The activity-streak fallback was wrapped in `!isExternalCliMode(mode)`. - The pane probe inside `_confirmIdle` would have matched, since Codex prints `esc to interrupt`, but arming it required the literal glyph `❯` and Codex draws `›` on its composer row. - The text detector sat inside `_processExpensiveParsers`, whose first statement returns early for an external CLI. Add an optional `workDetect: { promptGlyph, workingLine }` to CliCapabilities, so the two strings that differ per CLI are registry data rather than constants in the detector. Claude declares its existing pair and behaves as before. Codex declares `›` and `esc to interrupt`. The text detector moves above the external-CLI early return, guarded on the descriptor so a CLI without one still skips the ANSI strip that the early return used to save it. A CLI that declares no descriptor falls back to Claude's pair, and the activity-streak gate now reads "has a descriptor, or is not external", so the plain shell mode keeps the behaviour it had. Rewrite the test that asserted the old premise in its own comment, so it makes the same guarantee for a genuinely uncharacterised CLI, and add Codex coverage built from verbatim pane captures on Codex CLI 0.152.1. --- src/config/cli-registry/schema.ts | 20 ++++++++ src/config/cli-registry/stock.ts | 12 +++++ src/config/cli-registry/types.ts | 23 +++++++++ src/session.ts | 75 ++++++++++++++++++++-------- test/session-activity.test.ts | 82 ++++++++++++++++++++++++++++--- 5 files changed, 183 insertions(+), 29 deletions(-) diff --git a/src/config/cli-registry/schema.ts b/src/config/cli-registry/schema.ts index c63f9ee7..03bd44e8 100644 --- a/src/config/cli-registry/schema.ts +++ b/src/config/cli-registry/schema.ts @@ -274,6 +274,26 @@ const capabilitiesSchema = z effort: z.boolean(), agentSkillInjection: z.boolean(), statusLineTelemetry: z.boolean(), + workDetect: z + .object({ + promptGlyph: z.string().min(1).max(8), + // Compiled per session, so a broken pattern must fail at LOAD time rather than + // throw inside the PTY data handler. + workingLine: z + .string() + .min(1) + .max(400) + .refine((src) => { + try { + new RegExp(src); + return true; + } catch { + return false; + } + }, 'workingLine must be a valid regular expression'), + }) + .strict() + .optional(), model: z .object({ source: z.enum(['flag', 'claude-settings-file', 'none']), param: z.string().optional() }) .strict(), diff --git a/src/config/cli-registry/stock.ts b/src/config/cli-registry/stock.ts index 1a814073..da1df995 100644 --- a/src/config/cli-registry/stock.ts +++ b/src/config/cli-registry/stock.ts @@ -183,6 +183,13 @@ const CLAUDE: CliEntry = { }, capabilities: { external: false, + // The historical hard-coded pair, now stated as data. `workingLine` matches both the + // `✻ Actualizing… (39s · ↓ 2.0k tokens)` status line and the bare `esc to interrupt` + // footer, because tmux repaints partially and only one of the two may land in a chunk. + workDetect: { + promptGlyph: '❯', + workingLine: String.raw`…\s*\((?:\d+h\s+)?(?:\d+m\s+)?\d+s\b|esc to interrupt`, + }, requiresMux: false, // Claude installs Codeman's own hooks block into every workspace it runs in, so its // stop/idle signals are unconditional — no per-session veto, unlike deepseek's bridge. @@ -415,6 +422,11 @@ const CODEX: CliEntry = { }, capabilities: { ...agentDefaults(), + // Codex draws `› Ask Codex to do anything` on its composer row and + // `Working (2m 49s • esc to interrupt)` above it while a turn runs. It animates no + // braille spinner, and it never prints `esc to interrupt` at rest, so that phrase + // alone separates a running turn from an idle one. + workDetect: { promptGlyph: '›', workingLine: 'esc to interrupt' }, transcript: 'codex-rollout', altScreen: 'strip-full', echo: { policy: 'predict', anchor: { kind: 'cursor' }, predictProfile: 'codex' }, diff --git a/src/config/cli-registry/types.ts b/src/config/cli-registry/types.ts index deb2305b..fb44d8cb 100644 --- a/src/config/cli-registry/types.ts +++ b/src/config/cli-registry/types.ts @@ -306,6 +306,29 @@ export interface CliCapabilities { * independent — see this interface's own doc comment. */ external: boolean; + /** + * How to read this CLI's own TUI for whether it is mid-turn. + * + * Codeman infers a working agent from the pane, so the two strings it needs are the + * ones that differ per CLI: the glyph on the composer row, and the status line the CLI + * draws while a turn runs. Holding them here is what lets a non-Claude CLI report work + * at all — `external` used to gate the whole detector, so every external CLI reported + * itself permanently idle even mid-turn. + * + * `promptGlyph` only ARMS the idle confirmation and is never on its own evidence that a + * turn ended, because a CLI redraws its composer throughout a turn. `workingLine` is + * the evidence, and `_confirmIdle` consults it before believing the pane went quiet. + * + * An entry that omits this field keeps Codeman's historical behaviour: the Claude glyph + * arms the confirmation and the Claude working line answers it. Leave it out for a CLI + * whose TUI nobody has characterised, and its sessions report work exactly as before. + */ + workDetect?: { + /** The glyph this CLI draws on its composer row, e.g. Claude's `❯`, Codex's `›`. */ + promptGlyph: string; + /** Source of a regex matching the status line this CLI draws while a turn runs. */ + workingLine: string; + }; /** No direct-PTY fallback: the CLI must run inside tmux (secrets ride tmux setenv). */ requiresMux: boolean; /** diff --git a/src/session.ts b/src/session.ts index 1598df90..d8c6599e 100644 --- a/src/session.ts +++ b/src/session.ts @@ -465,6 +465,8 @@ export class Session extends EventEmitter { private _activityStreak: ActivityStreak | null = null; // Unbroken run of PTY repaints (working detection) private _lastPaneProbeAt = 0; // Throttle for the tmux screen probe private _lastPaneProbeWorking: boolean | null = null; // Its last verdict (null = could not read) + /** Lazily compiled `capabilities.workDetect.workingLine`. See _workingLinePattern(). */ + private _workingLineRe: RegExp | undefined = undefined; private _trustDialogAccepted: boolean = false; // Stops the trust-dialog scan (answered, or given up) private _trustDialogAttempts = 0; // Keystrokes sent at the trust dialog private _lastTrustDialogScanAt = 0; // Throttle for the trust-dialog screen read @@ -2298,12 +2300,14 @@ export class Session extends EventEmitter { * @param data raw PTY chunk, ANSI included */ private _detectInteractiveActivity(data: string): void { - // The prompt line contains "❯" when Claude is waiting for input. It only ARMS - // the check and is NOT evidence the turn ended: Claude redraws the composer - // about once a second all the way through a turn, which is exactly how a - // working session used to flip to idle two seconds in. _confirmIdle() waits - // for the pane to actually go quiet before believing it. - if (data.includes('❯')) { + const workDetect = getCli(this.mode)?.capabilities.workDetect; + // The composer row carries this glyph when the CLI is waiting for input. It only + // ARMS the check and is NOT evidence the turn ended: a CLI redraws its composer + // about once a second all the way through a turn, which is exactly how a working + // session used to flip to idle two seconds in. _confirmIdle() waits for the pane to + // actually go quiet before believing it. A CLI that declares no glyph keeps Claude's, + // which is the glyph every such session has been armed by until now. + if (data.includes(workDetect?.promptGlyph ?? '❯')) { // Only start a new timeout if we're not already awaiting idle confirmation. // This prevents status bar redraws (which include the prompt) from resetting it. if (!this._awaitingIdleConfirmation) { @@ -2322,9 +2326,10 @@ export class Session extends EventEmitter { // new status line does not rescue it either (tmux repaints partially, so the // complete line reaches the PTY only every few tens of seconds). An unbroken run // of repaints is the signal that survives. See session-activity.ts for the - // measurement. Claude only: an external CLI's TUI has no ❯, so nothing would - // ever arm the idle confirmation and such a session would latch busy forever. - if (!isExternalCliMode(this.mode)) { + // measurement. This needs a pane Codeman can read: without a glyph to arm the idle + // confirmation, a session latches busy forever. A CLI that declares work detection + // supplies its own glyph, and the non-external modes keep the run they always had. + if (workDetect || !isExternalCliMode(this.mode)) { this._activityStreak = trackActivityStreak(this._activityStreak, Date.now()); // A streak is the TRIGGER to look, not the verdict: typing into the composer // also produces a steady stream of repaints. The screen settles it, and only @@ -2359,10 +2364,32 @@ export class Session extends EventEmitter { if (now - this._lastPaneProbeAt < PANE_PROBE_MIN_INTERVAL_MS) return this._lastPaneProbeWorking; this._lastPaneProbeAt = now; const text = this._mux.capturePaneText?.(this._muxSession.muxName) ?? null; - this._lastPaneProbeWorking = text === null ? null : CLAUDE_WORKING_LINE_PATTERN.test(text); + this._lastPaneProbeWorking = text === null ? null : this._workingLinePattern().test(text); return this._lastPaneProbeWorking; } + /** + * The regex matching this CLI's "a turn is running" status line. + * + * Compiled once per session and cached: `_probePaneWorking` runs it against a whole + * pane capture on a timer, and the throttled text detector runs it against every + * accumulated chunk. A CLI that declares no pattern falls back to Claude's, which is + * the pattern every session used before the registry carried one. + */ + private _workingLinePattern(): RegExp { + if (this._workingLineRe === undefined) { + const src = getCli(this.mode)?.capabilities.workDetect?.workingLine; + // The schema validates `workingLine` at load time, so a throw here would mean a + // registry that never loaded. Falling back beats taking the session down. + try { + this._workingLineRe = src ? new RegExp(src) : CLAUDE_WORKING_LINE_PATTERN; + } catch { + this._workingLineRe = CLAUDE_WORKING_LINE_PATTERN; + } + } + return this._workingLineRe; + } + /** * Mark the pane as working. Idempotent: `working` is emitted on the transition * only, so the per-chunk detectors can all call it freely. @@ -2451,10 +2478,6 @@ export class Session extends EventEmitter { * PTY data chunk. Receives accumulated raw data to process in one batch. */ private _processExpensiveParsers(rawData: string): void { - // Skip Claude-specific parsers for external CLI sessions (Ralph tracker, - // BashToolParser, token + CLI-info parsing all depend on Claude's output format). - if (isExternalCliMode(this.mode)) return; - // Lazy ANSI strip: only compute cleanData when a consumer actually needs it. let _cleanData: string | null = null; const getCleanData = (): string => { @@ -2464,6 +2487,19 @@ export class Session extends EventEmitter { return _cleanData; }; + // Work detection by status line, ahead of the external-CLI gate below. The pattern + // comes from the CLI's own registry entry, so this is the one parser here that is not + // Claude-specific — and it sat under that gate, which is why an external CLI reported + // itself idle through an entire turn. Guarded on the descriptor so a CLI without one + // still skips the ANSI strip the gate used to save it. + if (!this._isWorking && getCli(this.mode)?.capabilities.workDetect) { + if (this._workingLinePattern().test(getCleanData())) this._markWorking(); + } + + // Skip Claude-specific parsers for external CLI sessions (Ralph tracker, + // BashToolParser, token + CLI-info parsing all depend on Claude's output format). + if (isExternalCliMode(this.mode)) return; + // Forward to Ralph tracker to detect Ralph loops and todos // (opencode sessions already returned early at line 1209) if (this._ralphTracker.enabled || !this._ralphTracker.autoEnableDisabled) { @@ -2495,16 +2531,13 @@ export class Session extends EventEmitter { this.parseTaskDescriptionsFromTerminalData(getCleanData()); } - // Work detection (text-based, needs clean data: the status line is coloured, - // so raw data has escape sequences between the `…` and the elapsed timer). - // Only check if a faster path didn't already trigger working state. + // Legacy gerunds, Claude-only. The status-line pattern above already ran for every + // CLI that declares one, so this adds only the older wording. Current Claude + // randomizes the word ("Actualizing…", "Finagling…"), so these catch a fraction of + // turns; the pattern above and the activity streak carry the rest. if (!this._isWorking) { const cleanData = getCleanData(); if ( - CLAUDE_WORKING_LINE_PATTERN.test(cleanData) || - // Legacy gerunds. Current Claude randomizes the word ("Actualizing…", - // "Finagling…"), so these catch only a fraction of turns; the pattern - // above and the activity streak carry the rest. cleanData.includes('Thinking') || cleanData.includes('Writing') || cleanData.includes('Reading') || diff --git a/test/session-activity.test.ts b/test/session-activity.test.ts index f67f45e3..dcda3bb4 100644 --- a/test/session-activity.test.ts +++ b/test/session-activity.test.ts @@ -1,5 +1,5 @@ /** - * Working/idle detection for an interactive Claude pane. + * Working/idle detection for an interactive agent pane, Claude's and Codex's. * * The bug this pins: Claude redraws the composer (`❯`) about once a second all * the way through a turn, so the old "saw a ❯, wait 2s, call it idle" rule @@ -7,11 +7,17 @@ * worker: `GET /api/sessions` reported `idle` for a session that had been * running for 17 minutes and was mid-tool-call. * + * A second bug this pins: work detection read Claude's glyph and Claude's status line + * for every CLI, so a Codex session reported itself idle through an entire turn. Each CLI + * now names its own pair in `capabilities.workDetect`, and a CLI that names none reports + * work exactly as before. + * * The status-line fixtures below are verbatim captures from live panes - * (`tmux -L codeman capture-pane -p`) on Claude Code 2.1.220. + * (`tmux -L codeman capture-pane -p`) on Claude Code 2.1.220 and Codex CLI 0.152.1. */ import { describe, expect, it, vi, afterEach } from 'vitest'; import { Session } from '../src/session.js'; +import { getCli } from '../src/config/cli-registry/index.js'; import { CLAUDE_WORKING_LINE_PATTERN } from '../src/utils/regex-patterns.js'; import { trackActivityStreak, @@ -38,7 +44,7 @@ function feed(session: Session, data: string): void { * A session whose mux reports a fixed (or scripted) screen, so the pane probe has * something to read. Only `capturePaneText` is exercised by these paths. */ -function withFakePane(screen: string | (() => string)): Session { +function withFakePane(screen: string | (() => string), mode: 'claude' | 'codex' = 'claude'): Session { const read = typeof screen === 'function' ? screen : () => screen; const mux = { isAvailable: () => true, @@ -46,12 +52,26 @@ function withFakePane(screen: string | (() => string)): Session { } as unknown as NonNullable[0]>['mux']; return new Session({ workingDir: '/tmp', - mode: 'claude', + mode, mux, muxSession: { muxName: 'codeman-test', sessionId: 'test', createdAt: Date.now() }, } as ConstructorParameters[0]); } +/** + * Codex's pane, verbatim, while a turn runs and once it has finished. Codex draws `›` on + * its composer row through the whole turn, exactly as Claude draws `❯`, and prints + * `esc to interrupt` only while the turn is live. + */ +const CODEX_WORKING = + 'Working (2m 49s • esc to interrupt)\n› Ask Codex to do anything\n' + + ' gpt-5.6-sol high · Context 59% left · ~/innovi/irisplus-ent-2 · main\n'; +const CODEX_FINISHED = + '─ Worked for 3m 47s ────────────────────\n› Ask Codex to do anything\n' + + ' gpt-5.6-sol high · Context 57% left · ~/innovi/irisplus-ent-2 · main\n'; +/** Codex's own composer repaint, the frame that arms the idle confirmation. */ +const CODEX_COMPOSER_REPAINT = '\x1b[31;1H\x1b[38;5;246m›\xa0\x1b[39m\x1b[0m'; + /** A composer repaint: the frame Claude ships roughly once a second while working. */ const COMPOSER_REPAINT = '\x1b[31;1H\x1b[38;5;246m❯\xa0\x1b[39m\x1b[0m\x1b[33;1H \x1b[38;5;246mOpus 5 in:143,699 out:669 ctx:14%\x1b[39m'; @@ -230,11 +250,13 @@ describe('Session interactive idle detection', () => { expect(session.status).toBe('idle'); }); - it('does not mark an external CLI pane working off raw activity', () => { + it('does not mark an uncharacterised CLI working off raw activity', () => { vi.useFakeTimers(); - // Codex/Gemini/OpenCode render their own TUIs and have no ❯, so nothing would - // arm the idle confirmation, so a session marked working here would never recover. - const session = new Session({ workingDir: '/tmp', mode: 'codex' }); + // Gemini and OpenCode render their own TUIs, and Codeman knows neither one's glyph, + // so nothing would arm the idle confirmation and a session marked working here would + // never recover. A CLI that names no glyph therefore reports no work at all. + expect(getCli('gemini')?.capabilities.workDetect).toBeUndefined(); + const session = new Session({ workingDir: '/tmp', mode: 'gemini' }); const events: string[] = []; session.on('working', () => events.push('working')); @@ -245,6 +267,50 @@ describe('Session interactive idle detection', () => { expect(events).toEqual([]); }); + + it('marks a Codex pane working, and lets the turn end', () => { + vi.useFakeTimers(); + let screen = CODEX_WORKING; + const session = withFakePane(() => screen, 'codex'); + const events: string[] = []; + session.on('working', () => events.push('working')); + session.on('idle', () => events.push('idle')); + + for (let i = 0; i < 3; i++) { + feed(session, CODEX_COMPOSER_REPAINT); + vi.advanceTimersByTime(1000); + } + vi.advanceTimersByTime(20_000); + + // The old code reported this session idle for the whole turn. + expect(events).toEqual(['working']); + expect(session.status).toBe('busy'); + + // Turn over: the working footer gives way to the finished line, which must NOT + // read as work — it sits on screen for the whole idle period afterwards. + screen = CODEX_FINISHED; + vi.advanceTimersByTime(20_000); + + expect(events).toEqual(['working', 'idle']); + expect(session.status).toBe('idle'); + }); +}); + +describe("codex's work-detection descriptor", () => { + const codex = getCli('codex')?.capabilities.workDetect; + + it('matches the footer Codex prints while a turn runs', () => { + expect(new RegExp(codex!.workingLine).test(CODEX_WORKING)).toBe(true); + }); + + it('does not match the finished line, nor the idle footer', () => { + expect(new RegExp(codex!.workingLine).test(CODEX_FINISHED)).toBe(false); + }); + + it('names the glyph Codex actually draws on its composer row', () => { + expect(CODEX_COMPOSER_REPAINT).toContain(codex!.promptGlyph); + expect(CODEX_WORKING).toContain(codex!.promptGlyph); + }); }); describe('wire activity stamp across recovery', () => {