fix(agent-launch): share the composer-readiness budget across all three delivery owners (STA-3367)

The cold-boot fix was correct but landed as a single-path exception, and it
double-spent its own budget. Three follow-ups so the behavior is a system rule:

1. Split the PTY-spawn wait from the composer wait in pasteDraftWhenAgentReady.
   Both were handed the same budget, so a codex tab took up to 41s to report a
   dropped prompt. "Tab has a PTY" and "composer accepts input" are separate
   states: spawn keeps a fixed 8s, and the readiness budget now starts once the
   PTY exists, so a slow spawn can't shorten a cold composer's window.

2. Move the per-signal budget to draftPasteReadyBudgetMs() beside the shared
   readiness scanner. The budget is a property of the ready signal — only that
   module knows which signals are marker-gated — so all three delivery owners
   (renderer tab paste, renderer startup paste, main runtime startup paste)
   consume one policy instead of three hardcoded 8s constants.

3. Give the main-runtime startup paste the process-ownership fallback both
   renderer paths already have. It resolved null on budget expiry, silently
   dropping the prompt on worktree-create / CLI / remote-host delivery — the
   same STA-3367 failure, on the path the original fix didn't reach.

Adds coverage for the main-runtime waiter, which had none.

Test: vitest src/main/runtime src/shared src/renderer/src/lib
      src/renderer/src/components/terminal-pane — all green; tsc clean.
