diff --git a/CLAUDE.md b/CLAUDE.md index adb38400..3a54ebf5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -138,7 +138,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph **Idle detection**: Multi-layer (completion message → AI check → output silence → token stability). See `docs/respawn-state-machine.md`. -⚠️ **A `❯` sighting is NOT the end of a turn, and neither is silence.** Claude redraws the composer all through a turn, and its working line (`✻ Actualizing… (13m 23s · …)`) is invisible to `SPINNER_PATTERN`, keyword lists and the raw stream. `_confirmIdle()` (session.ts) requires the pane to go quiet AND the SCREEN (`capturePaneText()` + the working-line pattern) to agree; a sustained run of repaints (`session-activity.ts`) marks a turn as started. ⚠️ The composer glyph and working line are per-CLI registry DATA (`capabilities.workDetect`), never Claude constants; a CLI declaring neither falls back to Claude's pair. ⚠️ `workingLine` is config-supplied and runs on the PTY hot path, so it must compile through `compileVersionRegex()` in BOTH the schema refine and `_workingLinePattern()` (ReDoS guard; null, not throw). → [architecture-invariants#idle-detection-composer-glyph-and-working-line](docs/architecture-invariants.md#idle-detection-composer-glyph-and-working-line) +⚠️ **A `❯` sighting is NOT the end of a turn, and neither is silence.** Claude redraws the composer all through a turn, and its working line (`✻ Actualizing… (13m 23s · …)`) is invisible to `SPINNER_PATTERN`, keyword lists and the raw stream. `_confirmIdle()` (session.ts) requires the pane to go quiet AND the SCREEN (`capturePaneText()` + the working-line pattern) to agree; a sustained run of repaints (`session-activity.ts`) marks a turn as started. ⚠️ The workspace-trust dialog's `❯` (on `No, exit`) arms the check too, so the same capture holds idle off while `isTrustDialogScreen()` matches inside `TRUST_DIALOG_WINDOW_MS`: an agent that waited for idle and prompted the dialog confirmed `No, exit` and killed the pane. ⚠️ The composer glyph and working line are per-CLI registry DATA (`capabilities.workDetect`), never Claude constants; a CLI declaring neither falls back to Claude's pair. ⚠️ `workingLine` is config-supplied and runs on the PTY hot path, so it must compile through `compileVersionRegex()` in BOTH the schema refine and `_workingLinePattern()` (ReDoS guard; null, not throw). → [architecture-invariants#idle-detection-composer-glyph-and-working-line](docs/architecture-invariants.md#idle-detection-composer-glyph-and-working-line) ⚠️ **A turn that ENDED waiting for its own workers is working, not idle.** Claude closes such a turn with `✻ Waiting for 1 dynamic workflow to finish` (background agents / ultracode) and resumes by itself; `capabilities.workDetect.awaitingLine` makes the idle probe count it as work. ⚠️ Claude never redraws that row, so it stays on screen after the workers finish: test it ONLY as the newest column-0 row above the composer (`isAwaitingWorkers()`), never pane-wide and never on the stream. → [architecture-invariants#idle-detection-composer-glyph-and-working-line](docs/architecture-invariants.md#idle-detection-composer-glyph-and-working-line) diff --git a/docs/api-reference.md b/docs/api-reference.md index db7ac4d2..83e754a4 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -250,9 +250,9 @@ the wait promptly instead of burning the caller's whole timeout on something tha can no longer happen). On a `claude` worker, prefer an explicit `until=stop,exit` once the session is up: the default set's `idle` also resolves on a spinner pause, and on a fresh session the **startup** `idle` (emitted when the CLI first comes up) -can land inside your first wait window and report a turn that never ran. Measured: -a session parked on the trust dialog emits no *further* `idle`, so it is the -startup transition, not the dialog, that produces the false success below. +can land inside your first wait window and report a turn that never ran. That +startup `idle` is held while Claude's workspace-trust dialog is on screen (inside +the 90 s startup window), because the dialog's own ❯ is not a composer. ⚠️ **`exit` means "nothing is running", which includes "not started yet".** The server answers from `pid === null` plus a mux-layer pane-death probe, and that @@ -290,11 +290,11 @@ instead. The same caution applies to the external CLIs. Nothing here reports "the agent is ready for a prompt", and no combination of `until`/`fresh` synthesizes one. A freshly created session reads as `exit` (above), and a `claude` worker in a brand-new case comes up on the CLI's **trust dialog**, -which contains a ❯ prompt of its own. Send-and-wait posted at that moment types the -prompt into the dialog, where the `\r` never gets past it, while the session's -startup `idle` lands inside the wait window: the wait resolves on `idle` in a -couple of seconds with `timedOut: false`, which looks exactly like a completed -turn. +which contains a ❯ of its own. A prompt posted at that moment is typed into the +dialog, and its `\r` confirms the highlighted `No, exit`: the CLI quits and the +pane dies, so send-and-wait resolves on `exit`. Codeman holds the session's `idle` +while the dialog is on screen, so waiting for `idle` before prompting no longer +clears too early, but that still does not make `idle` a readiness signal. The reliable sequence is: poll `GET /api/v1/sessions/:id` until `.data.pid` is non-null, then `wait-output` for the composer's own marker (`bypass`, the status diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index 1836d1bf..5859114f 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -270,6 +270,8 @@ Scoping needs no check of its own here: `setPaneExit()` already forces `paneExit So: `_confirmIdle()` (session.ts) requires the pane to go quiet, and then asks the SCREEN via `capturePaneText()` + `CLAUDE_WORKING_LINE_PATTERN` before believing it; a sustained run of repaints (`session-activity.ts`, pure + unit tested) is what marks a turn as started, with the same screen probe vetoing keystroke echo. Idle now lands ~3-5s after a turn ends instead of 2s into one. +⚠️ **The workspace-trust dialog is not a composer.** Its `❯` sits on `No, exit`, so it arms the confirmation, and the dialog is quiet with no working line, so a fresh case used to be concluded idle about 4 s after launch while the dialog was still up (measured on Claude Code 2.1.294: `GET .../wait?until=idle` answered `idle` at 3.8 s in 6 of 12 fresh-dir launches). An agent that waited for idle and then prompted typed into the dialog, and the prompt's Enter confirmed `No, exit`: the pane died and the wait reported `exit`. `_probePaneWorking()` therefore also records whether its capture shows `isTrustDialogScreen()` (a failed capture keeps the last answer, since it proves nothing), and `_confirmIdle()` re-checks on `PANE_PROBE_RECHECK_MS` instead of concluding while it does, but only inside the auto-accept's own `TRUST_DIALOG_WINDOW_MS`: a later screen that merely quotes the dialog must not hold a session busy. The status is left alone rather than marked working, since nothing is running. Tests: `test/session-trust-dialog.test.ts`. + ⚠️ **The composer glyph and the working line are per-CLI registry DATA** (`capabilities.workDetect`, #385), not Claude constants: claude declares `❯` plus the pattern above, codex declares `›` plus `[Ee]sc to interrupt`, pi declares its composer rule `─` plus the spinner it embeds there, opencode declares its composer bar `┃` plus its 8-cell footer spinner `[⬝■]{8}`, omp declares its input row `╰─` plus the spinner and elapsed time that replace the `π` in its status bar, gemini declares its composer's bottom bar `▀` plus its `(esc to cancel, s)` spinner line, and a CLI that declares neither falls back to Claude's pair, which is what every session used before the registry carried one. Before that, this whole mechanism was gated Claude-mode-only on the reasoning that an external CLI has no `❯`, which was true and still left every Codex session reporting `idle` for its entire life. ⚠️ `workingLine` is config-supplied (a user `clis.json` can set it) and the compiled pattern runs on the PTY hot path, so it goes through `compileVersionRegex()` in BOTH the schema refine and `_workingLinePattern()`: a nested quantifier there is a ReDoS against the event loop, and the helper returns null rather than throwing so the fallback is structural. diff --git a/docs/extending-codeman.md b/docs/extending-codeman.md index fb866056..da838621 100644 --- a/docs/extending-codeman.md +++ b/docs/extending-codeman.md @@ -227,10 +227,10 @@ whether your integration works, and the last one is what actually bites: ⚠️ **There is no readiness signal, and skipping readiness is the failure that looks like success.** A session reports `idle` before its CLI has spawned, and a `claude` worker in a brand-new case comes up on the CLI's **trust dialog**, which has a ❯ -prompt of its own. Prompt it at that moment and the text lands in the dialog, the -`\r` does not get past it, and the session's startup `idle` lands inside the wait -window: the wait resolves on `idle` in a couple of seconds with `timedOut: false`, -indistinguishable from a finished turn. Wait for the pid, then wait for the +prompt of its own. Prompt it at that moment and the text lands in the dialog, +whose highlighted option is `No, exit`, so the prompt's `\r` quits the CLI. Codeman +holds the session's `idle` while the dialog is on screen, which makes waiting for +`idle` safe but still not a readiness signal. Wait for the pid, then wait for the composer, answering the dialog only as the bounded fallback. ⚠️ **Answering it is not "press Enter".** Claude Code 2.1.252 dropped the options' diff --git a/docs/wiki/Driving-Codeman-From-An-Agent.md b/docs/wiki/Driving-Codeman-From-An-Agent.md index 4d62f05b..92a646f2 100644 --- a/docs/wiki/Driving-Codeman-From-An-Agent.md +++ b/docs/wiki/Driving-Codeman-From-An-Agent.md @@ -188,9 +188,9 @@ Read these before writing any code. Each one has cost somebody an afternoon. output marker instead. 7. **Nothing reports "ready", so wait for it explicitly.** A new session answers `{"signal":"exit","immediate":true}` until its PID exists, and that means *not started*, - not *crashed*. A Claude worker in a fresh case then sits on the CLI's trust dialog; prompt - it there and the wait resolves on idle in about two seconds looking exactly like a finished - turn, while your text sits stuck in the dialog. + not *crashed*. A Claude worker in a fresh case then sits on the CLI's trust dialog, whose + highlighted option is `No, exit`: prompt it there and your Enter quits the CLI. Codeman + holds `idle` while the dialog is on screen, but that is not a readiness signal either. ### Recipes diff --git a/src/session.ts b/src/session.ts index 5a2bb677..cc23aed3 100644 --- a/src/session.ts +++ b/src/session.ts @@ -563,6 +563,7 @@ 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) + private _lastPaneProbeTrustDialog = false; // Same capture showed the workspace-trust dialog (see _confirmIdle) /** * Background work the pane's own footer reports, e.g. `1 monitor`; null for none. * @@ -2711,6 +2712,7 @@ export class Session extends EventEmitter { this._interactiveStartedAt = Date.now(); this._trustDialogAttempts = 0; this._lastTrustDialogScanAt = 0; + this._lastPaneProbeTrustDialog = false; // the last pane's verdict, not this one's if (this._trustDialogTimer) { clearTimeout(this._trustDialogTimer); this._trustDialogTimer = null; @@ -3269,6 +3271,18 @@ export class Session extends EventEmitter { // composer is up and the pane is quiet, but the next turn starts without the user. this._lastPaneProbeWorking = text === null ? null : this._workingLinePattern().test(text) || this._paneAwaitsWorkers(text); + // A capture that failed proves nothing either way, so it keeps the last answer. + if (text !== null) { + const trustDialog = isTrustDialogScreen(text); + if ( + trustDialog && + !this._lastPaneProbeTrustDialog && + now - this._interactiveStartedAt <= TRUST_DIALOG_WINDOW_MS + ) { + console.log(`[Session] Holding idle while the workspace trust dialog is on screen: ${this.id}`); + } + this._lastPaneProbeTrustDialog = trustDialog; + } this._readWatching(text); this._readScreenModel(text); return this._lastPaneProbeWorking; @@ -3523,6 +3537,16 @@ export class Session extends EventEmitter { this.activityTimeout = setTimeout(() => this._confirmIdle(), PANE_PROBE_RECHECK_MS); return; } + // ⚠️ The workspace-trust dialog is not a composer, although its `❯` (on "No, exit") + // arms this check and the dialog sits quiet. Calling it idle let an agent that waited + // for idle send its prompt into the dialog, and the prompt's Enter answered *exit*. + // Not working either, so the status stays where it is until the dialog is gone. + // Bounded by the auto-accept's own startup window: later on, a screen that merely + // quotes the dialog (an agent reading session-trust-dialog.ts) must not hold idle off. + if (this._lastPaneProbeTrustDialog && Date.now() - this._interactiveStartedAt <= TRUST_DIALOG_WINDOW_MS) { + this.activityTimeout = setTimeout(() => this._confirmIdle(), PANE_PROBE_RECHECK_MS); + return; + } this._awaitingIdleConfirmation = false; this.activityTimeout = null; // Emit idle if either: diff --git a/test/session-trust-dialog.test.ts b/test/session-trust-dialog.test.ts index 7aaa50a3..5dacaa67 100644 --- a/test/session-trust-dialog.test.ts +++ b/test/session-trust-dialog.test.ts @@ -16,6 +16,17 @@ */ import { describe, expect, it, vi, afterEach } from 'vitest'; import { Session } from '../src/session.js'; + +// The restart case below starts a real direct-PTY session; the real `claude` must never +// run from a test, so pty.spawn runs a harmless stand-in instead. +vi.mock('node-pty', async (importOriginal) => { + const real = await importOriginal(); + return { + ...real, + spawn: (_file: string, _args: string[] | string, options: import('node-pty').IPtyForkOptions) => + real.spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], options), + }; +}); import { isTrustDialogScreen, compactScreenText, @@ -249,3 +260,104 @@ describe('Session trust-dialog auto-accept', () => { expect(writes).toEqual([]); }); }); + +describe('Session idle detection while the trust dialog is up', () => { + afterEach(() => vi.useRealTimers()); + + /** A just-launched pane (busy, inside the startup window) rendering `screen` (null: capture failed). */ + function launchingSession(screen: () => string | null) { + const mux = { isAvailable: () => true, capturePaneText: () => screen() }; + const session = new Session({ + workingDir: '/tmp', + mode: 'claude', + mux, + muxSession: { muxName: 'codeman-test', sessionId: 'test', createdAt: Date.now() }, + } as unknown as ConstructorParameters[0]); + const internals = session as unknown as { + _detectInteractiveActivity(data: string): void; + _markActivity(): void; + _interactiveStartedAt: number; + _status: string; + }; + internals._interactiveStartedAt = Date.now(); + internals._status = 'busy'; // what startInteractive() leaves a new pane at + let idles = 0; + session.on('idle', () => idles++); + /** One repaint carrying a `❯`, which is what arms the idle confirmation. */ + const paint = (chunk: string) => { + internals._markActivity(); + internals._detectInteractiveActivity(chunk); + }; + return { session, paint, idles: () => idles }; + } + + it('does not call the dialog idle, although its own ❯ arms the check', () => { + vi.useFakeTimers(); + const { session, paint, idles } = launchingSession(() => RENDERED_DIALOG_2_1_252); + paint(RENDERED_DIALOG_2_1_252); + vi.advanceTimersByTime(30_000); + expect(session.status).toBe('busy'); + expect(idles()).toBe(0); + }); + + it('goes idle once the dialog has been answered and the composer is up', () => { + vi.useFakeTimers(); + let screen = RENDERED_DIALOG_2_1_252; + const { session, paint, idles } = launchingSession(() => screen); + paint(RENDERED_DIALOG_2_1_252); + vi.advanceTimersByTime(10_000); + expect(session.status).toBe('busy'); + + // No new output needed: the check keeps re-reading the screen on its own. + screen = RENDERED_MAIN_UI.replace('✻ Actualizing… (13m 23s · ↓ 47.5k tokens)', ''); + vi.advanceTimersByTime(10_000); + expect(session.status).toBe('idle'); + expect(idles()).toBe(1); + }); + + it('keeps holding when a capture fails after the dialog was seen', () => { + vi.useFakeTimers(); + // A failed capture says nothing about whether the dialog went away. + let screen: string | null = RENDERED_DIALOG_2_1_252; + const { session, paint, idles } = launchingSession(() => screen); + paint(RENDERED_DIALOG_2_1_252); + vi.advanceTimersByTime(5000); + screen = null; + vi.advanceTimersByTime(20_000); + expect(session.status).toBe('busy'); + expect(idles()).toBe(0); + }); + + it("does not carry the last pane's dialog verdict into a restarted pane", async () => { + // The session saw the dialog on its last pane, then restarted where no capture can + // replace that verdict (here the direct-PTY fallback, which has no pane to read; a + // failed capture keeps it the same way). The new pane must still be able to go idle. + const session = new Session({ workingDir: '/tmp', mode: 'claude', useMux: false }); + (session as unknown as { _lastPaneProbeTrustDialog: boolean })._lastPaneProbeTrustDialog = true; + try { + await session.startInteractive(); + vi.useFakeTimers(); + const internals = session as unknown as { + _detectInteractiveActivity(data: string): void; + _markActivity(): void; + }; + internals._markActivity(); + internals._detectInteractiveActivity('❯ '); + vi.advanceTimersByTime(30_000); + expect(session.status).toBe('idle'); + } finally { + vi.useRealTimers(); + await session.stop(); + } + }); + + it('lets a dialog-looking screen go idle outside the startup window', () => { + vi.useFakeTimers(); + // A live agent can print the dialog's words hours in, and must not be held busy. + const { session, paint } = launchingSession(() => RENDERED_DIALOG_2_1_252); + vi.advanceTimersByTime(10 * 60_000); + paint(RENDERED_DIALOG_2_1_252); + vi.advanceTimersByTime(30_000); + expect(session.status).toBe('idle'); + }); +});