From 7ee0d62f29506ed4e198aea1dd032f49b54d1797 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Thu, 20 Aug 2026 12:14:47 -0700 Subject: [PATCH] fix(terminal): prevent consuming replaced queued startup commands When a queued startup is replaced before the pane's first spawn, the one-shot guard alone still allows consuming the replacement. Add isStillQueued callback to verify the slot still holds the originally captured command (STA-4876). --- .../use-terminal-pane-lifecycle.test.ts | 51 +++++++++++++++++-- .../use-terminal-pane-lifecycle.ts | 17 ++++++- 2 files changed, 63 insertions(+), 5 deletions(-) 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 b2b5fdf5d07..053a22405d5 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 @@ -233,9 +233,9 @@ describe('createQueuedStartupConsumer', () => { // A setup split's borrowed payload, structurally identical to the queued command. expect( - createQueuedStartupConsumer({ command: 'echo queued' }, queuedStartup, consume) + createQueuedStartupConsumer({ command: 'echo queued' }, queuedStartup, consume, () => true) ).toBeUndefined() - expect(createQueuedStartupConsumer(null, queuedStartup, consume)).toBeUndefined() + expect(createQueuedStartupConsumer(null, queuedStartup, consume, () => true)).toBeUndefined() expect(consume).not.toHaveBeenCalled() }) @@ -245,7 +245,7 @@ describe('createQueuedStartupConsumer', () => { const queuedStartup = { command: 'echo queued' } const consume = vi.fn() - const consumer = createQueuedStartupConsumer(queuedStartup, queuedStartup, consume) + const consumer = createQueuedStartupConsumer(queuedStartup, queuedStartup, consume, () => true) expect(consumer).toBeTypeOf('function') consumer?.() @@ -254,6 +254,51 @@ describe('createQueuedStartupConsumer', () => { expect(consume).toHaveBeenCalledTimes(1) }) + + // Why: a replacement can land before this pane's own spawn, so the one-shot guard alone still lets + // the callback delete a command it never launched (STA-4876). + it('leaves a command that replaced the captured one queued for its own launch', () => { + const capturedStartup = { command: 'echo captured' } + let pending: object | null = capturedStartup + const consume = vi.fn(() => { + pending = null + }) + + const consumer = createQueuedStartupConsumer( + capturedStartup, + capturedStartup, + consume, + () => pending === capturedStartup + ) + const replacement = { command: 'echo replacement' } + pending = replacement + + consumer?.() + + expect(consume).not.toHaveBeenCalled() + expect(pending).toBe(replacement) + }) + + // Why: the replacement belongs to the launch that queued it, so this pane's respawn ladder must not + // reach for it after skipping its own spent chance. + it('does not spend a replacement on a later spawn of the same pane', () => { + const capturedStartup = { command: 'echo captured' } + let pending: object | null = { command: 'echo replacement' } + const consume = vi.fn() + + const consumer = createQueuedStartupConsumer( + capturedStartup, + capturedStartup, + consume, + () => pending === capturedStartup + ) + + consumer?.() + pending = capturedStartup + consumer?.() + + expect(consume).not.toHaveBeenCalled() + }) }) describe('splitPaneWithOneShotStartup', () => { 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 bed4f443dfa..91c98a4285f 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 @@ -499,11 +499,15 @@ export function paneOwnsQueuedStartup( * 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. + * + * Why `isStillQueued` on top of that guard: the replacement can also arrive before this pane's very + * first spawn, so the slot is only spent while it still holds the command this pane launched. */ export function createQueuedStartupConsumer( paneStartup: object | null | undefined, queuedStartup: object | null | undefined, - consume: () => void + consume: () => void, + isStillQueued: () => boolean ): (() => void) | undefined { if (!paneOwnsQueuedStartup(paneStartup, queuedStartup)) { return undefined @@ -513,7 +517,12 @@ export function createQueuedStartupConsumer( if (spent) { return } + // Why spent regardless: this pane's launch is its one chance at the slot; a later spawn of the + // same pane must not spend whatever command took its place. spent = true + if (!isStillQueued()) { + return + } consume() } } @@ -1338,7 +1347,11 @@ export function useTerminalPaneLifecycle({ const onQueuedStartupSpawned = createQueuedStartupConsumer( ptyDeps.startup, startupWithSetupSplitWait, - () => useAppStore.getState().consumeTabStartupCommand(tabId) + () => useAppStore.getState().consumeTabStartupCommand(tabId), + // Why `startup` and not startupWithSetupSplitWait: setup-split hands the pane a copy, so only + // the raw prop still matches the object the store holds. Read and consume run in one + // synchronous step, so nothing can queue in between. + () => useAppStore.getState().pendingStartupByTabId[tabId] === startup ) const panePtyBinding = connectPanePty(pane, manager, { ...ptyDeps,