diff --git a/src/renderer/src/components/terminal-pane/pty-connection-queued-startup-consume.test.ts b/src/renderer/src/components/terminal-pane/pty-connection-queued-startup-consume.test.ts index c943066f635..a09e1584567 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection-queued-startup-consume.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection-queued-startup-consume.test.ts @@ -167,10 +167,25 @@ describe('connectPanePty queued startup consume', () => { expect(onQueuedStartupSpawned).not.toHaveBeenCalled() }) - it('does not strand the pane without a pty when the consume throws', async () => { + it('keeps a throwing consume from escaping into the spawn', async () => { const { connectPanePty } = await import('./pty-connection') const transport = createMockTransport() - spawnOnConnect(transport, 'fresh-pty') + // Mirror the transport contract: onPtySpawn is invoked from inside the connect promise, so + // anything it throws rejects that promise, strands the pane with no pty, and is far worse + // than the stale queued entry the callback exists to clear. + let escapedIntoSpawn = false + transport.connect.mockImplementation(async () => { + const onPtySpawn = createdTransportOptions[0]?.onPtySpawn as + | ((id: string) => void) + | undefined + try { + onPtySpawn?.('fresh-pty') + } catch { + escapedIntoSpawn = true + return undefined + } + return 'fresh-pty' + }) transportFactoryQueue.push(transport) const updateTabPtyId = vi.fn() @@ -185,8 +200,7 @@ describe('connectPanePty queued startup consume', () => { connectPanePty(createPane(1) as never, createManager(1) as never, deps as never) await flushAsyncTicks(20) - // onPtySpawn runs inside the connect promise: an unguarded throw would reject it and leave the - // pane with no pty at all, which is strictly worse than a stale queued entry. + expect(escapedIntoSpawn).toBe(false) expect(updateTabPtyId).toHaveBeenCalled() }) }) diff --git a/src/renderer/src/components/terminal-pane/pty-connection-types.ts b/src/renderer/src/components/terminal-pane/pty-connection-types.ts index c9aa89ec943..f502360ebea 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection-types.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection-types.ts @@ -71,7 +71,9 @@ export type PtyConnectionDeps = { > clearTabPtyId: (tabId: string, ptyId: string) => void /** Set only on the pane carrying the tab's queued startup command; called once its own fresh - * spawn exists, which is the first moment the command is guaranteed to reach a shell. */ + * spawn exists, which is the first moment a live shell exists to receive it. Not proof of + * delivery: Windows runs an argv-embedded command before this, and a POSIX shell can die + * before the shell-ready write. */ onQueuedStartupSpawned?: () => void consumeSuppressedPtyExit: (ptyId: string) => boolean isPtyShutdownPending: (ptyId: string) => boolean diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.test.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.test.ts index ada321f0cfc..b2b5fdf5d07 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.test.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.test.ts @@ -3,6 +3,7 @@ import { applyTerminalPaneCloseRequest, applyTerminalScrollbackRowsToMountedPanes, clearQueuedInitialCwdAfterFirstPane, + createQueuedStartupConsumer, getPreviousVisibleForTerminalPane, isTerminalPaneVisibilityResume, isTouchIOSUserAgent, @@ -225,6 +226,36 @@ describe('paneOwnsQueuedStartup', () => { }) }) +describe('createQueuedStartupConsumer', () => { + it('withholds the consumer from a pane that does not own the queued command', () => { + const queuedStartup = { command: 'echo queued' } + const consume = vi.fn() + + // A setup split's borrowed payload, structurally identical to the queued command. + expect( + createQueuedStartupConsumer({ command: 'echo queued' }, queuedStartup, consume) + ).toBeUndefined() + expect(createQueuedStartupConsumer(null, queuedStartup, consume)).toBeUndefined() + expect(consume).not.toHaveBeenCalled() + }) + + // Why: onPtySpawn fires again on hibernation wake and the respawn ladder. Spending the slot on a + // later spawn would drop a command queued after the first launch, without ever delivering it. + it('spends the queued command at most once across repeated spawns', () => { + const queuedStartup = { command: 'echo queued' } + const consume = vi.fn() + + const consumer = createQueuedStartupConsumer(queuedStartup, queuedStartup, consume) + expect(consumer).toBeTypeOf('function') + + consumer?.() + consumer?.() + consumer?.() + + expect(consume).toHaveBeenCalledTimes(1) + }) +}) + describe('splitPaneWithOneShotStartup', () => { it('only exposes startup to the intentional split and clears it afterwards', () => { const deps: { startup?: { command: string; env?: Record } | null } = { diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts index d831716fc37..bed4f443dfa 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts @@ -492,6 +492,32 @@ export function paneOwnsQueuedStartup( return queuedStartup != null && paneStartup === queuedStartup } +/** + * The callback that spends the tab's queued startup command, or `undefined` when this pane does + * not own it. + * + * Why one-shot: `onPtySpawn` fires on every fresh spawn a pane makes — hibernation wake, the + * respawn ladder — but only the first carried the queued command. A command queued onto the tab + * afterwards belongs to that later launch, and spending it here would drop it undelivered. + */ +export function createQueuedStartupConsumer( + paneStartup: object | null | undefined, + queuedStartup: object | null | undefined, + consume: () => void +): (() => void) | undefined { + if (!paneOwnsQueuedStartup(paneStartup, queuedStartup)) { + return undefined + } + let spent = false + return () => { + if (spent) { + return + } + spent = true + consume() + } +} + /** Scopes `deps.startup` to a single call of `splitPane()`, clearing it in `finally` so later splits do not replay the payload. */ export function splitPaneWithOneShotStartup( deps: SplitWithStartupDeps, @@ -1309,22 +1335,14 @@ export function useTerminalPaneLifecycle({ } } applyAppearance(manager) - // Why one-shot: onPtySpawn fires on every fresh spawn this pane makes (hibernation wake, - // the respawn ladder), but only the first carried the queued command. A command queued onto - // the tab afterwards belongs to that later launch, and spending it here would drop it - // without ever delivering it. - let queuedStartupSpent = false - const consumeQueuedStartupOnce = (): void => { - if (queuedStartupSpent) { - return - } - queuedStartupSpent = true - useAppStore.getState().consumeTabStartupCommand(tabId) - } - const ownsQueuedStartup = paneOwnsQueuedStartup(ptyDeps.startup, startupWithSetupSplitWait) + const onQueuedStartupSpawned = createQueuedStartupConsumer( + ptyDeps.startup, + startupWithSetupSplitWait, + () => useAppStore.getState().consumeTabStartupCommand(tabId) + ) const panePtyBinding = connectPanePty(pane, manager, { ...ptyDeps, - ...(ownsQueuedStartup ? { onQueuedStartupSpawned: consumeQueuedStartupOnce } : {}), + ...(onQueuedStartupSpawned ? { onQueuedStartupSpawned } : {}), // Why: spread order matters — spawnHints.cwd (source pane) must override ptyDeps.cwd (worktree root) so splits boot in the live cwd. ...(spawnHints?.cwd ? { cwd: spawnHints.cwd } : {}), restoredPtyIdByLeafId: spawnHints?.ptyId