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).
This commit is contained in:
Jinjing
2026-08-20 12:14:47 -07:00
parent dfb5d317e9
commit 7ee0d62f29
2 changed files with 63 additions and 5 deletions
@@ -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', () => {
@@ -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,