diff --git a/src/main/daemon/daemon-foreground-confirmation-protocol.test.ts b/src/main/daemon/daemon-foreground-confirmation-protocol.test.ts index f27331ece4e..142d1706f9c 100644 --- a/src/main/daemon/daemon-foreground-confirmation-protocol.test.ts +++ b/src/main/daemon/daemon-foreground-confirmation-protocol.test.ts @@ -3,7 +3,7 @@ import { PREVIOUS_DAEMON_PROTOCOL_VERSIONS, PROTOCOL_VERSION } from './types' describe('foreground-confirmation daemon protocol', () => { it('rejects daemons from before the fresh-confirmation RPC', () => { - expect(PROTOCOL_VERSION).toBe(35) + expect(PROTOCOL_VERSION).toBe(36) expect(PREVIOUS_DAEMON_PROTOCOL_VERSIONS).toContain(19) expect(PREVIOUS_DAEMON_PROTOCOL_VERSIONS).toContain(22) expect(PREVIOUS_DAEMON_PROTOCOL_VERSIONS).toContain(23) @@ -18,5 +18,6 @@ describe('foreground-confirmation daemon protocol', () => { expect(PREVIOUS_DAEMON_PROTOCOL_VERSIONS).toContain(32) expect(PREVIOUS_DAEMON_PROTOCOL_VERSIONS).toContain(33) expect(PREVIOUS_DAEMON_PROTOCOL_VERSIONS).toContain(34) + expect(PREVIOUS_DAEMON_PROTOCOL_VERSIONS).toContain(35) }) }) diff --git a/src/main/daemon/daemon-protocol-version.test.ts b/src/main/daemon/daemon-protocol-version.test.ts index 063eb645580..dfe84947ff5 100644 --- a/src/main/daemon/daemon-protocol-version.test.ts +++ b/src/main/daemon/daemon-protocol-version.test.ts @@ -5,6 +5,7 @@ import { ASYNC_CWD_VALIDATION_DAEMON_PROTOCOL_VERSION, CODEX_SHELL_LAUNCH_PREFLIGHT_DAEMON_PROTOCOL_VERSION, COMPLETION_PROCESS_INSPECTION_PROTOCOL_VERSION, + CONTENT_ADDRESSED_SHELL_WRAPPER_DAEMON_PROTOCOL_VERSION, GET_FOREGROUND_PROCESS_PROTOCOL_VERSION, HISTORY_SEED_TRANSFER_PROTOCOL_VERSION, MODE_2031_UNSUBSCRIBE_FACT_PROTOCOL_VERSION, @@ -18,7 +19,8 @@ import { describe('daemon protocol version', () => { it('ships bounded history transfer after the 2031-unsubscribe fact', () => { - expect(PROTOCOL_VERSION).toBe(35) + expect(PROTOCOL_VERSION).toBe(36) + expect(CONTENT_ADDRESSED_SHELL_WRAPPER_DAEMON_PROTOCOL_VERSION).toBe(36) expect(ASYNC_CWD_VALIDATION_DAEMON_PROTOCOL_VERSION).toBe(35) expect(CODEX_SHELL_LAUNCH_PREFLIGHT_DAEMON_PROTOCOL_VERSION).toBe(34) expect(WSL_POSIX_CWD_DAEMON_PROTOCOL_VERSION).toBe(33) @@ -31,7 +33,7 @@ describe('daemon protocol version', () => { expect(AGENT_SESSION_CLAIM_DAEMON_PROTOCOL_VERSION).toBe(26) expect(AGENT_SESSION_CREATE_OPERATION_DAEMON_PROTOCOL_VERSION).toBe(26) expect(PREVIOUS_DAEMON_PROTOCOL_VERSIONS).toEqual( - Array.from({ length: 34 }, (_, index) => index + 1) + Array.from({ length: 35 }, (_, index) => index + 1) ) }) diff --git a/src/main/daemon/daemon-protocol-version.ts b/src/main/daemon/daemon-protocol-version.ts index dcccaf2ac16..bae2ae972dc 100644 --- a/src/main/daemon/daemon-protocol-version.ts +++ b/src/main/daemon/daemon-protocol-version.ts @@ -1,6 +1,7 @@ // Why: daemons survive app updates, so wire behavior must be version-gated. -// v35 makes cwd validation async with request-scoped spawn cancellation; older owners stay attachable. -export const PROTOCOL_VERSION = 35 +// v36 launches shells from content-addressed wrapper trees; older owners stay attachable. +export const PROTOCOL_VERSION = 36 +export const CONTENT_ADDRESSED_SHELL_WRAPPER_DAEMON_PROTOCOL_VERSION = 36 export const ASYNC_CWD_VALIDATION_DAEMON_PROTOCOL_VERSION = 35 export const CODEX_SHELL_LAUNCH_PREFLIGHT_DAEMON_PROTOCOL_VERSION = 34 export const WSL_POSIX_CWD_DAEMON_PROTOCOL_VERSION = 33 @@ -29,7 +30,7 @@ export const CLEAN_DISCONNECT_PROTOCOL_VERSION = 24 export const MODE_2031_UNSUBSCRIBE_FACT_PROTOCOL_VERSION = 29 export const PREVIOUS_DAEMON_PROTOCOL_VERSIONS = [ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, - 28, 29, 30, 31, 32, 33, 34 + 28, 29, 30, 31, 32, 33, 34, 35 ] as const export function supportsPtyStartupIngress(protocolVersion: number): boolean { diff --git a/src/renderer/src/components/terminal-pane/TerminalPane.tsx b/src/renderer/src/components/terminal-pane/TerminalPane.tsx index e611e090321..3d66ead0734 100644 --- a/src/renderer/src/components/terminal-pane/TerminalPane.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalPane.tsx @@ -731,19 +731,12 @@ function TerminalPane( const [sessionRestoredBannerPaneIds, setSessionRestoredBannerPaneIds] = useState< Map >(() => new Map()) - const consumeTabStartupCommand = useAppStore((store) => store.consumeTabStartupCommand) const [setupSplit] = useState(() => useAppStore.getState().pendingSetupSplitByTabId[tabId]) const consumeTabSetupSplit = useAppStore((store) => store.consumeTabSetupSplit) const [issueCommandSplit] = useState( () => useAppStore.getState().pendingIssueCommandSplitByTabId[tabId] ) const consumeTabIssueCommandSplit = useAppStore((store) => store.consumeTabIssueCommandSplit) - useEffect(() => { - if (startup) { - consumeTabStartupCommand(tabId) - } - }, [startup, tabId, consumeTabStartupCommand]) - useLayoutEffect(() => { if (isVisible && shouldMeasureHiddenStartup) { // Why: hidden startup measurement is first-launch only; keeping it past first visibility would let inactive tabs refit and SIGWINCH. 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 new file mode 100644 index 00000000000..a09e1584567 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/pty-connection-queued-startup-consume.test.ts @@ -0,0 +1,206 @@ +import type * as React from 'react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { flushAsyncTicks } from './pty-connection-test-async' +import { + createMockTransport, + createPane, + createManager, + type MockTransport +} from './pty-connection-test-pane-fixtures' +import { buildPaneConnectionDeps } from './pty-connection-test-deps' +import { createInitialStoreState } from './pty-connection-test-store-fixtures' +import type { StoreState } from './pty-connection-test-store-state' +import { + installTerminalTestGlobals, + restoreTerminalTestGlobals +} from './pty-connection-test-environment' + +const { + resetAndRefreshAllTerminalWebglAtlases, + scheduleTerminalWebglAtlasRecovery, + scheduleRuntimeGraphSync, + shouldSeedCacheTimerOnInitialTitle, + toastInfo, + notifyCodexPaneBoundForStaleSweep +} = vi.hoisted(() => ({ + resetAndRefreshAllTerminalWebglAtlases: vi.fn(), + scheduleTerminalWebglAtlasRecovery: vi.fn(), + scheduleRuntimeGraphSync: vi.fn(), + shouldSeedCacheTimerOnInitialTitle: vi.fn(() => false), + toastInfo: vi.fn(), + notifyCodexPaneBoundForStaleSweep: vi.fn() +})) + +let mockStoreState: StoreState +let transportFactoryQueue: MockTransport[] = [] +let createdTransportOptions: Record[] = [] +let storeSubscribers: ((state: StoreState) => void)[] = [] + +vi.mock('@/runtime/sync-runtime-graph', () => ({ scheduleRuntimeGraphSync })) + +vi.mock('@/lib/pane-manager/pane-manager-registry', async (importOriginal) => ({ + ...(await importOriginal>()), + resetAndRefreshAllTerminalWebglAtlases +})) + +vi.mock('./terminal-webgl-atlas-recovery', () => ({ scheduleTerminalWebglAtlasRecovery })) + +vi.mock('@/store', () => ({ + useAppStore: { + getState: () => mockStoreState, + subscribe: (listener: (state: StoreState) => void) => { + storeSubscribers.push(listener) + return () => { + storeSubscribers = storeSubscribers.filter((candidate) => candidate !== listener) + } + } + } +})) + +vi.mock('@/lib/agent-status', async (importOriginal) => { + const { buildAgentStatusModuleMock } = await import('./pty-connection-test-environment') + return buildAgentStatusModuleMock(await importOriginal>()) +}) + +vi.mock('./cache-timer-seeding', () => ({ shouldSeedCacheTimerOnInitialTitle })) + +vi.mock('sonner', () => ({ toast: { info: toastInfo } })) + +vi.mock('@/lib/codex-stale-pane-sweep', () => ({ notifyCodexPaneBoundForStaleSweep })) + +vi.mock('react', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + useCallback: unknown>(fn: T): T => fn + } +}) + +vi.mock('./pty-transport', () => ({ + createIpcPtyTransport: vi.fn((options: Record) => { + createdTransportOptions.push(options) + const nextTransport = transportFactoryQueue.shift() + if (!nextTransport) { + throw new Error('No mock transport queued') + } + return nextTransport + }) +})) + +vi.mock('./pty-dispatcher', async (importOriginal) => { + const actual = await importOriginal>() + return { ...actual, getEagerPtyBufferHandle: vi.fn(() => undefined) } +}) + +function spawnOnConnect(transport: MockTransport, ptyId: string): void { + transport.connect.mockImplementation(async () => { + const onPtySpawn = createdTransportOptions[0]?.onPtySpawn as ((id: string) => void) | undefined + onPtySpawn?.(ptyId) + return ptyId + }) +} + +// Why this suite exists: the queued startup command is spent through onQueuedStartupSpawned, and +// both *when* it fires and *how often* are load-bearing. Spending it before the pty is bound +// unmounts the pane mid-spawn (the entry is what holds its worktree out of the retention-budget +// force-park); spending it on a later respawn drops a command queued after the first launch +// (STA-4876). +describe('connectPanePty queued startup consume', () => { + beforeEach(() => { + vi.resetModules() + vi.clearAllMocks() + transportFactoryQueue = [] + createdTransportOptions = [] + storeSubscribers = [] + mockStoreState = createInitialStoreState(() => mockStoreState) + installTerminalTestGlobals() + }) + + afterEach(async () => { + await restoreTerminalTestGlobals() + }) + + it('spends the queued startup only after the pty is bound to the tab', async () => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport() + spawnOnConnect(transport, 'fresh-pty') + transportFactoryQueue.push(transport) + + const callOrder: string[] = [] + const deps = buildPaneConnectionDeps(() => mockStoreState, { + startup: { command: 'echo queued' }, + updateTabPtyId: vi.fn(() => { + callOrder.push('updateTabPtyId') + }), + onQueuedStartupSpawned: vi.fn(() => { + callOrder.push('onQueuedStartupSpawned') + }) + }) + + connectPanePty(createPane(1) as never, createManager(1) as never, deps as never) + await flushAsyncTicks(20) + + expect(callOrder).toContain('onQueuedStartupSpawned') + expect(callOrder.indexOf('updateTabPtyId')).toBeGreaterThanOrEqual(0) + expect(callOrder.indexOf('updateTabPtyId')).toBeLessThan( + callOrder.indexOf('onQueuedStartupSpawned') + ) + }) + + it('leaves the queued startup unspent when the spawn never reaches onPtySpawn', async () => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport() + // A retired/reattached spawn resolves without ever announcing a fresh pty. + transport.connect.mockImplementation(async () => undefined) + transportFactoryQueue.push(transport) + + const onQueuedStartupSpawned = vi.fn() + const deps = buildPaneConnectionDeps(() => mockStoreState, { + startup: { command: 'echo queued' }, + onQueuedStartupSpawned + }) + + connectPanePty(createPane(1) as never, createManager(1) as never, deps as never) + await flushAsyncTicks(20) + + // The next mount must still find the command queued, or it is lost for good. + expect(onQueuedStartupSpawned).not.toHaveBeenCalled() + }) + + it('keeps a throwing consume from escaping into the spawn', async () => { + const { connectPanePty } = await import('./pty-connection') + const transport = createMockTransport() + // 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() + const deps = buildPaneConnectionDeps(() => mockStoreState, { + startup: { command: 'echo queued' }, + updateTabPtyId, + onQueuedStartupSpawned: vi.fn(() => { + throw new Error('store blew up') + }) + }) + + connectPanePty(createPane(1) as never, createManager(1) as never, deps as never) + await flushAsyncTicks(20) + + 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 9fef3423cbf..f502360ebea 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection-types.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection-types.ts @@ -70,6 +70,11 @@ export type PtyConnectionDeps = { (paneId: number, state: PtyTransportRecoveryState | null) => void > 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 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 updateTabTitle: (tabId: string, title: string) => void diff --git a/src/renderer/src/components/terminal-pane/pty-connection.ts b/src/renderer/src/components/terminal-pane/pty-connection.ts index abcebbdc417..cbe42205377 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.ts @@ -3102,6 +3102,16 @@ export function connectPanePty( // Why: Command Code has no prompt-start hook. Seed the visible working row // once the PTY exists, then let real hook events refine or complete it. bindActivePanePty(ptyId, { seedInitialAgentStatus: true }) + // Spend the queued command only after this pane has a bound PTY. Retired panes never reach + // this point, and the pending entry keeps the worktree from being force-parked during spawn. + // This marks spawn, not delivery: Windows may run argv-embedded commands earlier, while POSIX + // delivery can still be waiting for shell-ready. + try { + deps.onQueuedStartupSpawned?.() + } catch { + // Why swallowed: this callback runs inside the connect promise, so a throw would reject the + // spawn and strand the pane with no pty at all — a far worse failure than a stale entry. + } } const onPtyRebind = (ptyId: string, replacedPtyId: string): void => { if (!canAdoptCapturedDirectSshRetryPty(ptyId)) { 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 8e18ece6f2e..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 @@ -3,10 +3,12 @@ import { applyTerminalPaneCloseRequest, applyTerminalScrollbackRowsToMountedPanes, clearQueuedInitialCwdAfterFirstPane, + createQueuedStartupConsumer, getPreviousVisibleForTerminalPane, isTerminalPaneVisibilityResume, isTouchIOSUserAgent, mapRestoredPaneTitlesByPaneId, + paneOwnsQueuedStartup, resolvePaneLinkCwd, resolvePaneSeedCwd, resolveQueuedInitialCwd, @@ -177,6 +179,128 @@ describe('resetTerminalKeyboardProtocolAfterInterrupt', () => { }) }) +// Why: onPaneCreated uses paneOwnsQueuedStartup to decide whether a pane may spend the tab's queued +// startup command. Setup/issue splits borrow the same deps.startup field for their own one-shot +// payload, so a looser test would let a split pane spend a command it never runs — re-breaking +// STA-4876 for split-setup worktrees. +describe('paneOwnsQueuedStartup', () => { + it('grants ownership only to the pane still holding the queued object', () => { + const queuedStartup = { command: 'echo queued' } + const deps: { startup?: { command: string; env?: Record } | null } = { + startup: queuedStartup + } + const ownershipAtConnect: boolean[] = [] + const observeConnect = (): void => { + ownershipAtConnect.push(paneOwnsQueuedStartup(deps.startup, queuedStartup)) + } + + // Primary pane connects first and owns the queued command. + observeConnect() + // connectPanePty took it; the lifecycle nulls the outer slot so splits cannot replay it. + deps.startup = null + splitPaneWithOneShotStartup(deps, { command: 'orca setup' }, () => { + observeConnect() + return { id: 2 } + }) + splitPaneWithOneShotStartup(deps, { command: 'orca issue' }, () => { + observeConnect() + return { id: 3 } + }) + + expect(ownershipAtConnect).toEqual([true, false, false]) + }) + + // Why this case matters: a truthiness regression ("has a startup") passes the test above, because + // the split payload is non-null there too. Only a structurally-identical payload separates them. + it('denies ownership to a split payload structurally identical to the queued command', () => { + const queuedStartup = { command: 'orca setup' } + + expect(paneOwnsQueuedStartup({ command: 'orca setup' }, queuedStartup)).toBe(false) + expect(paneOwnsQueuedStartup(queuedStartup, queuedStartup)).toBe(true) + }) + + it('denies ownership when the tab queued nothing, so an unrelated pane cannot spend a slot', () => { + expect(paneOwnsQueuedStartup(null, null)).toBe(false) + expect(paneOwnsQueuedStartup(undefined, undefined)).toBe(false) + expect(paneOwnsQueuedStartup({ command: 'orca setup' }, null)).toBe(false) + }) +}) + +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, () => true) + ).toBeUndefined() + expect(createQueuedStartupConsumer(null, queuedStartup, consume, () => true)).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, () => true) + expect(consumer).toBeTypeOf('function') + + consumer?.() + consumer?.() + consumer?.() + + 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', () => { 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 4f3254fc281..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 @@ -478,6 +478,55 @@ function resolveTerminalHomePathFromEnv(env: Record | undefined) return homeDrive && homePath ? `${homeDrive}${homePath}` : null } +/** + * Whether this pane's startup is the tab's queued command rather than a payload a split borrowed. + * + * Why reference identity and not truthiness: setup/issue splits assign their own one-shot object to + * the same `deps.startup` field, so "has a startup" would let a split pane spend a command it never + * runs — and a split's payload can be structurally identical to the queued one (STA-4876). + */ +export function paneOwnsQueuedStartup( + paneStartup: object | null | undefined, + queuedStartup: object | null | undefined +): boolean { + 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. + * + * 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, + isStillQueued: () => boolean +): (() => void) | undefined { + if (!paneOwnsQueuedStartup(paneStartup, queuedStartup)) { + return undefined + } + let spent = false + return () => { + 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() + } +} + /** 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, @@ -1295,8 +1344,18 @@ export function useTerminalPaneLifecycle({ } } applyAppearance(manager) + const onQueuedStartupSpawned = createQueuedStartupConsumer( + ptyDeps.startup, + startupWithSetupSplitWait, + () => 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, + ...(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 diff --git a/src/renderer/src/store/slices/terminal-startup-command-retention.test.ts b/src/renderer/src/store/slices/terminal-startup-command-retention.test.ts new file mode 100644 index 00000000000..ea369e54f6a --- /dev/null +++ b/src/renderer/src/store/slices/terminal-startup-command-retention.test.ts @@ -0,0 +1,78 @@ +import { describe, expect, it } from 'vitest' +import { createTestStore, makeWorktree, seedStore } from './store-test-helpers' + +const WORKTREE_ID = 'repo1::/path/wt1' + +function seedWorktreeWithTab(store: ReturnType): string { + seedStore(store, { + worktreesByRepo: { + repo1: [makeWorktree({ id: WORKTREE_ID, repoId: 'repo1', path: '/path/wt1' })] + } + }) + return store.getState().createTab(WORKTREE_ID).id +} + +// Why this suite exists: TerminalPane snapshots pendingStartupByTabId in a useState initializer, +// so whatever is missing there at mount is missing for good. The command must therefore survive +// every store event that can precede its owning pane's fresh spawn, or a quick command leaves the +// user a titled tab sitting at a bare prompt (STA-4876). +describe('queued startup command retention', () => { + it('survives a recovery remount so the next mount can still deliver it', () => { + const store = createTestStore() + const tabId = seedWorktreeWithTab(store) + store.getState().queueTabStartupCommand(tabId, { command: 'echo hi' }) + + store.getState().remountTerminalTabForRecovery(tabId) + + expect(store.getState().pendingStartupByTabId[tabId]).toEqual({ command: 'echo hi' }) + }) + + // Why: binding is pane-scoped but this map is tab-scoped. A tab routinely runs several panes + // (setup/issue splits land on the same tabId), and the sibling often binds first — if a bind + // spent the command, the pane that actually owns it would find an empty slot on remount. + it('is not spent by a sibling pane binding a pty to the same tab', () => { + const store = createTestStore() + const tabId = seedWorktreeWithTab(store) + store.getState().queueTabStartupCommand(tabId, { command: 'echo hi' }) + + store.getState().updateTabPtyId(tabId, 'pty-sibling') + + expect(store.getState().pendingStartupByTabId[tabId]).toEqual({ command: 'echo hi' }) + }) + + // Only the owning pane spends it, via this action, once its own fresh spawn exists. + it('is spent through consumeTabStartupCommand and does not replay afterwards', () => { + const store = createTestStore() + const tabId = seedWorktreeWithTab(store) + store.getState().queueTabStartupCommand(tabId, { command: 'echo hi' }) + + expect(store.getState().consumeTabStartupCommand(tabId)).toEqual({ command: 'echo hi' }) + + store.getState().remountTerminalTabForRecovery(tabId) + expect(store.getState().pendingStartupByTabId[tabId]).toBeUndefined() + expect(store.getState().consumeTabStartupCommand(tabId)).toBeNull() + }) + + // Why: retention is bounded by tab lifetime, so a spawn that never binds cannot leak forever. + it('is dropped when the tab closes without ever spawning', () => { + const store = createTestStore() + const tabId = seedWorktreeWithTab(store) + store.getState().queueTabStartupCommand(tabId, { command: 'echo hi' }) + + store.getState().closeTab(tabId) + + expect(store.getState().pendingStartupByTabId[tabId]).toBeUndefined() + }) + + it('leaves a sibling tab’s queued command alone', () => { + const store = createTestStore() + const tabId = seedWorktreeWithTab(store) + const siblingId = store.getState().createTab(WORKTREE_ID).id + store.getState().queueTabStartupCommand(tabId, { command: 'echo mine' }) + store.getState().queueTabStartupCommand(siblingId, { command: 'echo theirs' }) + + store.getState().consumeTabStartupCommand(tabId) + + expect(store.getState().pendingStartupByTabId[siblingId]).toEqual({ command: 'echo theirs' }) + }) +}) diff --git a/src/shared/local-build-compatibility-contract.json b/src/shared/local-build-compatibility-contract.json index aa2cc016f4b..7c918c73a0d 100644 --- a/src/shared/local-build-compatibility-contract.json +++ b/src/shared/local-build-compatibility-contract.json @@ -3,9 +3,9 @@ "appId": "com.stablyai.orca", "stateSchemaVersion": 1, "readableStateSchemaVersions": [1], - "daemonProtocolVersion": 35, + "daemonProtocolVersion": 36, "attachableDaemonProtocolVersions": [ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, - 27, 28, 29, 30, 31, 32, 33, 34, 35 + 27, 28, 29, 30, 31, 32, 33, 34, 35, 36 ] } diff --git a/src/shared/local-build-compatibility-contract.ts b/src/shared/local-build-compatibility-contract.ts index 4ff84aa6dfa..7e61c309180 100644 --- a/src/shared/local-build-compatibility-contract.ts +++ b/src/shared/local-build-compatibility-contract.ts @@ -3,9 +3,9 @@ export const LOCAL_BUILD_COMPATIBILITY_CONTRACT = { appId: 'com.stablyai.orca', stateSchemaVersion: 1, readableStateSchemaVersions: [1], - daemonProtocolVersion: 35, + daemonProtocolVersion: 36, attachableDaemonProtocolVersions: [ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, - 27, 28, 29, 30, 31, 32, 33, 34, 35 + 27, 28, 29, 30, 31, 32, 33, 34, 35, 36 ] } as const