From 762fb05caa6e252e34768ca80d9d41946c41e1cf Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Wed, 26 Aug 2026 22:31:57 -0700 Subject: [PATCH] fix(agents): one-shot submit-retry Enter for codex (STA-5379) (#16689) Codex silently discards Enter for ~75-150ms after its composer glyph first renders, and the boundary widens with prompt size and machine load, so no fixed first-Enter delay is provably safe on slow hosts. Submit success is not verifiable from PTY output, but a redundant Enter is a measured no-op on codex in both post-submit states, so send one blind retry after the first Enter. - tui-agent-config: new optional submitRetryDelayMs knob, set to 1200 on codex only; every other agent is byte-identical to today. - agent-paste-draft: after the post-paste '\r', wait the configured gap and send exactly one more '\r' inside the same PTY input transaction, so a concurrent paste cannot interleave. The retry is best-effort and never downgrades the first Enter's result. - Retry tests live in a new file to keep agent-paste-draft.test.ts under the max-lines budget. active-agent-note-send is deliberately exempt: its Enter rides the terminal.send RPC (different transport, server-side sendable guard), has no local agent identity to read the config from, and only fires on an already-running agent, where the codex cold-boot submit gate cannot occur. --- .../agent-paste-draft-submit-retry.test.ts | 197 ++++++++++++++++++ .../src/lib/agent-paste-draft.test.ts | 4 +- src/renderer/src/lib/agent-paste-draft.ts | 29 ++- src/shared/tui-agent-config.ts | 5 +- 4 files changed, 227 insertions(+), 8 deletions(-) create mode 100644 src/renderer/src/lib/agent-paste-draft-submit-retry.test.ts diff --git a/src/renderer/src/lib/agent-paste-draft-submit-retry.test.ts b/src/renderer/src/lib/agent-paste-draft-submit-retry.test.ts new file mode 100644 index 00000000000..5b763aa5c6d --- /dev/null +++ b/src/renderer/src/lib/agent-paste-draft-submit-retry.test.ts @@ -0,0 +1,197 @@ +import { beforeEach, afterEach, describe, expect, it, vi } from 'vitest' +import { + AGENT_DRAFT_PASTE_DIRECT_MAX_BYTES, + pasteDraftWhenAgentReady, + POST_PASTE_SUBMIT_DELAY_MS, + sendAgentDraftPasteContent +} from './agent-paste-draft' +import { TUI_AGENT_CONFIG } from '../../../shared/tui-agent-config' + +const testState = vi.hoisted(() => ({ + appState: { + settings: {} as Record, + ptyIdsByTabId: { 'tab-1': ['pty-1'] } as Record, + runtimePaneTitlesByTabId: {}, + tabsByWorktree: {} as Record, + repos: [] as { id: string; connectionId: string | null; executionHostId?: string | null }[], + worktreesByRepo: {} as Record + }, + ptyObserver: null as ((data: string) => void) | null, + unsubscribe: vi.fn(), + subscribeToPtyData: vi.fn(), + replayPreHandlerPtyData: vi.fn(), + isRemoteRuntimePtyId: vi.fn(), + sendRuntimePtyInputVerified: vi.fn(), + inspectRuntimeTerminalProcess: vi.fn(), + subscribeToRuntimeTerminalData: vi.fn() +})) + +vi.mock('@/store', () => ({ + useAppStore: { + getState: () => testState.appState, + subscribe: () => () => {} + } +})) + +vi.mock('@/components/terminal-pane/pty-data-sidecar-subscriptions', () => ({ + subscribeToPtyData: testState.subscribeToPtyData +})) + +vi.mock('@/components/terminal-pane/pty-pre-handler-buffer', () => ({ + replayPreHandlerPtyData: testState.replayPreHandlerPtyData +})) + +vi.mock('@/runtime/runtime-terminal-inspection', () => ({ + isRemoteRuntimePtyId: testState.isRemoteRuntimePtyId, + sendRuntimePtyInputVerified: testState.sendRuntimePtyInputVerified, + inspectRuntimeTerminalProcess: testState.inspectRuntimeTerminalProcess +})) + +vi.mock('@/runtime/runtime-terminal-stream', () => ({ + subscribeToRuntimeTerminalData: testState.subscribeToRuntimeTerminalData +})) + +const DECSET_BRACKETED_PASTE = '\x1b[?2004h' +const CODEX_COMPOSER_PROMPT_RENDER = '\x1b[1m›\x1b[0m Ask Codex to do anything' +const RENDER_QUIET_MS = 1500 +const ISSUE_URL = 'https://github.com/stablyai/orca/issues/123' +const PASTED_ISSUE_URL = `\x1b[200~${ISSUE_URL}\x1b[201~` +const CODEX_SUBMIT_RETRY_DELAY_MS = TUI_AGENT_CONFIG.codex.submitRetryDelayMs ?? 0 + +describe('post-paste submit retry Enter', () => { + beforeEach(() => { + vi.useFakeTimers() + vi.stubGlobal('window', { + setTimeout: globalThis.setTimeout, + clearTimeout: globalThis.clearTimeout + }) + testState.appState.settings = {} + testState.appState.ptyIdsByTabId = { 'tab-1': ['pty-1'] } + testState.appState.tabsByWorktree = {} + testState.appState.repos = [] + testState.appState.worktreesByRepo = {} + testState.ptyObserver = null + testState.unsubscribe.mockReset() + testState.subscribeToPtyData.mockReset() + testState.subscribeToPtyData.mockImplementation( + (_ptyId: string, observer: (data: string) => void) => { + testState.ptyObserver = observer + return testState.unsubscribe + } + ) + testState.replayPreHandlerPtyData.mockReset() + testState.isRemoteRuntimePtyId.mockReset() + testState.isRemoteRuntimePtyId.mockReturnValue(false) + testState.sendRuntimePtyInputVerified.mockReset() + testState.sendRuntimePtyInputVerified.mockResolvedValue(true) + testState.inspectRuntimeTerminalProcess.mockReset() + testState.subscribeToRuntimeTerminalData.mockReset() + }) + + afterEach(() => { + vi.unstubAllGlobals() + vi.useRealTimers() + }) + + it('sends one retry Enter after the configured gap for agents that can eat the first Enter', async () => { + const promise = startCodexSubmit() + await signalCodexComposerReady() + await vi.advanceTimersByTimeAsync(POST_PASTE_SUBMIT_DELAY_MS) + + expect(enterWrites()).toHaveLength(1) + await vi.advanceTimersByTimeAsync(CODEX_SUBMIT_RETRY_DELAY_MS - 1) + expect(enterWrites()).toHaveLength(1) + await vi.advanceTimersByTimeAsync(1) + + await expect(promise).resolves.toBe(true) + expect(enterWrites()).toHaveLength(2) + expect(testState.sendRuntimePtyInputVerified).toHaveBeenLastCalledWith({}, 'pty-1', '\r') + expect(vi.getTimerCount()).toBe(0) + }) + + it('sends exactly one Enter for agents without a submit retry delay', async () => { + const promise = pasteDraftWhenAgentReady({ + tabId: 'tab-1', + content: ISSUE_URL, + agent: 'gemini', + submit: true + }) + await flushMicrotasks() + testState.ptyObserver?.(DECSET_BRACKETED_PASTE) + await vi.advanceTimersByTimeAsync(RENDER_QUIET_MS) + await flushMicrotasks() + await vi.advanceTimersByTimeAsync(POST_PASTE_SUBMIT_DELAY_MS + CODEX_SUBMIT_RETRY_DELAY_MS) + + await expect(promise).resolves.toBe(true) + expect(enterWrites()).toHaveLength(1) + expect(vi.getTimerCount()).toBe(0) + }) + + it('holds the PTY input transaction across the retry Enter', async () => { + const writes: string[] = [] + testState.sendRuntimePtyInputVerified.mockImplementation( + async (_settings: unknown, _ptyId: string, data: string) => { + writes.push(data) + return true + } + ) + + const promise = startCodexSubmit() + await signalCodexComposerReady() + await vi.advanceTimersByTimeAsync(POST_PASTE_SUBMIT_DELAY_MS) + // Competing paste on the same PTY: it must not open a frame the retry can land in. + const competing = sendAgentDraftPasteContent( + {}, + 'pty-1', + 'y'.repeat(AGENT_DRAFT_PASTE_DIRECT_MAX_BYTES + 1) + ) + await flushMicrotasks(10) + expect(writes).toEqual([PASTED_ISSUE_URL, '\r']) + + await vi.advanceTimersByTimeAsync(CODEX_SUBMIT_RETRY_DELAY_MS) + await expect(promise).resolves.toBe(true) + await expect(competing).resolves.toBe(true) + + expect(writes.slice(0, 3)).toEqual([PASTED_ISSUE_URL, '\r', '\r']) + expect(writes.at(3)).toBe('\x1b[200~') + }) + + it('keeps a successful submit successful when the retry Enter is rejected', async () => { + testState.sendRuntimePtyInputVerified + .mockResolvedValueOnce(true) + .mockResolvedValueOnce(true) + .mockRejectedValueOnce(new Error('terminal_not_writable')) + + const promise = startCodexSubmit() + await signalCodexComposerReady() + await vi.advanceTimersByTimeAsync(POST_PASTE_SUBMIT_DELAY_MS + CODEX_SUBMIT_RETRY_DELAY_MS) + + await expect(promise).resolves.toBe(true) + expect(enterWrites()).toHaveLength(2) + }) +}) + +function enterWrites(): unknown[][] { + return testState.sendRuntimePtyInputVerified.mock.calls.filter((call) => call[2] === '\r') +} + +function startCodexSubmit(): Promise { + return pasteDraftWhenAgentReady({ + tabId: 'tab-1', + content: ISSUE_URL, + agent: 'codex', + submit: true + }) +} + +async function signalCodexComposerReady(): Promise { + await flushMicrotasks() + testState.ptyObserver?.(`${DECSET_BRACKETED_PASTE}${CODEX_COMPOSER_PROMPT_RENDER}`) + await flushMicrotasks() +} + +async function flushMicrotasks(iterations = 2): Promise { + for (let index = 0; index < iterations; index += 1) { + await Promise.resolve() + } +} diff --git a/src/renderer/src/lib/agent-paste-draft.test.ts b/src/renderer/src/lib/agent-paste-draft.test.ts index a09c88901c4..c8a660f2576 100644 --- a/src/renderer/src/lib/agent-paste-draft.test.ts +++ b/src/renderer/src/lib/agent-paste-draft.test.ts @@ -13,6 +13,7 @@ import { sendBracketedPasteToRunningAgent, submitPromptToAgentPty } from './agent-paste-draft' +import { TUI_AGENT_CONFIG } from '../../../shared/tui-agent-config' const testState = vi.hoisted(() => ({ appState: { @@ -70,6 +71,7 @@ const CODEX_COMPOSER_PROMPT_RENDER = '\x1b[1m›\x1b[0m Ask Codex to do anything const CODEX_DYNAMIC_COMPOSER_PROMPT_RENDER = '\x1b[?1049h\x1b[1m›\x1b[0m Implement {feature}' const ISSUE_URL = 'https://github.com/stablyai/orca/issues/123' const PASTED_ISSUE_URL = `\x1b[200~${ISSUE_URL}\x1b[201~` +const CODEX_SUBMIT_RETRY_DELAY_MS = TUI_AGENT_CONFIG.codex.submitRetryDelayMs ?? 0 describe('pasteDraftWhenAgentReady', () => { beforeEach(() => { @@ -169,7 +171,7 @@ describe('pasteDraftWhenAgentReady', () => { 'pty-1', PASTED_ISSUE_URL ) - await vi.advanceTimersByTimeAsync(POST_PASTE_SUBMIT_DELAY_MS) + await vi.advanceTimersByTimeAsync(POST_PASTE_SUBMIT_DELAY_MS + CODEX_SUBMIT_RETRY_DELAY_MS) await expect(promise).resolves.toBe(true) expect(testState.sendRuntimePtyInputVerified).toHaveBeenNthCalledWith(2, {}, 'pty-1', '\r') expect(testState.unsubscribe).toHaveBeenCalledTimes(1) diff --git a/src/renderer/src/lib/agent-paste-draft.ts b/src/renderer/src/lib/agent-paste-draft.ts index f078383333d..72e467ee59e 100644 --- a/src/renderer/src/lib/agent-paste-draft.ts +++ b/src/renderer/src/lib/agent-paste-draft.ts @@ -129,7 +129,8 @@ export async function pasteDraftWhenAgentReady(args: { settings, ptyId, content, - submit: submit === true + submit: submit === true, + agent }) } @@ -168,7 +169,8 @@ export async function pasteDraftToAgentPtyWhenReady(args: { settings, ptyId, content, - submit: submit === true + submit: submit === true, + agent }) } @@ -197,11 +199,13 @@ async function sendBracketedPasteToAgent(args: { ptyId: string content: string submit: boolean + agent?: TuiAgent }): Promise { - const { settings = useAppStore.getState().settings, ptyId, content, submit } = args + const { settings = useAppStore.getState().settings, ptyId, content, submit, agent } = args + const submitRetryDelayMs = agent ? TUI_AGENT_CONFIG[agent]?.submitRetryDelayMs : undefined try { - // Why: paste + Enter must be one transaction, or a concurrent paste on this PTY - // can slip between them and submit a half-written prompt. + // Why: paste + Enter (+ retry Enter) must be one transaction, or a concurrent + // paste on this PTY can slip between them and submit a half-written prompt. return await runTerminalPtyInputTransaction(ptyId, async () => { const pasted = await sendAgentDraftPasteContentNow(settings, ptyId, content) if (!pasted || !submit) { @@ -212,7 +216,20 @@ async function sendBracketedPasteToAgent(args: { // Enter arrive in the same PTY write. Split the submit into the next turn so // the TUI processes bracketed-paste termination before handling Enter. await new Promise((resolve) => window.setTimeout(resolve, POST_PASTE_SUBMIT_DELAY_MS)) - return await sendRuntimePtyInputVerified(settings, ptyId, '\r') + const submitted = await sendRuntimePtyInputVerified(settings, ptyId, '\r') + + if (submitRetryDelayMs !== undefined) { + // Why: agents that render their composer before Enter is live silently eat + // the first Enter; the retry is best-effort and never downgrades `submitted`. + await new Promise((resolve) => window.setTimeout(resolve, submitRetryDelayMs)) + try { + await sendRuntimePtyInputVerified(settings, ptyId, '\r') + } catch { + // Why: a rejected retry leaves the first Enter's verdict untouched. + } + } + + return submitted }) } catch { return false diff --git a/src/shared/tui-agent-config.ts b/src/shared/tui-agent-config.ts index de3c79a68b1..4a89e1cb13b 100644 --- a/src/shared/tui-agent-config.ts +++ b/src/shared/tui-agent-config.ts @@ -42,6 +42,8 @@ export type TuiAgentConfig = { draftPasteReadySignal?: DraftPasteReadySignal /** Hard deadline for the agent's composer readiness signal. */ draftPasteReadyTimeoutMs?: number + /** Delay before one extra blind submit Enter, for agents that render their composer before Enter is live (codex); a no-op if the first Enter landed. */ + submitRetryDelayMs?: number /** Windows Shift+Enter encoding override; omitted agents keep the legacy Esc+CR path. */ windowsShiftEnterEncoding?: 'csi-u' /** Paste newlines for TUIs that read Windows console input records instead of VT paste frames. */ @@ -90,7 +92,8 @@ export const TUI_AGENT_CONFIG: Record = { windowsInputRecordPasteNewline: 'alt-enter', preflightTrust: 'codex', draftPasteReadySignal: 'codex-composer-prompt', - draftPasteReadyTimeoutMs: 20_000 + draftPasteReadyTimeoutMs: 20_000, + submitRetryDelayMs: 1200 }, autohand: { detectCmd: 'autohand',