From 3440d8a34634a0782289e4c7d0f4f29f34794253 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 11 Jul 2026 02:29:32 -0700 Subject: [PATCH] fix: attach PTY dispatcher before background spawn --- config/reliability-gates.jsonc | 7 ++++--- .../pty-dispatcher-pi-routing.test.ts | 18 +++++++++++++++++ .../terminal-pane/pty-dispatcher.ts | 3 +++ .../pty-pre-handler-buffer.test.ts | 20 +++++++++++-------- .../lib/launch-agent-background-session.ts | 3 +-- ...unch-worktree-background-terminals.test.ts | 5 +++++ .../launch-worktree-background-terminals.ts | 5 +++++ 7 files changed, 48 insertions(+), 13 deletions(-) diff --git a/config/reliability-gates.jsonc b/config/reliability-gates.jsonc index 42d1855decb..e22eae5c405 100644 --- a/config/reliability-gates.jsonc +++ b/config/reliability-gates.jsonc @@ -3247,8 +3247,8 @@ "motivatingLinks": [ "https://github.com/stablyai/orca/pull/7695" ], - "invariant": "Before a PTY handler mounts, retained data, exit, and warning identities remain capped; data drains before exit; a fresh provider-reused PTY id cannot consume an older lifecycle's events; explicit reattach keeps its current backlog; and failed shutdown restore drains detached data before later live output.", - "oracle": "Drive more than 64 exit-before-handler identities, warning-producing data churn, duplicate/reused ids, fresh-spawn and explicit-reattach ordering, handler registration at cap, and failed-shutdown restoration; assert bounded eviction, exact latest-code delivery once, fresh-generation filtering, data-before-exit order, and detached-before-live output.", + "invariant": "Before a PTY handler mounts, the singleton dispatcher is already listening and retained data, exit, and warning identities remain capped; data drains before exit; a fresh provider-reused PTY id cannot consume an older lifecycle's events; explicit reattach keeps its current backlog; and failed shutdown restore drains detached data before later live output.", + "oracle": "Drive first-use background exit-before-spawn-resolution, more than 64 exit-before-handler identities, warning-producing data churn, duplicate/reused ids, fresh-spawn and explicit-reattach ordering, handler registration at cap, and failed-shutdown restoration; assert listener-first capture, bounded eviction, exact latest-code delivery once, fresh-generation filtering, data-before-exit order, and detached-before-live output.", "commands": [ "pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.test.ts src/renderer/src/components/terminal-pane/pty-transport.test.ts src/renderer/src/components/terminal-pane/pty-dispatcher-pi-routing.test.ts src/renderer/src/lib/launch-agent-background-session.test.ts src/renderer/src/lib/launch-worktree-background-terminals.test.ts" ], @@ -3273,6 +3273,7 @@ "assertions": [ "pre-handler data drains before its exit", "fresh PTY-id reuse and failed-reattach fresh fallback filter old lifecycle events while explicit daemon reattach preserves pre-connect backlog", + "first-use eager registration attaches the singleton dispatcher before a fast background PTY can emit data or exit", "a registered exit handler remains authoritative while 65 orphan exits churn through the bounded buffer" ] }, @@ -3291,7 +3292,7 @@ "command": "pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.test.ts src/renderer/src/components/terminal-pane/pty-transport.test.ts src/renderer/src/components/terminal-pane/pty-dispatcher-pi-routing.test.ts src/renderer/src/lib/launch-agent-background-session.test.ts src/renderer/src/lib/launch-worktree-background-terminals.test.ts", "result": "passed", "durationSeconds": 1, - "summary": "5 test files passed, 114 tests passed on main@20ebe858af plus the #7695 candidate. Main@20ebe858af retained the 65th oldest exit (received code 0 instead of eviction); the bounded candidate evicts it and preserves all ordering/reuse cases." + "summary": "5 test files passed, 115 tests passed on main@20ebe858af plus the #7695 candidate. Main@20ebe858af retained the 65th oldest exit (received code 0 instead of eviction); the bounded candidate evicts it and preserves listener-first and ordering/reuse cases." } ], "runtimeBudget": { diff --git a/src/renderer/src/components/terminal-pane/pty-dispatcher-pi-routing.test.ts b/src/renderer/src/components/terminal-pane/pty-dispatcher-pi-routing.test.ts index 0dc36efbc7a..8eb4bd3c480 100644 --- a/src/renderer/src/components/terminal-pane/pty-dispatcher-pi-routing.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-dispatcher-pi-routing.test.ts @@ -441,6 +441,24 @@ describe('dispatcher → transport → onTitleChange for Pi spinner', () => { expect(registeredExit).toHaveBeenCalledWith('pty-registered', 9) }) + it('attaches the dispatcher before first-use eager-buffer spawn can emit an exit', async () => { + const { captureEagerPtyBufferRegistration } = await import('./pty-dispatcher') + const eagerExit = vi.fn() + + expect(exitDispatcherCallback).toBeNull() + const registerEagerPtyBuffer = captureEagerPtyBufferRegistration() + expect(exitDispatcherCallback).not.toBeNull() + expect(window.api.pty.rendererDispatcherReady).toHaveBeenCalledOnce() + + dispatcherCallback?.({ id: 'pty-first-background', data: 'fast-output' }) + exitDispatcherCallback?.({ id: 'pty-first-background', code: 7 }) + const handle = registerEagerPtyBuffer('pty-first-background', eagerExit) + + expect(handle.flush()).toBe('fast-output') + await Promise.resolve() + expect(eagerExit).toHaveBeenCalledWith('pty-first-background', 7) + }) + it('filters old lifecycle events for delayed background eager-buffer registration', async () => { const { captureEagerPtyBufferRegistration, ensurePtyDispatcher } = await import('./pty-dispatcher') diff --git a/src/renderer/src/components/terminal-pane/pty-dispatcher.ts b/src/renderer/src/components/terminal-pane/pty-dispatcher.ts index 8e6402553e7..dffa3aceeb2 100644 --- a/src/renderer/src/components/terminal-pane/pty-dispatcher.ts +++ b/src/renderer/src/components/terminal-pane/pty-dispatcher.ts @@ -311,6 +311,9 @@ export function getEagerPtyBufferHandle(ptyId: string): EagerPtyHandle | undefin const EAGER_BUFFER_MAX_BYTES = TERMINAL_SCROLLBACK_SESSION_BUFFER_BYTE_LIMIT export function captureEagerPtyBufferRegistration(): typeof registerEagerPtyBuffer { + // Why: pty:exit is not boot-gated like pty:data. Attach the singleton before + // spawn so a first-use background PTY cannot exit into a listener-less page. + ensurePtyDispatcher() const afterCursor = capturePreHandlerPtyEventCursor() return (ptyId, onExit) => registerEagerPtyBuffer(ptyId, onExit, afterCursor) } diff --git a/src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.test.ts b/src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.test.ts index 3a2bc3ceb17..a78dea355f9 100644 --- a/src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-pre-handler-buffer.test.ts @@ -191,14 +191,18 @@ describe('pre-handler PTY buffer', () => { const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) const warningChunk = 'x'.repeat(64 * 1_024 + 1) - for (const ptyId of WARN_PTY_IDS) { - bufferPreHandlerPtyData(ptyId, warningChunk) - } - expect(warn).toHaveBeenCalledTimes(PRE_HANDLER_PTY_MAX_PTYS + 1) + try { + for (const ptyId of WARN_PTY_IDS) { + bufferPreHandlerPtyData(ptyId, warningChunk) + } + expect(warn).toHaveBeenCalledTimes(PRE_HANDLER_PTY_MAX_PTYS + 1) - // The first PTY was evicted when the 65th identity arrived. Reusing it - // must be warnable again instead of remaining forever in the warning set. - bufferPreHandlerPtyData(WARN_PTY_IDS[0], warningChunk) - expect(warn).toHaveBeenCalledTimes(PRE_HANDLER_PTY_MAX_PTYS + 2) + // The first PTY was evicted when the 65th identity arrived. Reusing it + // must be warnable again instead of remaining forever in the warning set. + bufferPreHandlerPtyData(WARN_PTY_IDS[0], warningChunk) + expect(warn).toHaveBeenCalledTimes(PRE_HANDLER_PTY_MAX_PTYS + 2) + } finally { + warn.mockRestore() + } }) }) diff --git a/src/renderer/src/lib/launch-agent-background-session.ts b/src/renderer/src/lib/launch-agent-background-session.ts index ee7a94f7612..99e11c53bc2 100644 --- a/src/renderer/src/lib/launch-agent-background-session.ts +++ b/src/renderer/src/lib/launch-agent-background-session.ts @@ -62,7 +62,6 @@ export async function launchAgentBackgroundSession( // Best-effort: continue with launch. The user can still accept the trust menu. } } - const cmdOverrides = store.settings?.agentCmdOverrides ?? {} const agentArgs = resolveTuiAgentLaunchArgs(agent, store.settings?.agentDefaultArgs) const agentEnv = resolveTuiAgentLaunchEnv(agent, store.settings?.agentDefaultEnv) const launchPlatform = repo @@ -87,7 +86,7 @@ export async function launchAgentBackgroundSession( const startupPlan = buildAgentStartupPlan({ agent, prompt: hasPrompt && !isFollowupPath ? trimmedPrompt : '', - cmdOverrides, + cmdOverrides: store.settings?.agentCmdOverrides ?? {}, agentArgs, agentEnv, platform: launchPlatform, diff --git a/src/renderer/src/lib/launch-worktree-background-terminals.test.ts b/src/renderer/src/lib/launch-worktree-background-terminals.test.ts index a33671bf9df..8700bcaf42b 100644 --- a/src/renderer/src/lib/launch-worktree-background-terminals.test.ts +++ b/src/renderer/src/lib/launch-worktree-background-terminals.test.ts @@ -8,6 +8,7 @@ const mockSetTabLayout = vi.fn() const mockUpdateTabPtyId = vi.fn() const mockClearTabPtyId = vi.fn() const mockCloseTab = vi.fn() +const mockEnsurePtyDispatcher = vi.fn() const mockRegisterEagerPtyBuffer = vi.fn() const mockGetActiveRuntimeTarget = vi.fn() @@ -52,6 +53,7 @@ vi.mock('@/lib/browser-uuid', () => ({ })) vi.mock('@/components/terminal-pane/pty-dispatcher', () => ({ + ensurePtyDispatcher: mockEnsurePtyDispatcher, registerEagerPtyBuffer: mockRegisterEagerPtyBuffer })) @@ -140,6 +142,9 @@ describe('launchWorktreeBackgroundTerminals', () => { expect(mockUpdateTabPtyId).toHaveBeenCalledWith('tab-1', 'pty-1') expect(mockUpdateTabPtyId).toHaveBeenCalledWith('tab-2', 'pty-2') expect(mockRegisterEagerPtyBuffer).toHaveBeenCalledTimes(2) + expect(mockEnsurePtyDispatcher.mock.invocationCallOrder[0]).toBeLessThan( + mockSpawn.mock.invocationCallOrder[0] ?? 0 + ) }) it('spawns setup in a split when setup launch mode requests a split', async () => { diff --git a/src/renderer/src/lib/launch-worktree-background-terminals.ts b/src/renderer/src/lib/launch-worktree-background-terminals.ts index 68173223d5b..ff6033be2d2 100644 --- a/src/renderer/src/lib/launch-worktree-background-terminals.ts +++ b/src/renderer/src/lib/launch-worktree-background-terminals.ts @@ -1,4 +1,5 @@ import { + ensurePtyDispatcher, registerEagerPtyBuffer, type EagerPtyHandle } from '@/components/terminal-pane/pty-dispatcher' @@ -177,6 +178,8 @@ async function createBackgroundTab(args: { const leafId = createBrowserUuid() store.setTabLayout(tab.id, singlePaneLayoutSnapshot(leafId)) let ptyId: string + // Why: pty:exit is not held by main's renderer-ready data gate. + ensurePtyDispatcher() const preHandlerCursor = capturePreHandlerPtyEventCursor() try { ptyId = await spawnPane({ @@ -206,6 +209,8 @@ async function addSetupSplit(args: { }): Promise { const store = useAppStore.getState() const setupLeafId = createBrowserUuid() + // Why: a setup shell may exit before spawn resolves on a first-use renderer. + ensurePtyDispatcher() const preHandlerCursor = capturePreHandlerPtyEventCursor() const setupPtyId = await spawnPane({ worktree: args.worktree,