fix(idle): hold idle while the workspace trust dialog is on screen

The dialog's own prompt glyph armed idle confirmation and the quiet dialog passed it, so a fresh case reported idle about 4 s in. An agent that waited for idle and then prompted typed into the dialog, and its Enter confirmed No, exit and killed the pane.
This commit is contained in:
JD committed 2026-10-14 22:16:49 -04:00
1 parent a8a8bb71a3
commit 2a5f5b30be
7 files changed
+154 -16

No files matched your search

+1 -1
View File
@@ -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`. **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) ⚠️ **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)
+8 -8
View File
@@ -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` 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, 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) 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: can land inside your first wait window and report a turn that never ran. That
a session parked on the trust dialog emits no *further* `idle`, so it is the startup `idle` is held while Claude's workspace-trust dialog is on screen (inside
startup transition, not the dialog, that produces the false success below. 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 ⚠️ **`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 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 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), `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**, 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 which contains a ❯ of its own. A prompt posted at that moment is typed into the
prompt into the dialog, where the `\r` never gets past it, while the session's dialog, and its `\r` confirms the highlighted `No, exit`: the CLI quits and the
startup `idle` lands inside the wait window: the wait resolves on `idle` in a pane dies, so send-and-wait resolves on `exit`. Codeman holds the session's `idle`
couple of seconds with `timedOut: false`, which looks exactly like a completed while the dialog is on screen, so waiting for `idle` before prompting no longer
turn. 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 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 non-null, then `wait-output` for the composer's own marker (`bypass`, the status
+2
View File
@@ -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. 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, <n>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. ⚠️ **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, <n>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. ⚠️ `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.
+4 -4
View File
@@ -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 ⚠️ **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` 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 ❯ 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 prompt of its own. Prompt it at that moment and the text lands in the dialog,
`\r` does not get past it, and the session's startup `idle` lands inside the wait whose highlighted option is `No, exit`, so the prompt's `\r` quits the CLI. Codeman
window: the wait resolves on `idle` in a couple of seconds with `timedOut: false`, holds the session's `idle` while the dialog is on screen, which makes waiting for
indistinguishable from a finished turn. Wait for the pid, then wait for the `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. composer, answering the dialog only as the bounded fallback.
⚠️ **Answering it is not "press Enter".** Claude Code 2.1.252 dropped the options' ⚠️ **Answering it is not "press Enter".** Claude Code 2.1.252 dropped the options'
+3 -3
View File
@@ -188,9 +188,9 @@ Read these before writing any code. Each one has cost somebody an afternoon.
output marker instead. output marker instead.
7. **Nothing reports "ready", so wait for it explicitly.** A new session answers 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*, `{"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 not *crashed*. A Claude worker in a fresh case then sits on the CLI's trust dialog, whose
it there and the wait resolves on idle in about two seconds looking exactly like a finished highlighted option is `No, exit`: prompt it there and your Enter quits the CLI. Codeman
turn, while your text sits stuck in the dialog. holds `idle` while the dialog is on screen, but that is not a readiness signal either.
### Recipes ### Recipes
+24
View File
@@ -563,6 +563,7 @@ export class Session extends EventEmitter {
private _activityStreak: ActivityStreak | null = null; // Unbroken run of PTY repaints (working detection) private _activityStreak: ActivityStreak | null = null; // Unbroken run of PTY repaints (working detection)
private _lastPaneProbeAt = 0; // Throttle for the tmux screen probe private _lastPaneProbeAt = 0; // Throttle for the tmux screen probe
private _lastPaneProbeWorking: boolean | null = null; // Its last verdict (null = could not read) 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. * 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._interactiveStartedAt = Date.now();
this._trustDialogAttempts = 0; this._trustDialogAttempts = 0;
this._lastTrustDialogScanAt = 0; this._lastTrustDialogScanAt = 0;
this._lastPaneProbeTrustDialog = false; // the last pane's verdict, not this one's
if (this._trustDialogTimer) { if (this._trustDialogTimer) {
clearTimeout(this._trustDialogTimer); clearTimeout(this._trustDialogTimer);
this._trustDialogTimer = null; 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. // composer is up and the pane is quiet, but the next turn starts without the user.
this._lastPaneProbeWorking = this._lastPaneProbeWorking =
text === null ? null : this._workingLinePattern().test(text) || this._paneAwaitsWorkers(text); 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._readWatching(text);
this._readScreenModel(text); this._readScreenModel(text);
return this._lastPaneProbeWorking; return this._lastPaneProbeWorking;
@@ -3523,6 +3537,16 @@ export class Session extends EventEmitter {
this.activityTimeout = setTimeout(() => this._confirmIdle(), PANE_PROBE_RECHECK_MS); this.activityTimeout = setTimeout(() => this._confirmIdle(), PANE_PROBE_RECHECK_MS);
return; 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._awaitingIdleConfirmation = false;
this.activityTimeout = null; this.activityTimeout = null;
// Emit idle if either: // Emit idle if either:
+112
View File
@@ -16,6 +16,17 @@
*/ */
import { describe, expect, it, vi, afterEach } from 'vitest'; import { describe, expect, it, vi, afterEach } from 'vitest';
import { Session } from '../src/session.js'; 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<typeof import('node-pty')>();
return {
...real,
spawn: (_file: string, _args: string[] | string, options: import('node-pty').IPtyForkOptions) =>
real.spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], options),
};
});
import { import {
isTrustDialogScreen, isTrustDialogScreen,
compactScreenText, compactScreenText,
@@ -249,3 +260,104 @@ describe('Session trust-dialog auto-accept', () => {
expect(writes).toEqual([]); 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<typeof Session>[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');
});
});