mirror of
https://github.com/stablyai/orca.git
synced 2026-09-28 08:02:43 +00:00
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.
This commit is contained in:
@@ -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<string, unknown>,
|
||||
ptyIdsByTabId: { 'tab-1': ['pty-1'] } as Record<string, string[]>,
|
||||
runtimePaneTitlesByTabId: {},
|
||||
tabsByWorktree: {} as Record<string, { id: string }[]>,
|
||||
repos: [] as { id: string; connectionId: string | null; executionHostId?: string | null }[],
|
||||
worktreesByRepo: {} as Record<string, { id: string; repoId: string }[]>
|
||||
},
|
||||
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<boolean> {
|
||||
return pasteDraftWhenAgentReady({
|
||||
tabId: 'tab-1',
|
||||
content: ISSUE_URL,
|
||||
agent: 'codex',
|
||||
submit: true
|
||||
})
|
||||
}
|
||||
|
||||
async function signalCodexComposerReady(): Promise<void> {
|
||||
await flushMicrotasks()
|
||||
testState.ptyObserver?.(`${DECSET_BRACKETED_PASTE}${CODEX_COMPOSER_PROMPT_RENDER}`)
|
||||
await flushMicrotasks()
|
||||
}
|
||||
|
||||
async function flushMicrotasks(iterations = 2): Promise<void> {
|
||||
for (let index = 0; index < iterations; index += 1) {
|
||||
await Promise.resolve()
|
||||
}
|
||||
}
|
||||
@@ -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)
|
||||
|
||||
@@ -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<boolean> {
|
||||
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<void>((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<void>((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
|
||||
|
||||
@@ -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<TuiAgent, TuiAgentConfig> = {
|
||||
windowsInputRecordPasteNewline: 'alt-enter',
|
||||
preflightTrust: 'codex',
|
||||
draftPasteReadySignal: 'codex-composer-prompt',
|
||||
draftPasteReadyTimeoutMs: 20_000
|
||||
draftPasteReadyTimeoutMs: 20_000,
|
||||
submitRetryDelayMs: 1200
|
||||
},
|
||||
autohand: {
|
||||
detectCmd: 'autohand',
|
||||
|
||||
Reference in New Issue
Block a user