This commit is contained in:
Brennan Benson
2026-08-06 14:01:24 -07:00
parent bcd0c6e810
commit 7cf428f89e
7 changed files with 299 additions and 35 deletions
+29 -3
View File
@@ -416,7 +416,10 @@ import {
isTuiAgent,
TUI_AGENT_CONFIG
} from '../../shared/tui-agent-config'
import { createDraftPasteReadyScanner } from '../../shared/draft-paste-ready-scanner'
import {
createDraftPasteReadyScanner,
draftPasteReadyBudgetMs
} from '../../shared/draft-paste-ready-scanner'
import { detectInstalledAgentsWithShellPathHydration, detectRemoteAgents } from '../ipc/preflight'
import {
markCodexProjectTrusted,
@@ -1737,7 +1740,6 @@ const FOREGROUND_AGENT_WRAPPER_RETRY_TIMEOUT_MS = 6_500
const BRACKETED_PASTE_BEGIN = '\x1b[200~'
const BRACKETED_PASTE_END = '\x1b[201~'
const BRACKETED_PASTE_QUIET_MS = 1500
const DRAFT_PASTE_READY_TIMEOUT_MS = 8000
const MOBILE_TERMINAL_SURFACE_TIMEOUT_MS = 10_000
const MOBILE_TERMINAL_READY_FALLBACK_MS = 1000
const RECENT_PTY_PATH_CANDIDATE_LIMIT = 1024
@@ -21366,10 +21368,34 @@ export class OrcaRuntimeService {
if (replay) {
observeData(replay)
}
hardTimer = setTimeout(() => finish(null), DRAFT_PASTE_READY_TIMEOUT_MS)
// Why: a marker that never rendered is not proof the agent is absent — the
// sidecar can attach after a fast TUI's handshake, and a cold boot can
// outlast the budget. Fall back to process ownership like the renderer
// delivery paths do, instead of silently dropping the prompt (STA-3367).
hardTimer = setTimeout(() => {
void this.resolveDraftPastePtyIfAgentOwns(ptyId, agent).then(finish)
}, draftPasteReadyBudgetMs(readySignal))
})
}
/** Best-effort post-timeout check: does the launched agent own this PTY? */
private async resolveDraftPastePtyIfAgentOwns(
ptyId: string,
agent: TuiAgent
): Promise<string | null> {
if (!this.ptyController) {
return null
}
try {
const foregroundProcess = await this.ptyController.getForegroundProcess(ptyId)
return isExpectedAgentProcess(foregroundProcess, TUI_AGENT_CONFIG[agent].expectedProcess)
? ptyId
: null
} catch {
return null
}
}
async prefetchManagedWorktreeCreateBase(args: {
repoSelector: string
baseBranch?: string
@@ -0,0 +1,135 @@
/**
* Why: the main-runtime startup draft paste (worktree-create / CLI / remote host
* delivery) had no coverage, and resolved `null` on budget expiry — silently
* dropping the prompt where both renderer paths fall back to process ownership
* (STA-3367). Drives the real prototype methods through a stub host so the
* scanner, the shared budget, and the fallback are all exercised.
*/
import { beforeEach, afterEach, describe, expect, it, vi, type Mock } from 'vitest'
import { OrcaRuntimeService } from './orca-runtime'
const DECSET_BRACKETED_PASTE = '\x1b[?2004h'
const CODEX_COMPOSER_PROMPT_RENDER = '\x1b[1m›\x1b[0m Ask Codex to do anything'
const PTY_ID = 'pty-1'
const HANDLE = 'term-1'
const CODEX_MARKER_BUDGET_MS = 20000
type RuntimeHost = {
getLivePtyForHandle: (handle: string) => { pty: { ptyId: string } } | null
ptyController: { getForegroundProcess: (ptyId: string) => Promise<string | null> } | null
recentPtyOutputById: Map<string, { read: () => string }>
subscribeToTerminalData: (ptyId: string, observe: (data: string) => void) => () => void
resolveDraftPastePtyIfAgentOwns: unknown
}
// Bracket access is the sanctioned escape hatch for private members; calling the
// real prototype keeps this from degrading into a re-implementation test.
const waitForStartupDraftReady = OrcaRuntimeService.prototype['waitForStartupDraftReady'] as (
this: RuntimeHost,
handle: string,
agent: string
) => Promise<string | null>
describe('waitForStartupDraftReady', () => {
let observer: ((data: string) => void) | null
let unsubscribe: Mock<() => void>
let getForegroundProcess: Mock<(ptyId: string) => Promise<string | null>>
let host: RuntimeHost
beforeEach(() => {
vi.useFakeTimers()
observer = null
unsubscribe = vi.fn<() => void>()
getForegroundProcess = vi
.fn<(ptyId: string) => Promise<string | null>>()
.mockResolvedValue('bash')
host = {
getLivePtyForHandle: () => ({ pty: { ptyId: PTY_ID } }),
ptyController: {
getForegroundProcess: (ptyId: string) => getForegroundProcess(ptyId)
},
recentPtyOutputById: new Map(),
subscribeToTerminalData: (_ptyId, observe) => {
observer = observe
return unsubscribe
},
resolveDraftPastePtyIfAgentOwns:
OrcaRuntimeService.prototype['resolveDraftPastePtyIfAgentOwns']
}
})
afterEach(() => {
vi.useRealTimers()
})
it('resolves as soon as the codex composer glyph renders', async () => {
const promise = waitForStartupDraftReady.call(host, HANDLE, 'codex')
await Promise.resolve()
observer?.(DECSET_BRACKETED_PASTE)
observer?.(CODEX_COMPOSER_PROMPT_RENDER)
await expect(promise).resolves.toBe(PTY_ID)
expect(getForegroundProcess).not.toHaveBeenCalled()
expect(unsubscribe).toHaveBeenCalled()
expect(vi.getTimerCount()).toBe(0)
})
it('waits the full marker budget before giving up on a cold codex composer', async () => {
const promise = waitForStartupDraftReady.call(host, HANDLE, 'codex')
await Promise.resolve()
observer?.(DECSET_BRACKETED_PASTE)
// The old 8s budget would already have resolved (to null) here.
await vi.advanceTimersByTimeAsync(8000)
expect(getForegroundProcess).not.toHaveBeenCalled()
// Still inside the marker budget: a late glyph is the real ready proof, and
// must win over the ownership fallback.
await vi.advanceTimersByTimeAsync(CODEX_MARKER_BUDGET_MS - 8000 - 1)
observer?.(CODEX_COMPOSER_PROMPT_RENDER)
await expect(promise).resolves.toBe(PTY_ID)
expect(getForegroundProcess).not.toHaveBeenCalled()
})
it('delivers best-effort when the marker never renders but codex owns the PTY (STA-3367)', async () => {
// Why: the sidecar can attach after a fast TUI's handshake, and a cold boot
// can outlast the budget. Neither is proof the agent is absent, so a missing
// marker must fall back to ownership instead of dropping the prompt.
getForegroundProcess.mockResolvedValue('codex')
const promise = waitForStartupDraftReady.call(host, HANDLE, 'codex')
await Promise.resolve()
await vi.advanceTimersByTimeAsync(CODEX_MARKER_BUDGET_MS)
await expect(promise).resolves.toBe(PTY_ID)
expect(getForegroundProcess).toHaveBeenCalledWith(PTY_ID)
expect(unsubscribe).toHaveBeenCalled()
})
it('does not paste blind when the PTY fell back to the shell', async () => {
getForegroundProcess.mockResolvedValue('bash')
const promise = waitForStartupDraftReady.call(host, HANDLE, 'codex')
await Promise.resolve()
await vi.advanceTimersByTimeAsync(CODEX_MARKER_BUDGET_MS)
await expect(promise).resolves.toBeNull()
})
it('treats a failed foreground inspection as not-ready rather than throwing', async () => {
getForegroundProcess.mockRejectedValue(new Error('pty gone'))
const promise = waitForStartupDraftReady.call(host, HANDLE, 'codex')
await Promise.resolve()
await vi.advanceTimersByTimeAsync(CODEX_MARKER_BUDGET_MS)
await expect(promise).resolves.toBeNull()
})
it('returns null without subscribing when the handle has no live PTY', async () => {
host.getLivePtyForHandle = () => null
await expect(waitForStartupDraftReady.call(host, HANDLE, 'codex')).resolves.toBeNull()
expect(observer).toBeNull()
})
})
@@ -304,7 +304,10 @@ import {
import type { SetupSplitDirection, TuiAgent } from '../../../../shared/types'
import { isWslUncPath } from '../../../../shared/wsl-paths'
import { isTuiAgent, TUI_AGENT_CONFIG } from '../../../../shared/tui-agent-config'
import { createDraftPasteReadyScanner } from '../../../../shared/draft-paste-ready-scanner'
import {
createDraftPasteReadyScanner,
draftPasteReadyBudgetMs
} from '../../../../shared/draft-paste-ready-scanner'
import { sendAgentDraftPasteContent } from '@/lib/agent-draft-paste-content'
import { writeTerminalPastePtyInput } from './terminal-pty-paste-writer'
import {
@@ -343,7 +346,6 @@ const STARTUP_DRAFT_PASTE_QUIET_MS = 1500
// contain private repo/user names; the terminal itself shows where it opened.
export const STARTUP_CWD_FALLBACK_NOTICE =
'\r\n[Orca opened this terminal at the workspace root because its saved start folder no longer exists.]\r\n'
const STARTUP_DRAFT_PASTE_TIMEOUT_MS = 8000
const HIDDEN_OUTPUT_RESTORE_PENDING_CHARS = 512 * 1024
const HIDDEN_OUTPUT_RESTORE_DEFERRED_RETRY_MS = 50
const HIDDEN_OUTPUT_RESTORE_DEFERRED_RETRY_MAX = 3
@@ -4783,10 +4785,10 @@ export function connectPanePty(
}
schedulePendingStartupCommandDelivery()
}
const startupDraftReadySignal =
startupDraftAgentConfig?.draftPasteReadySignal ?? 'render-quiet-after-bracketed-paste'
const startupDraftReadyScanner = ownsStartupDraftPaste
? createDraftPasteReadyScanner(
startupDraftAgentConfig?.draftPasteReadySignal ?? 'render-quiet-after-bracketed-paste'
)
? createDraftPasteReadyScanner(startupDraftReadySignal)
: null
let startupDraftReadinessArmed = false
let startupDraftPasteSettled = !ownsStartupDraftPaste
@@ -4881,7 +4883,7 @@ export function connectPanePty(
startupDraftHardTimer = setTimeout(() => {
startupDraftHardTimer = null
void deliverStartupDraftIfAgentOwnsPty()
}, STARTUP_DRAFT_PASTE_TIMEOUT_MS)
}, draftPasteReadyBudgetMs(startupDraftReadySignal))
}
const armStartupDraftQuietTimer = (): void => {
if (!startupDraftReadyScanner || startupDraftPasteSettled) {
+57 -1
View File
@@ -17,7 +17,7 @@ import {
const testState = vi.hoisted(() => ({
appState: {
settings: {},
ptyIdsByTabId: { 'tab-1': ['pty-1'] },
ptyIdsByTabId: { 'tab-1': ['pty-1'] } as Record<string, string[]>,
runtimePaneTitlesByTabId: {},
tabsByWorktree: {} as Record<string, { id: string; title?: string }[]>,
repos: [] as { id: string; connectionId: string | null; executionHostId?: string | null }[],
@@ -458,6 +458,62 @@ describe('pasteDraftWhenAgentReady', () => {
expect(vi.getTimerCount()).toBe(0)
})
it('gives up on a never-spawned PTY at the spawn budget, not the composer budget (STA-3367)', async () => {
// Why: the two waits must not each spend the marker budget. A tab whose PTY
// never appears is a failed launch, and must not also burn the 20s cold-boot
// composer window before the caller is told.
testState.appState.ptyIdsByTabId = {}
const onTimeout = vi.fn()
const promise = pasteDraftWhenAgentReady({
tabId: 'tab-1',
content: ISSUE_URL,
agent: 'codex',
onTimeout
})
await vi.advanceTimersByTimeAsync(8000)
await flushMicrotasks(5)
await expect(promise).resolves.toBe(false)
expect(onTimeout).toHaveBeenCalledTimes(1)
expect(testState.sendRuntimePtyInputVerified).not.toHaveBeenCalled()
expect(vi.getTimerCount()).toBe(0)
})
it('starts the composer budget once the PTY exists, so a slow spawn does not shorten it', async () => {
// Why: "tab has a PTY" and "composer accepts input" are separate states. A
// spawn that eats most of a shared budget would leave a cold codex too little
// room and re-drop the prompt — the exact STA-3367 failure.
testState.appState.ptyIdsByTabId = {}
const promise = pasteDraftWhenAgentReady({
tabId: 'tab-1',
content: ISSUE_URL,
agent: 'codex'
})
// PTY takes 4s to appear — most of the old shared 8s budget.
await vi.advanceTimersByTimeAsync(4000)
testState.appState.ptyIdsByTabId = { 'tab-1': ['pty-1'] }
await vi.advanceTimersByTimeAsync(100)
await flushMicrotasks(5)
testState.ptyObserver?.(DECSET_BRACKETED_PASTE)
// 19s of cold boot after the PTY appeared: past a shared budget, inside the
// composer's own window.
await vi.advanceTimersByTimeAsync(19000)
await flushMicrotasks()
expect(testState.sendRuntimePtyInputVerified).not.toHaveBeenCalled()
testState.ptyObserver?.(CODEX_COMPOSER_PROMPT_RENDER)
await expect(promise).resolves.toBe(true)
expect(testState.sendRuntimePtyInputVerified).toHaveBeenCalledWith(
{},
'pty-1',
PASTED_ISSUE_URL
)
})
it('honors the fallback inspection deadline for pty-bound draft paste', async () => {
const onTimeout = vi.fn()
testState.inspectRuntimeTerminalProcess.mockReturnValue(new Promise(() => {}))
+16 -24
View File
@@ -1,5 +1,6 @@
import type { TuiAgent, GlobalSettings } from '../../../shared/types'
import { TUI_AGENT_CONFIG, type DraftPasteReadySignal } from '../../../shared/tui-agent-config'
import { TUI_AGENT_CONFIG } from '../../../shared/tui-agent-config'
import { draftPasteReadyBudgetMs } from '../../../shared/draft-paste-ready-scanner'
import { useAppStore } from '@/store'
import {
inspectRuntimeTerminalProcess,
@@ -38,25 +39,12 @@ export function sanitizeBracketedPasteContent(content: string): string {
return sanitizeTerminalPasteText(content)
}
// Why: deterministic signal can fail in two ways: (1) the agent never
// emits DECSET 2004 (no shipped agent does this — guarded as a fallback),
// or (2) the launch fails outright. The hard timeout caps the wait so a
// stuck launch doesn't pin a Promise forever.
const READINESS_TIMEOUT_MS = 8000
// Why: marker-gated signals (Codex '›', opencode show-cursor) are positive
// readiness proofs — the paste fires only when the marker actually renders, so a
// longer budget can never paste prematurely; it only tolerates a slow cold boot.
// First-run codex on a cold app can take >8s to mount its composer, past which
// the old budget gave up and dropped the launch prompt (STA-3367). The default
// quiet-window signal has no such proof, so it keeps the tighter budget.
const MARKER_READINESS_TIMEOUT_MS = 20000
function readinessBudgetForSignal(readySignal: DraftPasteReadySignal): number {
return readySignal === 'render-quiet-after-bracketed-paste'
? READINESS_TIMEOUT_MS
: MARKER_READINESS_TIMEOUT_MS
}
// Why: "the tab has a PTY" and "the agent's composer accepts input" are separate
// states with separate failure modes, so they get separate budgets. A PTY that
// hasn't appeared in 8s means the launch itself failed — waiting the (longer)
// composer budget on top would only delay that verdict. Keeping them distinct
// also stops one slow step from spending the other's budget (STA-3367).
const PTY_SPAWN_TIMEOUT_MS = 8000
export function getSettingsForAgentTabRuntimeOwner(
tabId: string
@@ -79,7 +67,9 @@ export function getSettingsForAgentTabRuntimeOwner(
*
* Returns true when the paste was issued, false on timeout or missing
* PTY. `onTimeout` lets the caller surface a UI hint (e.g. toast) when
* the agent doesn't reach a ready state inside `timeoutMs`.
* the agent doesn't reach a ready state. `timeoutMs` overrides the
* per-signal readiness budget only; waiting for the PTY to spawn keeps its
* own fixed budget.
*
* Readiness combines DECSET 2004 with one agent-specific follow-up signal:
* 1. `\x1b[?2004h` (DECSET 2004 — bracketed-paste-enable) on the PTY
@@ -111,14 +101,16 @@ export async function pasteDraftWhenAgentReady(args: {
}
const readySignal = agentConfig?.draftPasteReadySignal ?? 'render-quiet-after-bracketed-paste'
const budget = timeoutMs ?? readinessBudgetForSignal(readySignal)
const ptyId = await waitForPtyId(tabId, budget)
const ptyId = await waitForPtyId(tabId, PTY_SPAWN_TIMEOUT_MS)
if (!ptyId) {
onTimeout?.()
return false
}
const settings = getSettingsForAgentTabRuntimeOwner(tabId)
// Why: the readiness budget starts once the PTY exists, so a slow spawn
// shortens nothing — a cold composer still gets its full window.
const budget = timeoutMs ?? draftPasteReadyBudgetMs(readySignal)
const ready = await waitForAgentDraftInputReady(ptyId, budget, readySignal, settings)
if (!ready) {
// Why: fast-starting TUIs can emit the paste-ready escape sequence before
@@ -161,7 +153,7 @@ export async function pasteDraftToAgentPtyWhenReady(args: {
const settings = getSettingsForAgentTabRuntimeOwner(tabId)
const readySignal = agentConfig?.draftPasteReadySignal ?? 'render-quiet-after-bracketed-paste'
const budget = timeoutMs ?? readinessBudgetForSignal(readySignal)
const budget = timeoutMs ?? draftPasteReadyBudgetMs(readySignal)
const ready = await waitForAgentDraftInputReady(ptyId, budget, readySignal, settings)
if (!ready) {
const fallbackReady = agentConfig
+27 -1
View File
@@ -1,5 +1,5 @@
import { describe, expect, it } from 'vitest'
import { createDraftPasteReadyScanner } from './draft-paste-ready-scanner'
import { createDraftPasteReadyScanner, draftPasteReadyBudgetMs } from './draft-paste-ready-scanner'
const DECSET_BRACKETED_PASTE = '\x1b[?2004h'
const SHOW_CURSOR = '\x1b[?25h'
@@ -136,3 +136,29 @@ describe('createDraftPasteReadyScanner', () => {
})
})
})
describe('draftPasteReadyBudgetMs', () => {
it('gives marker-gated signals room for a cold boot (STA-3367)', () => {
// Why: the marker is a positive proof, so waiting longer can only tolerate a
// slow first-run codex — it can never paste into a composer that isn't there.
expect(draftPasteReadyBudgetMs('codex-composer-prompt')).toBe(20000)
expect(draftPasteReadyBudgetMs('render-cursor-after-bracketed-paste')).toBe(20000)
})
it('keeps the markerless quiet window tight', () => {
// Why: silence is a guess, not a proof; a longer budget just delays a wrong answer.
expect(draftPasteReadyBudgetMs('render-quiet-after-bracketed-paste')).toBe(8000)
})
it('gives every marker-gated signal a strictly larger budget than the quiet window', () => {
// Guards the invariant rather than the numbers: a new marker signal added to
// the scanner must not silently inherit the tighter markerless budget.
const quiet = draftPasteReadyBudgetMs('render-quiet-after-bracketed-paste')
for (const signal of [
'codex-composer-prompt',
'render-cursor-after-bracketed-paste'
] as const) {
expect(draftPasteReadyBudgetMs(signal)).toBeGreaterThan(quiet)
}
})
})
+27
View File
@@ -20,6 +20,33 @@ export type DraftPasteReadyScanResult = {
armQuietTimer: boolean
}
// Why: the markerless quiet-window signal guesses readiness from output silence,
// so a long budget just delays a wrong answer. Keep it tight.
const QUIET_WINDOW_READY_TIMEOUT_MS = 8000
// Why: marker-gated signals (Codex '›', opencode show-cursor) are positive
// readiness proofs — the paste fires only when the marker actually renders, so a
// longer budget can never paste prematurely; it only tolerates a slow cold boot.
// First-run codex on a cold app can take >8s to mount its composer, past which
// the old budget gave up and dropped the launch prompt (STA-3367).
const MARKER_READY_TIMEOUT_MS = 20000
/**
* How long a delivery path should wait for `readySignal` before falling back.
*
* Lives beside the scanner because the budget is a property of the signal — only
* this module knows which signals are marker-gated — not of the calling path. All
* three draft-delivery owners (renderer tab paste, renderer startup paste, main
* runtime startup paste) consume this so the policy cannot drift between them.
*
* This is a backstop, not a correctness gate: every caller must still attempt a
* best-effort delivery when the agent owns the PTY after the budget expires.
*/
export function draftPasteReadyBudgetMs(readySignal: DraftPasteReadySignal): number {
return readySignal === 'render-quiet-after-bracketed-paste'
? QUIET_WINDOW_READY_TIMEOUT_MS
: MARKER_READY_TIMEOUT_MS
}
/**
* Pure, incremental scanner shared by the renderer and main-process draft-paste
* readiness waiters so the two delivery paths (desktop-local vs runtime/SSH/