From 618a8b0758dbf754ca3be61ab9e48513f84f1bd2 Mon Sep 17 00:00:00 2001 From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com> Date: Sun, 27 Sep 2026 18:17:04 -0400 Subject: [PATCH] fix(terminal): keep a dead app's input modes recoverable after a refuted proof (#23474) A command's armed input modes were demoted at the first OSC 133;D whether or not the foreground proof confirmed, and the alternate-screen trigger was spent the same way. A full-screen agent's nested command shells leak their own D onto the main PTY; the refuted proof for that stray D used up the only trigger, so the app's real death later went unrecovered. Every D now re-asks while a command's mode (or the alternate screen) is still up; only the confirmed ground or the app's own disable clears it. Proofs that can never succeed (wsl.exe, no Windows job reads) already refute immediately without spawning anything. The accepted cost is one process read plus a bounded output hold per D while a command-owned mode stays up and the proof keeps refuting: a live agent leaking D, a stopped job, a nested subshell, or an rc that execs another shell. --- src/main/daemon/terminal-armed-input-modes.ts | 11 +-- ...al-shell-armed-input-mode-recovery.test.ts | 29 ++++++-- .../terminal-shell-lifecycle-scanner.test.ts | 17 +++-- .../terminal-shell-lifecycle-scanner.ts | 13 +--- .../terminal-shell-recovery-barrier.test.ts | 74 +++++++++++++++---- .../pty-shell-ownership-mirror.test.ts | 18 +++++ 6 files changed, 118 insertions(+), 44 deletions(-) diff --git a/src/main/daemon/terminal-armed-input-modes.ts b/src/main/daemon/terminal-armed-input-modes.ts index e2ba02a2ca2..ca0e37a6410 100644 --- a/src/main/daemon/terminal-armed-input-modes.ts +++ b/src/main/daemon/terminal-armed-input-modes.ts @@ -8,7 +8,7 @@ const HOST_ARMABLE_MODE = 1004 type ModeKey = number | 'kitty-main' | 'kitty-alt' // host: HOST_ARMABLE_MODE armed before any marker or by a prompt a 133;C proved; the ground keeps it. // prompt: armed outside a command, unproven until C. -// command: armed after C; still on at 133;D, it triggers the ground. +// command: armed after C; still on at any 133;D, it triggers the ground until disarmed. // stale: anything else still on; the ground clears it without it ever triggering. type ModeOwner = 'host' | 'prompt' | 'command' | 'stale' // Matches xterm.js's eviction limit, so the model drops the same entries. @@ -104,13 +104,14 @@ export class TerminalArmedInputModes { this.enableOwner = 'prompt' } - /** OSC 133;D: true when the command left a mode it armed. Demoting makes it one-shot. */ + /** OSC 133;D: true when a command's mode is still on. Not demoted: a nested shell's + * stray D gets a refuted proof, and the app's real D must still trigger. */ markCommandEnd(): boolean { let left = false for (const [key, owner] of this.owners) { - if (owner === 'command' || owner === 'prompt') { - // The other screen's kitty flags stay parked in xterm and reach no input. - left ||= owner === 'command' && (typeof key === 'number' || key === this.kittyKey()) + // The other screen's kitty flags stay parked in xterm and reach no input. + left ||= owner === 'command' && (typeof key === 'number' || key === this.kittyKey()) + if (owner === 'prompt') { this.owners.set(key, 'stale') } } diff --git a/src/main/daemon/terminal-shell-armed-input-mode-recovery.test.ts b/src/main/daemon/terminal-shell-armed-input-mode-recovery.test.ts index fef4d762c69..630fe21d0bf 100644 --- a/src/main/daemon/terminal-shell-armed-input-mode-recovery.test.ts +++ b/src/main/daemon/terminal-shell-armed-input-mode-recovery.test.ts @@ -50,14 +50,14 @@ describe('armed input modes arm the unclean-death trigger', () => { expect(triggers(scanner, chunk)).toBe(false) }) - it('stays one-shot until a fresh enable', () => { + it('re-triggers at every D until the mode is disarmed', () => { const scanner = new TerminalShellLifecycleScanner() expect(triggers(scanner, `${COMMAND_START}\x1b[?1004hRUN${COMMAND_DONE}`)).toBe(true) - // A refuted proof leaves ?1004 armed; later prompts must not pause again. - expect(triggers(scanner, `$ ${COMMAND_START}ls${COMMAND_DONE}`)).toBe(false) - expect(triggers(scanner, `$ ${COMMAND_START}ls${COMMAND_DONE}`)).toBe(false) - expect(triggers(scanner, `$ ${COMMAND_START}\x1b[?1000hRUN${COMMAND_DONE}`)).toBe(true) + // A refuted proof leaves ?1004 armed and command-owned; each later D re-asks. + expect(triggers(scanner, `$ ${COMMAND_START}ls${COMMAND_DONE}`)).toBe(true) + expect(triggers(scanner, `$ ${COMMAND_START}ls${COMMAND_DONE}`)).toBe(true) + expect(triggers(scanner, `\x1b[?1004l$ ${COMMAND_START}ls${COMMAND_DONE}`)).toBe(false) }) it('treats modes the prompt armed before the command started as shell-owned', () => { @@ -342,6 +342,25 @@ describe('Session grounds a proven normal-buffer death', () => { expect(snapshot?.snapshotAnsi).toContain('\x1b[?1004h') }) + it("grounds an agent's modes when it dies after nested shells' refuted Ds", async () => { + const leak = { data: `${COMMAND_START}nested\r\n${COMMAND_DONE}`, confirm: false } + const { snapshot, records, proofs } = await runSteps([ + { data: `${PROMPT_START}$ ${COMMAND_START}\x1b[>1u\x1b[?1003hAGENT` }, + ...Array.from({ length: 5 }, () => leak), + { data: `EXIT\r\n${COMMAND_DONE}${PROMPT_START}$ `, confirm: true }, + { data: `${COMMAND_START}ls\r\n${COMMAND_DONE}${PROMPT_START}$ ` } + ]) + + // The confirmed ground disarms the modes: the next prompt opens no episode. + expect(proofs).toBe(6) + expect( + records.some( + (record) => record.kind === 'output' && record.data.includes(PROCESS_BOUNDARY_GROUND) + ) + ).toBe(true) + expect(snapshot?.modes.mouseTrackingMode).toBe('none') + }) + it('flushes without the ground when the proof is refuted', async () => { const { snapshot, records } = await runNormalBufferDeath(false) diff --git a/src/main/daemon/terminal-shell-lifecycle-scanner.test.ts b/src/main/daemon/terminal-shell-lifecycle-scanner.test.ts index 7d5136d9050..08d2d8c9325 100644 --- a/src/main/daemon/terminal-shell-lifecycle-scanner.test.ts +++ b/src/main/daemon/terminal-shell-lifecycle-scanner.test.ts @@ -268,18 +268,19 @@ describe('TerminalShellLifecycleScanner', () => { }) describe('unclean trigger arming', () => { - it('fires once per alternate-screen occupancy and re-arms only on a fresh entry', () => { + it('fires at every D while the alternate screen stays up, and never once it is left', () => { const scanner = new TerminalShellLifecycleScanner() + const prompt = '\x1b]133;C\x07ls\r\n\x1b]133;D;0\x07' expect(scanner.scan('\x1b[?1049hTUI\x1b]133;D;137\x07').uncleanDeathTriggerEnd).toBeDefined() - // Refuted path: no reset scanned, alt still active — a later prompt's D must not re-trigger. - const second = scanner.scan('\x1b]133;C\x07ls\r\n\x1b]133;D;0\x07') - expect(second.uncleanDeathTriggerEnd).toBeUndefined() - expect(second.cleanExitCandidate).toBeUndefined() + // Refuted path: no reset scanned, so each later D re-asks — one may be the TUI's real death. + for (let index = 0; index < 5; index += 1) { + expect(scanner.scan(prompt).uncleanDeathTriggerEnd).toBeDefined() + } expect(scanner.isAlternateScreenActive).toBe(true) - expect( - scanner.scan('\x1b]133;C\x07\x1b[?1049hAGAIN\x1b]133;D;9\x07').uncleanDeathTriggerEnd - ).toBeDefined() + const left = scanner.scan(`\x1b[?1049l${prompt}`) + expect(left.uncleanDeathTriggerEnd).toBeUndefined() + expect(scanner.scan(prompt).uncleanDeathTriggerEnd).toBeUndefined() }) }) diff --git a/src/main/daemon/terminal-shell-lifecycle-scanner.ts b/src/main/daemon/terminal-shell-lifecycle-scanner.ts index f62ab47cfd3..1e655cd4896 100644 --- a/src/main/daemon/terminal-shell-lifecycle-scanner.ts +++ b/src/main/daemon/terminal-shell-lifecycle-scanner.ts @@ -54,11 +54,6 @@ export class TerminalShellLifecycleScanner { private altActive = false private commandEnteredAlternateScreen = false private readonly inputModes = new TerminalArmedInputModes() - // Why one-shot: a refuted proof leaves the alt screen up (no reset was ever - // scanned), and without disarming every later prompt's D would re-open a full - // pause-and-inspect episode. Only a fresh alternate-screen enable re-arms; input - // modes are one-shot by markCommandEnd's demotion. - private uncleanTriggerArmed = false get owner(): TerminalOwner | undefined { return this.ownerState @@ -97,7 +92,6 @@ export class TerminalShellLifecycleScanner { // this a mirror seeded mid-TUI never arms its unclean-death trigger and // the whole occupancy loses recovery. this.altActive = opts.alternateScreen - this.uncleanTriggerArmed = opts.alternateScreen this.commandEnteredAlternateScreen = opts.alternateScreen } } @@ -123,7 +117,6 @@ export class TerminalShellLifecycleScanner { this.inputModes.reset() this.altActive = false this.commandEnteredAlternateScreen = false - this.uncleanTriggerArmed = false continue } if (oscPayload !== undefined) { @@ -142,13 +135,12 @@ export class TerminalShellLifecycleScanner { } // An alternate screen or a command's input mode still up at command-finished // means the app died without its own teardown; the caller must repair first. - const leftInputModes = this.inputModes.markCommandEnd() - const uncleanDeath = (this.uncleanTriggerArmed && this.altActive) || leftInputModes + // Every D re-asks: a refuted one may be a nested shell's while the app lives. + const uncleanDeath = this.inputModes.markCommandEnd() || this.altActive const cleanExit = !uncleanDeath && this.commandEnteredAlternateScreen && !this.altActive this.revoke() this.commandEnteredAlternateScreen = false if (uncleanDeath) { - this.uncleanTriggerArmed = false this.scanTail = '' // Why the clamp: a complete OSC can never sit wholly inside the // carried tail (extractScanTail keeps incomplete ones only), so this @@ -195,7 +187,6 @@ export class TerminalShellLifecycleScanner { this.altActive = enabled if (enabled) { this.commandEnteredAlternateScreen = true - this.uncleanTriggerArmed = true } } } diff --git a/src/main/daemon/terminal-shell-recovery-barrier.test.ts b/src/main/daemon/terminal-shell-recovery-barrier.test.ts index 7cea4ed314e..872d33fe766 100644 --- a/src/main/daemon/terminal-shell-recovery-barrier.test.ts +++ b/src/main/daemon/terminal-shell-recovery-barrier.test.ts @@ -317,25 +317,69 @@ describe('TerminalShellRecoveryBarrier', () => { await expect(barrier.idle()).resolves.toBeUndefined() }) - it('opens at most one episode per alternate-screen occupancy after a refuted proof', async () => { + it('asks at every D of a live TUI and releases every byte, in order, unmodified', async () => { const { barrier, released, confirm } = createBarrier({ confirm: async () => false }) - - barrier.accept(passthrough(`${TRIGGER}prompt`)) - await vi.waitFor(() => expect(released).toHaveLength(2)) - expect(confirm).toHaveBeenCalledTimes(1) - - // Why: the refuted path never scans a reset, so alt stays active — later - // ordinary prompts must not each re-open a pause-and-inspect episode. + const leak = '\x1b]133;C\x07nested\x1b]133;D;0\x07' + const sent = [`\x1b[?1049h\x1b[?1003hTUI${leak}frame`] for (let index = 0; index < 5; index += 1) { - const prompt = passthrough(`\x1b]133;C\x07ls\r\n\x1b]133;D;0\x07`) - barrier.accept(prompt) - expect(released.at(-1)).toBe(prompt) + sent.push(`${leak}frame${index}`) } - expect(confirm).toHaveBeenCalledTimes(1) - // A fresh alternate-screen entry re-arms recovery. - barrier.accept(passthrough(`\x1b]133;C\x07\x1b[?1049hAGAIN\x1b]133;D;9\x07`)) - expect(confirm).toHaveBeenCalledTimes(2) + let seq = 0 + for (const data of sent) { + barrier.accept(passthrough(data, seq)) + seq += data.length + } + await vi.waitFor(() => expect(confirm).toHaveBeenCalledTimes(6)) + await barrier.idle() + + expect(released.map((emission) => emission.data).join('')).toBe(sent.join('')) + expect(released.every((emission) => !emission.transformed)).toBe(true) + expect(barrier.getOwner()).toBeUndefined() + }) + + it("grounds a TUI that dies after many nested shells' Ds were refuted", async () => { + let dead = false + const { barrier, released, confirm } = createBarrier({ confirm: async () => dead }) + const leak = '\x1b]133;C\x07nested\x1b]133;D;0\x07' + + barrier.accept(passthrough(`\x1b[?1049hTUI${leak.repeat(5)}`)) + await vi.waitFor(() => expect(confirm).toHaveBeenCalledTimes(5)) + await barrier.idle() + dead = true + barrier.accept(passthrough('\x1b]133;D;137\x07prompt')) + + await vi.waitFor(() => expect(barrier.getOwner()).toBe('shell')) + expect(released.slice(-3).map((emission) => emission.data)).toEqual([ + '\x1b]133;D;137\x07', + PROCESS_BOUNDARY_GROUND, + 'prompt' + ]) + // Grounded once: the next prompt opens no episode. + barrier.accept(passthrough('\x1b]133;C\x07ls\x1b]133;D;0\x07')) + expect(confirm).toHaveBeenCalledTimes(6) + }) + + it('grounds a real death D that arrives while a stray D is still being proven', async () => { + const proofs: ((confirmed: boolean) => void)[] = [] + const { barrier, released, confirm } = createBarrier({ + confirm: () => new Promise((resolve) => void proofs.push(resolve)) + }) + const leak = '\x1b[?1049h\x1b[?1003hTUI\x1b]133;C\x07nested\x1b]133;D;0\x07' + + barrier.accept(passthrough(leak, 0)) + barrier.accept(passthrough('frame\x1b]133;D;137\x07prompt', leak.length)) + proofs[0]?.(false) + await vi.waitFor(() => expect(confirm).toHaveBeenCalledTimes(2)) + proofs[1]?.(true) + + await vi.waitFor(() => expect(barrier.getOwner()).toBe('shell')) + expect(released.map((emission) => emission.data)).toEqual([ + leak, + 'frame\x1b]133;D;137\x07', + PROCESS_BOUNDARY_GROUND, + 'prompt' + ]) }) it('bounds awaitProofSettled by its own deadline when a clean-exit proof hangs', async () => { diff --git a/src/main/runtime/pty-shell-ownership-mirror.test.ts b/src/main/runtime/pty-shell-ownership-mirror.test.ts index 20468a194cd..b0525940a86 100644 --- a/src/main/runtime/pty-shell-ownership-mirror.test.ts +++ b/src/main/runtime/pty-shell-ownership-mirror.test.ts @@ -16,6 +16,24 @@ describe('PtyShellOwnershipMirror', () => { expect(mirror.owner).toBe('shell') }) + it('asks at the real D after refuted stray Ds, in lockstep with the host barrier', async () => { + let hostGrounded = false + const confirm = vi.fn(async () => hostGrounded) + const mirror = new PtyShellOwnershipMirror(confirm) + mirror.scan('\x1b]133;C\x07\x1b[>1u\x1b[?1049hAGENT') + + for (let index = 1; index <= 5; index += 1) { + mirror.scan('\x1b]133;C\x07nested\x1b]133;D;0\x07') + await vi.waitFor(() => expect(confirm).toHaveBeenCalledTimes(index)) + await mirror.settle() + } + hostGrounded = true + mirror.scan('\x1b]133;D;137\x07') + + await vi.waitFor(() => expect(mirror.owner).toBe('shell')) + expect(confirm).toHaveBeenCalledTimes(6) + }) + it('does not arm from a seed on the normal buffer', async () => { const confirm = vi.fn(async () => true) const mirror = new PtyShellOwnershipMirror(confirm)