diff --git a/src/renderer/src/lib/prompt-echo-wait.test.ts b/src/renderer/src/lib/prompt-echo-wait.test.ts new file mode 100644 index 00000000000..86c83fedd87 --- /dev/null +++ b/src/renderer/src/lib/prompt-echo-wait.test.ts @@ -0,0 +1,142 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { watchForPromptEcho } from './prompt-echo-wait' + +const mocks = vi.hoisted(() => ({ + subscribeToPtyData: vi.fn(), + isRemoteRuntimePtyId: vi.fn(), + subscribeToRuntimeTerminalData: vi.fn() +})) + +vi.mock('@/components/terminal-pane/pty-data-sidecar-subscriptions', () => ({ + subscribeToPtyData: mocks.subscribeToPtyData +})) +vi.mock('@/runtime/runtime-terminal-inspection', () => ({ + isRemoteRuntimePtyId: mocks.isRemoteRuntimePtyId +})) +vi.mock('@/runtime/runtime-terminal-stream', () => ({ + subscribeToRuntimeTerminalData: mocks.subscribeToRuntimeTerminalData +})) + +const PROMPT = + 'Resolve the current merge conflicts UNIQUEMARKER7466 and report the final git status.' +// Real codex composer echo: every word placed by a CSI cursor move. +const ECHO = + '\x1b[11;3H\x1b[22mResolve\x1b[11;11Hthe\x1b[11;15Hcurrent\x1b[11;23Hmerge\x1b[11;29Hconflicts\x1b[11;39HUNIQUEMARKER7466' + +async function flushMicrotasks(iterations = 4): Promise { + for (let index = 0; index < iterations; index += 1) { + await Promise.resolve() + } +} + +describe('watchForPromptEcho', () => { + let localObserver: ((data: string) => void) | null + let unsubscribe: ReturnType + + beforeEach(() => { + vi.useFakeTimers() + vi.stubGlobal('window', { + setTimeout: globalThis.setTimeout, + clearTimeout: globalThis.clearTimeout + }) + localObserver = null + unsubscribe = vi.fn() + mocks.subscribeToPtyData.mockReset() + mocks.subscribeToPtyData.mockImplementation((_ptyId: string, observer: (d: string) => void) => { + localObserver = observer + return unsubscribe + }) + mocks.isRemoteRuntimePtyId.mockReset() + mocks.isRemoteRuntimePtyId.mockReturnValue(false) + mocks.subscribeToRuntimeTerminalData.mockReset() + }) + + afterEach(() => { + vi.useRealTimers() + vi.unstubAllGlobals() + }) + + it('resolves true and unsubscribes once the pasted content renders (local)', async () => { + const watch = watchForPromptEcho('pty-1', PROMPT, 10_000, {}) + localObserver?.(ECHO) + await expect(watch.result).resolves.toBe(true) + expect(unsubscribe).toHaveBeenCalledTimes(1) + }) + + it('resolves false and unsubscribes on timeout with no echo (local)', async () => { + const watch = watchForPromptEcho('pty-1', PROMPT, 10_000, {}) + localObserver?.('\x1b[2;2H\x1b[K') // redraw only, no pasted characters + await vi.advanceTimersByTimeAsync(10_000) + await expect(watch.result).resolves.toBe(false) + expect(unsubscribe).toHaveBeenCalledTimes(1) + }) + + it('resolves false and unsubscribes when cancelled', async () => { + const watch = watchForPromptEcho('pty-1', PROMPT, 10_000, {}) + watch.cancel() + await expect(watch.result).resolves.toBe(false) + expect(unsubscribe).toHaveBeenCalledTimes(1) + }) + + it('does not fire a late timer after the echo already resolved (local)', async () => { + const watch = watchForPromptEcho('pty-1', PROMPT, 10_000, {}) + localObserver?.(ECHO) + await expect(watch.result).resolves.toBe(true) + // Advancing past the deadline must not flip or re-settle the result. + await vi.advanceTimersByTimeAsync(10_000) + await expect(watch.result).resolves.toBe(true) + expect(unsubscribe).toHaveBeenCalledTimes(1) + }) + + describe('remote runtime transport', () => { + let remoteObserver: ((data: string) => void) | null + let remoteUnsubscribe: ReturnType + + beforeEach(() => { + mocks.isRemoteRuntimePtyId.mockReturnValue(true) + remoteObserver = null + remoteUnsubscribe = vi.fn() + }) + + it('subscribes through the host stream and resolves true on echo', async () => { + mocks.subscribeToRuntimeTerminalData.mockImplementation( + (_settings, _ptyId, _label, observer: (d: string) => void) => { + remoteObserver = observer + return Promise.resolve(remoteUnsubscribe) + } + ) + const watch = watchForPromptEcho('remote-pty', PROMPT, 10_000, {}) + await flushMicrotasks() + remoteObserver?.(ECHO) + await expect(watch.result).resolves.toBe(true) + expect(remoteUnsubscribe).toHaveBeenCalledTimes(1) + }) + + it('disposes a subscription that resolves after the watch already settled', async () => { + let resolveSubscribe!: (u: ReturnType) => void + const pending = new Promise>((resolve) => { + resolveSubscribe = resolve + }) + mocks.subscribeToRuntimeTerminalData.mockImplementation( + (_settings, _ptyId, _label, observer: (d: string) => void) => { + remoteObserver = observer + return pending + } + ) + const watch = watchForPromptEcho('remote-pty', PROMPT, 10_000, {}) + watch.cancel() + await expect(watch.result).resolves.toBe(false) + // The subscription attaches only now — it must be torn down, not leaked. + resolveSubscribe(remoteUnsubscribe) + await flushMicrotasks() + expect(remoteUnsubscribe).toHaveBeenCalledTimes(1) + }) + + it('resolves false when the remote subscription rejects', async () => { + mocks.subscribeToRuntimeTerminalData.mockRejectedValue(new Error('closed')) + const watch = watchForPromptEcho('remote-pty', PROMPT, 10_000, {}) + await flushMicrotasks() + await expect(watch.result).resolves.toBe(false) + }) + }) +}) diff --git a/src/shared/prompt-echo-scanner.test.ts b/src/shared/prompt-echo-scanner.test.ts index 959f2a19589..fcff11084fc 100644 --- a/src/shared/prompt-echo-scanner.test.ts +++ b/src/shared/prompt-echo-scanner.test.ts @@ -81,7 +81,7 @@ describe('createPromptEchoScanner', () => { expect(scanner.observe('unrelated')).toBe(true) }) - it('treats content that folds below the minimum sample as echoed on any render', () => { + it('treats punctuation-only content that folds to nothing as echoed on any render', () => { const scanner = createPromptEchoScanner('!?— \n') expect(scanner.observe('any repaint at all')).toBe(true) }) @@ -90,4 +90,17 @@ describe('createPromptEchoScanner', () => { const scanner = createPromptEchoScanner('!?— \n') expect(scanner.observe('')).toBe(false) }) + + it('does not fire a short prompt on a swallow-screen redraw that renders none of it', () => { + // #7466: "fix ci" folds to 5 chars — it must still match on its head + // sample, not fall back to "any render", or the submit lands on the + // swallowing screen exactly as before this fix. + const scanner = createPromptEchoScanner('fix ci') + expect(scanner.observe(CODEX_TRUST_SWALLOW_REDRAW)).toBe(false) + }) + + it('fires a short prompt once its characters actually render', () => { + const scanner = createPromptEchoScanner('fix ci') + expect(scanner.observe('\x1b[11;3Hfix\x1b[11;7Hci')).toBe(true) + }) }) diff --git a/src/shared/prompt-echo-scanner.ts b/src/shared/prompt-echo-scanner.ts index 4f18b888b61..53b1cc2aefc 100644 --- a/src/shared/prompt-echo-scanner.ts +++ b/src/shared/prompt-echo-scanner.ts @@ -41,7 +41,6 @@ export function foldTerminalTextForEchoMatch(raw: string): string { const FOLDED_PLACEHOLDER_PATTERNS = [/pastedcontent\d+chars/, /pastedtext\d+/] const SAMPLE_LENGTH = 24 -const MIN_SAMPLE_LENGTH = 8 // Why: refold the whole raw ring on every chunk instead of delta-folding — // escape sequences straddling chunk seams make incremental folds misalign, // and the scanner only lives for the few seconds after one paste, so the @@ -58,10 +57,13 @@ export function createPromptEchoScanner(pastedContent: string): PromptEchoScanne const folded = foldTerminalTextForEchoMatch(pastedContent) const head = folded.slice(0, SAMPLE_LENGTH) const tail = folded.length > SAMPLE_LENGTH ? folded.slice(-SAMPLE_LENGTH) : '' - // Why: content that folds to almost nothing (punctuation-only prompts) - // cannot be matched reliably — treat any post-paste render as echo rather - // than blocking delivery on an impossible match. - const unmatchable = head.length < MIN_SAMPLE_LENGTH + // Why: only content that folds to nothing (punctuation/whitespace-only + // prompts) is genuinely impossible to match — treat any post-paste render as + // echo there rather than blocking delivery forever. Short-but-nonempty + // prompts (e.g. "fix ci") must still match on the head sample below so a + // swallow screen that renders none of their characters withholds the submit + // (#7466); firing on an arbitrary redraw would resubmit the exact bug. + const unmatchable = folded.length === 0 let rawRing = '' let echoed = false