mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-20 09:09:35 +02:00
Merge pull request #635 from JDProfresh/fix/trust-dialog-deferred-read
fix(trust-dialog): re-read a throttled trust-dialog scan instead of dropping it Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
commit
eec2895dd7
4 files changed
+64
-8
No files matched your search
@@ -148,7 +148,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph
|
||||
|
||||
**Dead-pane respawn resume pin** (`_buildRespawnPaneOptionsWithResumePin()`, session.ts): recovering a dead pane, like a custom-model `restartCli()`, must pin the conversation or claude refuses the reused `--session-id`. The pin takes the first transcript-backed candidate (chain tail, launch seed, own id), never `_claudeSessionId`, adds nothing when none is backed, and is never applied to remote or docker sessions. → [architecture-invariants#dead-pane-respawn-the-resume-pin](docs/architecture-invariants.md#dead-pane-respawn-the-resume-pin)
|
||||
|
||||
**Workspace-trust dialog auto-accept** (`session-trust-dialog.ts`, pure): Claude Code's per-directory trust dialog is always answered yes, or the session is stuck. ⚠️ Match the compacted SCREEN (`compactScreenText()`, all whitespace removed), never the stream (tmux sends words joined by cursor-forwards, not spaces). ⚠️ Never answer with a blind `\r` (newer versions highlight "No, exit" first): `trustDialogNextKey()` returns ONE key per re-read frame (arrow, then Enter only once `❯` is on the trust option), and the LAST marked option wins. ⚠️ All three guards must hold: startup window `TRUST_DIALOG_WINDOW_MS` (90s), two-marker match (`isTrustDialogScreen`), attempt cap `TRUST_DIALOG_MAX_ATTEMPTS` (6). ⚠️ Read `capturePaneText()`; only a direct-PTY session falls back to a SHORT buffer tail. ⚠️ The scan must schedule its own next read (`_trustDialogTimer`, cleared in `_clearAllTimers()`), not rely on PTY output. → [architecture-invariants#workspace-trust-dialog-auto-accept](docs/architecture-invariants.md#workspace-trust-dialog-auto-accept)
|
||||
**Workspace-trust dialog auto-accept** (`session-trust-dialog.ts`, pure): Claude Code's per-directory trust dialog is always answered yes, or the session is stuck. ⚠️ Match the compacted SCREEN (`compactScreenText()`, all whitespace removed), never the stream (tmux sends words joined by cursor-forwards, not spaces). ⚠️ Never answer with a blind `\r` (newer versions highlight "No, exit" first): `trustDialogNextKey()` returns ONE key per re-read frame (arrow, then Enter only once `❯` is on the trust option), and the LAST marked option wins. ⚠️ All three guards must hold: startup window `TRUST_DIALOG_WINDOW_MS` (90s), two-marker match (`isTrustDialogScreen`), attempt cap `TRUST_DIALOG_MAX_ATTEMPTS` (6). ⚠️ Read `capturePaneText()`; only a direct-PTY session falls back to a SHORT buffer tail. ⚠️ The scan must schedule its own next read (`_trustDialogTimer`, cleared in `_clearAllTimers()`), not rely on PTY output, and a throttled read is DEFERRED onto that timer, never dropped (the dialog's paint is often the pane's last output); a stopped session never scans. → [architecture-invariants#workspace-trust-dialog-auto-accept](docs/architecture-invariants.md#workspace-trust-dialog-auto-accept)
|
||||
|
||||
**Process-tree walks are bounded** (`proc-tree.ts`, pure): `collectDescendants(pid, byParent)` is the ONE descendant traversal, fed by one cached `ps -eo pid=,ppid=` snapshot (`refreshProcSnapshot()` in tmux-manager.ts: async, in-flight-shared, and ANY error discards the result rather than caching a truncated `ps`). ⚠️ Never walk a process tree with per-node `pgrep` or unbounded recursion (the unbounded version took a machine down): the walk must terminate on cycles, cap depth (`PROC_WALK_MAX_DEPTH`) and node count (`PROC_WALK_MAX_NODES`), and never spawn anything. ⚠️ Keep it in its own module so the test exercises the shipped code, and report truncation through `onTruncated` naming both caps, never silently. → [architecture-invariants#process-tree-walks-are-bounded](docs/architecture-invariants.md#process-tree-walks-are-bounded)
|
||||
|
||||
|
||||
@@ -300,7 +300,7 @@ So: `_confirmIdle()` (session.ts) requires the pane to go quiet, and then asks t
|
||||
|
||||
⚠️ It reads `capturePaneText()` and falls back to a deliberately SHORT tail of the terminal buffer only on a direct-PTY session, which has no pane: that buffer is append-only, so a longer tail would keep re-matching a dialog answered minutes ago.
|
||||
|
||||
⚠️ **The scan must schedule its own next read** (`_trustDialogTimer`, cleared in `_clearAllTimers()`): it runs from the PTY `onData` handler, which was enough while one Enter answered the dialog, but the arrow that moves the cursor is the LAST output the pane produces, so a two-keystroke answer waiting on more output parks forever with the cursor sitting on the right option (measured on a live 2.1.252 spawn: cursor moved at 6 s, then nothing).
|
||||
⚠️ **The scan must schedule its own next read** (`_trustDialogTimer`, cleared in `_clearAllTimers()`): it runs from the PTY `onData` handler, which was enough while one Enter answered the dialog, but the arrow that moves the cursor is the LAST output the pane produces, so a two-keystroke answer waiting on more output parks forever with the cursor sitting on the right option (measured on a live 2.1.252 spawn: cursor moved at 6 s, then nothing). ⚠️ For the same reason a read the `TRUST_DIALOG_RETRY_MS` throttle refuses is DEFERRED onto that timer, never dropped: the dialog's own paint is often the last output a launching pane produces and lands inside the throttle of the read the blank screen before it triggered, so dropping it left 2 of 12 fresh launches on Claude Code 2.1.294 sitting on the dialog with no keystroke ever sent. And a stopped session never scans: killing the pane produces output, the buffer tail still holds the dialog, and a deleted session went on scanning it and arming the timer for several seconds.
|
||||
|
||||
### Owner tab layouts
|
||||
|
||||
|
||||
+20
-6
@@ -3143,13 +3143,24 @@ export class Session extends EventEmitter {
|
||||
* a two-marker match (isTrustDialogScreen), and an attempt cap.
|
||||
*/
|
||||
private _maybeAcceptTrustDialog(): void {
|
||||
if (this._trustDialogAccepted) return;
|
||||
// Stopped: the kill itself produces PTY output, and the buffer tail still holds the
|
||||
// dialog, so without this a deleted session kept "answering" it for several seconds.
|
||||
if (this._trustDialogAccepted || this._isStopped) return;
|
||||
const now = Date.now();
|
||||
if (now - this._interactiveStartedAt > TRUST_DIALOG_WINDOW_MS) {
|
||||
this._trustDialogAccepted = true; // window closed; anything matching now is not the dialog
|
||||
return;
|
||||
}
|
||||
if (now - this._lastTrustDialogScanAt < TRUST_DIALOG_RETRY_MS) return;
|
||||
const sinceScan = now - this._lastTrustDialogScanAt;
|
||||
if (sinceScan < TRUST_DIALOG_RETRY_MS) {
|
||||
// ⚠️ Defer a throttled read, never drop it. The dialog's own paint is usually the
|
||||
// last output a launching pane produces, and it tends to land inside the throttle
|
||||
// of the read the blank screen before it triggered: dropped, nothing re-read the
|
||||
// screen and the session sat on the dialog for good (2 of 12 fresh launches on
|
||||
// Claude Code 2.1.294). A read already scheduled covers this one.
|
||||
if (!this._trustDialogTimer) this._rescanTrustDialogIn(TRUST_DIALOG_RETRY_MS - sinceScan + 100);
|
||||
return;
|
||||
}
|
||||
this._lastTrustDialogScanAt = now;
|
||||
|
||||
// Prefer the pane; fall back to the buffer tail on a direct-PTY session,
|
||||
@@ -3184,14 +3195,17 @@ export class Session extends EventEmitter {
|
||||
// on a live 2.1.252 spawn: cursor moved at 6 s, then nothing). The timer is
|
||||
// one-shot and self-rearming through this same path, and every exit route
|
||||
// goes through _clearAllTimers().
|
||||
// The +100ms puts the re-entry OUTSIDE the scan throttle above; firing at
|
||||
// exactly the throttle boundary would let the scan return early and break
|
||||
// the chain with the dialog still on screen.
|
||||
// The +100ms puts the re-entry OUTSIDE the scan throttle above.
|
||||
this._rescanTrustDialogIn(TRUST_DIALOG_RETRY_MS + 100);
|
||||
}
|
||||
|
||||
/** Re-run the trust-dialog scan after `delayMs`, replacing any read already scheduled. */
|
||||
private _rescanTrustDialogIn(delayMs: number): void {
|
||||
if (this._trustDialogTimer) clearTimeout(this._trustDialogTimer);
|
||||
this._trustDialogTimer = setTimeout(() => {
|
||||
this._trustDialogTimer = null;
|
||||
this._maybeAcceptTrustDialog();
|
||||
}, TRUST_DIALOG_RETRY_MS + 100);
|
||||
}, delayMs);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -239,6 +239,48 @@ describe('Session trust-dialog auto-accept', () => {
|
||||
expect(writes).toEqual([]);
|
||||
});
|
||||
|
||||
it('reads the dialog that painted inside the throttle of the read before it', () => {
|
||||
vi.useFakeTimers();
|
||||
// Measured on 2.1.294: one read at +156 ms saw a blank pane, the dialog painted
|
||||
// within the throttle, and a quiet dialog sends no further output to read on.
|
||||
let screen = '';
|
||||
const { writes, tick } = sessionShowing(() => screen);
|
||||
tick();
|
||||
vi.advanceTimersByTime(500);
|
||||
screen = RENDERED_DIALOG_2_1_252;
|
||||
tick(); // the dialog's own paint: throttled, and the last output the pane produces
|
||||
vi.advanceTimersByTime(5000);
|
||||
expect(writes[0]).toBe(TRUST_KEY_DOWN);
|
||||
});
|
||||
|
||||
it('cancels a deferred read when the session is stopped', async () => {
|
||||
vi.useFakeTimers();
|
||||
// DELETE goes through stop(), and a read deferred just before it must not outlive it.
|
||||
let screen = '';
|
||||
const { session, tick } = sessionShowing(() => screen);
|
||||
tick();
|
||||
screen = RENDERED_DIALOG_2_1_252;
|
||||
const before = vi.getTimerCount();
|
||||
tick(); // throttled, so a read is deferred
|
||||
expect(vi.getTimerCount()).toBe(before + 1);
|
||||
await session.stop(false);
|
||||
expect(vi.getTimerCount()).toBe(0);
|
||||
});
|
||||
|
||||
it('neither scans nor arms a timer once the session is stopped', async () => {
|
||||
vi.useFakeTimers();
|
||||
// Killing the pane produces output of its own, and the buffer tail still shows the
|
||||
// dialog, so a deleted session went on "answering" it on a timer for several seconds.
|
||||
const { session, tick } = sessionShowing(() => RENDERED_DIALOG_2_1_252);
|
||||
await session.stop(false);
|
||||
const log = vi.spyOn(console, 'log').mockImplementation(() => {});
|
||||
tick();
|
||||
tick(); // inside the throttle of the first
|
||||
expect(vi.getTimerCount()).toBe(0);
|
||||
expect(log.mock.calls.some(([line]) => String(line).includes('trust dialog'))).toBe(false);
|
||||
log.mockRestore();
|
||||
});
|
||||
|
||||
it('does not press Enter on a normal screen', () => {
|
||||
vi.useFakeTimers();
|
||||
const { writes, tick } = sessionShowing(() => RENDERED_MAIN_UI);
|
||||
|
||||
Reference in new issue
Block a user