diff --git a/src/main/persistence-host-admitted-terminal-membership.test.ts b/src/main/persistence-host-admitted-terminal-membership.test.ts index a19d2640db80..d71dbc9882c0 100644 --- a/src/main/persistence-host-admitted-terminal-membership.test.ts +++ b/src/main/persistence-host-admitted-terminal-membership.test.ts @@ -89,6 +89,29 @@ describe('host-admitted terminal membership survives a stale renderer replay', ( expect(persistedTabIds(store.getWorkspaceSession(), OTHER_WORKTREE)).toContain('host-tab-other') }) + it('stamps default-terminal-tab markers so a renderer persist snapshot cannot un-apply them', async () => { + const store = await createStore() + store.setWorkspaceSession(rendererSession()) + + expect( + store.persistPtyBinding({ + worktreeId: WORKTREE, + tabId: 'host-tab', + leafId: TEST_LEAF_2, + ptyId: 'host-pty', + hostAdmittedMembership: true + }) + ).toBe(true) + expect(store.getWorkspaceSession().defaultTerminalTabsAppliedByWorktreeId?.[WORKTREE]).toBe( + true + ) + + store.setWorkspaceSession(rendererSession()) + expect(store.getWorkspaceSession().defaultTerminalTabsAppliedByWorktreeId?.[WORKTREE]).toBe( + true + ) + }) + // Polarity: without the flag the renderer still owns membership, so a renderer // spawn racing its own writer must not freeze the tab list. it('leaves renderer-owned membership alone when the binding is not host-admitted', async () => { diff --git a/src/main/persistence/loading-store/pty-binding-persistence.ts b/src/main/persistence/loading-store/pty-binding-persistence.ts index 9cdb51fffe75..c3cbff0796a8 100644 --- a/src/main/persistence/loading-store/pty-binding-persistence.ts +++ b/src/main/persistence/loading-store/pty-binding-persistence.ts @@ -218,6 +218,11 @@ function applyPtyBinding( [bindingWorktreeId]: session.activeTabIdByWorktree?.[bindingWorktreeId] ?? args.tabId } } + // Why: host-initiated persist snapshots used to omit this write-once guard, so every launch or reattach treated the worktree as never having default terminals applied. + session.defaultTerminalTabsAppliedByWorktreeId = { + ...session.defaultTerminalTabsAppliedByWorktreeId, + [bindingWorktreeId]: true + } if (!isTerminalLeafId(args.leafId)) { // Why: keep legacy renderer-local pane ids out of durable leaf-keyed layout state after the UUID migration. advanceTopologyFence() diff --git a/src/main/persistence/runtime-authored-workspace-session-fields.test.ts b/src/main/persistence/runtime-authored-workspace-session-fields.test.ts index 4badf344d41f..5f5ca100f697 100644 --- a/src/main/persistence/runtime-authored-workspace-session-fields.test.ts +++ b/src/main/persistence/runtime-authored-workspace-session-fields.test.ts @@ -63,6 +63,27 @@ describe('preserving runtime-authored workspace session fields', () => { expect(preserveRuntimeAuthoredWorkspaceSessionFields(next, session())).toBe(next) expect(preserveRuntimeAuthoredWorkspaceSessionFields(next, null)).toBe(next) }) + + it('keeps default-terminal-tab markers a persist snapshot omitted', () => { + const next = preserveRuntimeAuthoredWorkspaceSessionFields(session(), { + ...session(), + defaultTerminalTabsAppliedByWorktreeId: { 'repo-1::/wt-a': true } + }) + + expect(next.defaultTerminalTabsAppliedByWorktreeId).toEqual({ 'repo-1::/wt-a': true }) + }) + + it('unions default-terminal-tab markers instead of replacing with a partial snapshot', () => { + const next = preserveRuntimeAuthoredWorkspaceSessionFields( + { ...session(), defaultTerminalTabsAppliedByWorktreeId: { 'repo-1::/wt-b': true } }, + { ...session(), defaultTerminalTabsAppliedByWorktreeId: { 'repo-1::/wt-a': true } } + ) + + expect(next.defaultTerminalTabsAppliedByWorktreeId).toEqual({ + 'repo-1::/wt-a': true, + 'repo-1::/wt-b': true + }) + }) }) function session( diff --git a/src/main/persistence/runtime-authored-workspace-session-fields.ts b/src/main/persistence/runtime-authored-workspace-session-fields.ts index a07454776cda..e87b47cd5355 100644 --- a/src/main/persistence/runtime-authored-workspace-session-fields.ts +++ b/src/main/persistence/runtime-authored-workspace-session-fields.ts @@ -1,32 +1,50 @@ import type { WorkspaceSessionState } from '../../shared/workspace-session-state-types' /** - * Keeps runtime-authored session state alive across a renderer's full write. + * Keeps session fields the renderer persist snapshot does not author across a full write. * - * A session write replaces the stored object, and the renderer builds its payload from Zustand -- - * which has no idea the runtime authority sharing this profile also persists the client-hosted - * pages it owns. Without this, every ordinary desktop session write erases them, and the loss only - * shows up a restart later when there is nothing left to rehydrate. + * A session write replaces the stored object. Zustand-built payloads omit runtime-owned + * client-hosted pages, and they omit-when-empty the write-once default-terminal-tab marker. + * Without this, those slices vanish on the next desktop write and only show up missing after + * restart. * * Callers do not opt in: the Store applies this inside setLocalWorkspaceSession and - * setHostWorkspaceSession, so the before-unload stage path inherits it too. Guarding the individual - * writers instead is what let the quit write -- the most common one there is -- erase the field. + * setHostWorkspaceSession, so the before-unload stage path inherits it too. * - * A runtime clearing its own rows writes an empty map, not `undefined`, so this can never pin a - * stale set: only an author that never mentioned the field inherits the previous one. + * A runtime clearing client-hosted pages writes an empty map, not `undefined`. The default-tab + * marker is write-once: omission means "never stamped", not "never applied". */ +function unionWriteOnceDefaultTerminalTabsApplied( + next: WorkspaceSessionState, + prior: WorkspaceSessionState | null | undefined +): WorkspaceSessionState { + const priorMarks = prior?.defaultTerminalTabsAppliedByWorktreeId + if (!priorMarks || Object.keys(priorMarks).length === 0) { + return next + } + const nextMarks = next.defaultTerminalTabsAppliedByWorktreeId + const merged = { ...priorMarks, ...nextMarks } + if (nextMarks && Object.keys(merged).length === Object.keys(nextMarks).length) { + return next + } + return { ...next, defaultTerminalTabsAppliedByWorktreeId: merged } +} + export function preserveRuntimeAuthoredWorkspaceSessionFields( next: WorkspaceSessionState, prior: WorkspaceSessionState | null | undefined ): WorkspaceSessionState { + let result = next if ( - next.clientHostedBrowserPagesByWorktree !== undefined || - prior?.clientHostedBrowserPagesByWorktree === undefined + next.clientHostedBrowserPagesByWorktree === undefined && + prior?.clientHostedBrowserPagesByWorktree !== undefined ) { - return next - } - return { - ...next, - clientHostedBrowserPagesByWorktree: prior.clientHostedBrowserPagesByWorktree + result = { + ...result, + clientHostedBrowserPagesByWorktree: prior.clientHostedBrowserPagesByWorktree + } } + // Why union: persist snapshots omit this write-once map (empty Zustand slice, omit-when-empty + // payload). Treating omission as "never applied" re-spawns default terminals on every attach. + return unionWriteOnceDefaultTerminalTabsApplied(result, prior) } diff --git a/src/main/runtime/workspace-session-worktree-id.test.ts b/src/main/runtime/workspace-session-worktree-id.test.ts new file mode 100644 index 000000000000..5f648e22dd5d --- /dev/null +++ b/src/main/runtime/workspace-session-worktree-id.test.ts @@ -0,0 +1,62 @@ +/** + * Re-keying a workspace's session rows must not invent state the user never created. + * + * `tabsByWorktree` carries three states, not two: an absent row is "never initialized" and an + * explicit empty row is the closed-last-terminal tombstone that `shouldAutoCreateInitialTerminal` + * honours. Writing `[]` for a source that had no row turns the first into the second, and the + * workspace then never gets its initial terminal. + */ +import { describe, expect, it } from 'vitest' +import { getDefaultWorkspaceSession } from '../../shared/constants' +import type { WorkspaceSessionState } from '../../shared/workspace-session-state-types' +import { canonicalizeTerminalSessionWorktreeId } from './workspace-session-worktree-id' + +const SOURCE = 'repo-1::/old/path' +const TARGET = 'repo-1::/new/path' + +function tab(id: string, worktreeId: string) { + return { + id, + ptyId: `pty-${id}`, + worktreeId, + title: id, + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } +} + +function session(overrides: Partial = {}): WorkspaceSessionState { + return { ...getDefaultWorkspaceSession(), ...overrides } +} + +describe('canonicalizeTerminalSessionWorktreeId', () => { + it('does not fabricate a tombstone for a workspace that had no tabs row', () => { + const next = session({ tabsByWorktree: {} }) + + canonicalizeTerminalSessionWorktreeId(next, SOURCE, TARGET) + + expect(Object.hasOwn(next.tabsByWorktree, TARGET)).toBe(false) + }) + + it('carries a real closed-last-terminal tombstone across the re-key', () => { + // The other direction: an explicit empty row is user intent and must survive, so the guard has + // to be about the source row's presence and not about the tabs being empty. + const next = session({ tabsByWorktree: { [SOURCE]: [] } }) + + canonicalizeTerminalSessionWorktreeId(next, SOURCE, TARGET) + + expect(Object.hasOwn(next.tabsByWorktree, TARGET)).toBe(true) + expect(next.tabsByWorktree[TARGET]).toEqual([]) + expect(Object.hasOwn(next.tabsByWorktree, SOURCE)).toBe(false) + }) + + it('re-keys the tabs and their own worktree ids', () => { + const next = session({ tabsByWorktree: { [SOURCE]: [tab('tab-1', SOURCE)] } }) + + canonicalizeTerminalSessionWorktreeId(next, SOURCE, TARGET) + + expect(next.tabsByWorktree[TARGET]?.map((entry) => entry.worktreeId)).toEqual([TARGET]) + }) +}) diff --git a/src/main/runtime/workspace-session-worktree-id.ts b/src/main/runtime/workspace-session-worktree-id.ts index ead67f93baf8..7cf3435ca2fd 100644 --- a/src/main/runtime/workspace-session-worktree-id.ts +++ b/src/main/runtime/workspace-session-worktree-id.ts @@ -8,12 +8,18 @@ export function canonicalizeTerminalSessionWorktreeId( if (sourceWorktreeId === targetWorktreeId) { return } - const tabs = session.tabsByWorktree[sourceWorktreeId] ?? [] - delete session.tabsByWorktree[sourceWorktreeId] - session.tabsByWorktree[targetWorktreeId] = tabs.map((tab) => ({ - ...tab, - worktreeId: targetWorktreeId - })) + // Why presence and not `?? []`: an absent row means never initialized, an explicit empty one is + // the closed-last-terminal tombstone. Writing `[]` for a source that had no row invents that + // tombstone, and the workspace then never gets its initial terminal. Same guard the keyed maps + // below already use. + if (Object.hasOwn(session.tabsByWorktree, sourceWorktreeId)) { + const tabs = session.tabsByWorktree[sourceWorktreeId] ?? [] + delete session.tabsByWorktree[sourceWorktreeId] + session.tabsByWorktree[targetWorktreeId] = tabs.map((tab) => ({ + ...tab, + worktreeId: targetWorktreeId + })) + } const groups = session.tabGroups?.[sourceWorktreeId] if (groups) { diff --git a/src/renderer/src/lib/default-terminal-tabs-applied-persist.test.ts b/src/renderer/src/lib/default-terminal-tabs-applied-persist.test.ts new file mode 100644 index 000000000000..f8398e9ca9f6 --- /dev/null +++ b/src/renderer/src/lib/default-terminal-tabs-applied-persist.test.ts @@ -0,0 +1,132 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { getDefaultWorkspaceSession } from '../../../shared/constants' +import { parseWorkspaceSession } from '../../../shared/workspace-session-schema' +import { useAppStore, type AppState } from '@/store' +import { applyDefaultTerminalTabs } from './worktree-default-terminal-tabs' +import { + createSessionWriteSubscriber, + type WorkspaceSessionWrite +} from './session-write-subscriber' +import { createTestStore, makeWorktree, seedStore } from '@/store/slices/store-test-helpers' + +let initialState: AppState + +const WORKTREE_ID = 'repo1::/wt-1' +const DEFAULT_TABS = { + runCommands: false, + tabs: [{ title: 'Claude' }] +} + +function preserveRuntimeAuthoredWorkspaceSessionFields>( + next: T, + prior: T +): T { + const marks = prior.defaultTerminalTabsAppliedByWorktreeId + if (!marks || typeof marks !== 'object') { + return next + } + return { + ...next, + defaultTerminalTabsAppliedByWorktreeId: { + ...marks, + ...(next.defaultTerminalTabsAppliedByWorktreeId as Record | undefined) + } + } +} + +describe('defaultTerminalTabsAppliedByWorktreeId persist round-trip', () => { + beforeEach(() => { + initialState = useAppStore.getState() + vi.useFakeTimers() + }) + + afterEach(() => { + vi.useRealTimers() + useAppStore.setState(initialState, true) + }) + + it('keeps the marker through mark, persist patch, omitted snapshot, and hydration', () => { + const persist = vi.fn<(payload: WorkspaceSessionWrite) => void>() + const cleanup = createSessionWriteSubscriber({ store: useAppStore, persist }) + + useAppStore.setState({ workspaceSessionReady: true, hydrationSucceeded: true }) + vi.advanceTimersByTime(200) + persist.mockClear() + + useAppStore.getState().markDefaultTerminalTabsApplied(WORKTREE_ID) + vi.advanceTimersByTime(200) + + expect(persist.mock.calls[0][0].patch.defaultTerminalTabsAppliedByWorktreeId).toEqual({ + [WORKTREE_ID]: true + }) + + const patched = { + ...getDefaultWorkspaceSession(), + ...persist.mock.calls[0][0].patch + } + // Persist snapshots omit this write-once slice (empty payload / renderer never mentioned it). + const omittedSnapshot = getDefaultWorkspaceSession() + const preserved = preserveRuntimeAuthoredWorkspaceSessionFields(omittedSnapshot, patched) + const parsed = parseWorkspaceSession(JSON.parse(JSON.stringify(preserved))) + expect(parsed.ok).toBe(true) + if (!parsed.ok) { + return + } + + const store = createTestStore() + seedStore(store, { + worktreesByRepo: { + repo1: [makeWorktree({ id: WORKTREE_ID, repoId: 'repo1', path: '/wt-1' })] + } + }) + store.getState().hydrateWorkspaceSession(parsed.value) + + expect(store.getState().defaultTerminalTabsAppliedByWorktreeId[WORKTREE_ID]).toBe(true) + expect( + applyDefaultTerminalTabs( + store.getState(), + WORKTREE_ID, + undefined, + undefined, + undefined, + DEFAULT_TABS, + undefined, + undefined + ) + ).toBeNull() + expect(store.getState().tabsByWorktree[WORKTREE_ID]).toBeUndefined() + cleanup() + }) + + it('does not drop in-memory markers when hydration omits the map', () => { + const store = createTestStore() + seedStore(store, { + worktreesByRepo: { + repo1: [makeWorktree({ id: WORKTREE_ID, repoId: 'repo1', path: '/wt-1' })] + } + }) + store.getState().markDefaultTerminalTabsApplied(WORKTREE_ID) + + store.getState().hydrateWorkspaceSession({ + activeRepoId: 'repo1', + activeWorktreeId: WORKTREE_ID, + activeTabId: null, + terminalLayoutsByTabId: {}, + tabsByWorktree: {} + }) + + expect(store.getState().defaultTerminalTabsAppliedByWorktreeId[WORKTREE_ID]).toBe(true) + expect( + applyDefaultTerminalTabs( + store.getState(), + WORKTREE_ID, + undefined, + undefined, + undefined, + DEFAULT_TABS, + undefined, + undefined + ) + ).toBeNull() + }) +}) diff --git a/src/renderer/src/lib/session-write-subscriber.test.ts b/src/renderer/src/lib/session-write-subscriber.test.ts index 9753d8303b71..2f31550c6c02 100644 --- a/src/renderer/src/lib/session-write-subscriber.test.ts +++ b/src/renderer/src/lib/session-write-subscriber.test.ts @@ -190,6 +190,25 @@ describe('createSessionWriteSubscriber', () => { cleanup() }) + it('persists defaultTerminalTabsAppliedByWorktreeId after markDefaultTerminalTabsApplied', () => { + const persist = vi.fn<(payload: WorkspaceSessionWrite) => void>() + const cleanup = createSessionWriteSubscriber({ store: useAppStore, persist }) + + useAppStore.setState({ workspaceSessionReady: true, hydrationSucceeded: true }) + vi.advanceTimersByTime(200) + persist.mockClear() + + const worktreeId = 'repo1::/wt-1' + useAppStore.getState().markDefaultTerminalTabsApplied(worktreeId) + vi.advanceTimersByTime(200) + + expect(persist).toHaveBeenCalledTimes(1) + expect(persist.mock.calls[0][0].patch.defaultTerminalTabsAppliedByWorktreeId).toEqual({ + [worktreeId]: true + }) + cleanup() + }) + it('writes exactly once when a relevant field changes', () => { const persist = vi.fn<(payload: WorkspaceSessionWrite) => void>() const cleanup = createSessionWriteSubscriber({ store: useAppStore, persist }) diff --git a/src/renderer/src/lib/web-runtime-worktree-terminal-after-wake.ts b/src/renderer/src/lib/web-runtime-worktree-terminal-after-wake.ts index bb3cf9f26a84..c577e143d635 100644 --- a/src/renderer/src/lib/web-runtime-worktree-terminal-after-wake.ts +++ b/src/renderer/src/lib/web-runtime-worktree-terminal-after-wake.ts @@ -21,6 +21,7 @@ import { initialAgentTabViewModeProps } from '@/lib/native-chat-initial-view-mod import { isNativeChatTranscriptLocalReadable } from '@/lib/native-chat-transcript-readability' import { getConnectionId } from '@/lib/connection-context' import { toast } from 'sonner' +import { shouldAutoCreateInitialTerminal } from '@/components/terminal/initial-terminal' export function ensureWebRuntimeWorktreeTerminalAfterWake( worktreeId: string, @@ -32,6 +33,10 @@ export function ensureWebRuntimeWorktreeTerminalAfterWake( } ): void { const state = useAppStore.getState() + const worktree = state.getKnownWorktreeById(worktreeId) + if (!worktree) { + return + } const runtimeEnvironmentId = opts && 'runtimeEnvironmentId' in opts ? (opts.runtimeEnvironmentId ?? null) @@ -70,7 +75,16 @@ export function ensureWebRuntimeWorktreeTerminalAfterWake( } const { renderableTabCount } = state.reconcileWorktreeTabModel(worktreeId) - if (tabs.length > 0 && renderableTabCount === 0) { + if (tabs.length === 0) { + if ( + !shouldAutoCreateInitialTerminal( + renderableTabCount, + Object.hasOwn(state.tabsByWorktree, worktreeId) + ) + ) { + return + } + } else if (renderableTabCount === 0) { return } } diff --git a/src/renderer/src/lib/worktree-activation-empty-remote.test.ts b/src/renderer/src/lib/worktree-activation-empty-remote.test.ts index c16a8d6a4484..9cb414eed491 100644 --- a/src/renderer/src/lib/worktree-activation-empty-remote.test.ts +++ b/src/renderer/src/lib/worktree-activation-empty-remote.test.ts @@ -170,4 +170,151 @@ describe('empty remote worktree activation', () => { ) expect(callRuntimeEnvironment).toHaveBeenCalledTimes(1) }) + + it('does not create a terminal for a workspace the user emptied', async () => { + // The second re-seed door. The stream-frame path declines on the tombstone, but it only runs + // when the mirror has rows to send; an emptied workspace routes exclusively here, and + // activation calls this on every `activateAndRevealWorktree`, not only after a wake. Reading + // the explicit empty row as "never initialized" re-seeds the workspace on every focus, which is + // the defect this PR exists to close. + const worktree = makeWorktree() + const callRuntimeEnvironment = vi.fn() + ;(globalThis as { __ORCA_WEB_CLIENT__?: boolean }).__ORCA_WEB_CLIENT__ = true + vi.stubGlobal('window', { + api: { runtimeEnvironments: { call: callRuntimeEnvironment, subscribe: vi.fn() } } + }) + + useAppStore.setState({ + repos: [ + { + id: 'repo-1', + path: REPO_PATH, + displayName: 'repo', + badgeColor: '#000000', + addedAt: 0 + } + ], + worktreesByRepo: { 'repo-1': [worktree] }, + tabsByWorktree: { [worktree.id]: [] }, + ptyIdsByTabId: {}, + settings: { + ...getDefaultSettings(ORCA_WORKSPACES_PATH), + activeRuntimeEnvironmentId: 'web-runtime-1' + }, + reconcileWorktreeTabModel: vi.fn(() => ({ + renderableTabCount: 0, + activeRenderableTabId: null + })) + }) + + ensureWebRuntimeWorktreeTerminalAfterWake(worktree.id) + await new Promise((resolve) => setTimeout(resolve, 0)) + + expect(callRuntimeEnvironment).not.toHaveBeenCalled() + }) + + it('declines when tab rows exist but none of them render', async () => { + // The arm the seed guard must not swallow in the other direction: rows are present and nothing + // renders, so the model is mid-reconcile and a create here duplicates a pane. Pinned because the + // restructure moved this line, and reverting only this arm broke no other test. + const worktree = makeWorktree() + const callRuntimeEnvironment = vi.fn() + ;(globalThis as { __ORCA_WEB_CLIENT__?: boolean }).__ORCA_WEB_CLIENT__ = true + vi.stubGlobal('window', { + api: { runtimeEnvironments: { call: callRuntimeEnvironment, subscribe: vi.fn() } } + }) + + useAppStore.setState({ + repos: [ + { + id: 'repo-1', + path: REPO_PATH, + displayName: 'repo', + badgeColor: '#000000', + addedAt: 0 + } + ], + worktreesByRepo: { 'repo-1': [worktree] }, + tabsByWorktree: { + [worktree.id]: [ + { + id: 'local-tab', + ptyId: 'dead-pty', + worktreeId: worktree.id, + title: 'Terminal', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + ] + }, + ptyIdsByTabId: {}, + settings: { + ...getDefaultSettings(ORCA_WORKSPACES_PATH), + activeRuntimeEnvironmentId: 'web-runtime-1' + }, + reconcileWorktreeTabModel: vi.fn(() => ({ + renderableTabCount: 0, + activeRenderableTabId: null + })) + }) + + ensureWebRuntimeWorktreeTerminalAfterWake(worktree.id) + await new Promise((resolve) => setTimeout(resolve, 0)) + + expect(callRuntimeEnvironment).not.toHaveBeenCalled() + }) + + it('still respawns a woke workspace whose tab rows outlived their PTYs', async () => { + // The other branch that used to share the same line: rows are present and renderable, the host + // PTYs are gone. Tombstone-irrelevant, and the case the seed guard must not swallow. + const worktree = makeWorktree() + const callRuntimeEnvironment = vi.fn().mockResolvedValueOnce({ ok: true, result: {} }) + ;(globalThis as { __ORCA_WEB_CLIENT__?: boolean }).__ORCA_WEB_CLIENT__ = true + vi.stubGlobal('window', { + api: { runtimeEnvironments: { call: callRuntimeEnvironment, subscribe: vi.fn() } } + }) + + useAppStore.setState({ + repos: [ + { + id: 'repo-1', + path: REPO_PATH, + displayName: 'repo', + badgeColor: '#000000', + addedAt: 0 + } + ], + worktreesByRepo: { 'repo-1': [worktree] }, + tabsByWorktree: { + [worktree.id]: [ + { + id: 'local-tab', + ptyId: 'dead-pty', + worktreeId: worktree.id, + title: 'Terminal', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + ] + }, + ptyIdsByTabId: {}, + settings: { + ...getDefaultSettings(ORCA_WORKSPACES_PATH), + activeRuntimeEnvironmentId: 'web-runtime-1' + }, + reconcileWorktreeTabModel: vi.fn(() => ({ + renderableTabCount: 1, + activeRenderableTabId: 'local-tab' + })) + }) + + ensureWebRuntimeWorktreeTerminalAfterWake(worktree.id) + await vi.waitFor(() => { + expect(callRuntimeEnvironment).toHaveBeenCalled() + }) + }) }) diff --git a/src/renderer/src/runtime/web-runtime-initial-terminal-bootstrap-dispatch.ts b/src/renderer/src/runtime/web-runtime-initial-terminal-bootstrap-dispatch.ts new file mode 100644 index 000000000000..d4f097a2a688 --- /dev/null +++ b/src/renderer/src/runtime/web-runtime-initial-terminal-bootstrap-dispatch.ts @@ -0,0 +1,52 @@ +import { useAppStore } from '../store' +import { createWebRuntimeSessionTerminal } from './web-runtime-session' +import type { WebRuntimeTerminalCreateOutcome } from './web-runtime-session-types' +import { + beginWebRuntimeInitialTerminalBootstrap, + endWebRuntimeInitialTerminalBootstrap, + markWebRuntimeInitialTerminalBootstrapAwaitingMirror +} from './web-runtime-initial-terminal-bootstrap' + +/** + * Claim the initial-terminal latch for this environment's worktree, create the terminal, and decide + * how the latch is released. + * + * A thrown or returned failure releases it at once: nothing was created, so a later focus may retry. + * A success whose mirrored `tabsByWorktree` row already exists releases too. A success with no row + * yet is neither: the refresh the create awaits can resolve on an empty, unconfirmed frame while the + * host does hold the tab, and releasing there let the next empty frame seed a duplicate. That case + * is parked as awaiting-mirror and released by the next frame the mirror accepts for the worktree + * (see web-runtime-initial-terminal-bootstrap.ts). + * + * Returns true only when this call owned a create that was not reported as failed, because the + * caller latches a closure-local flag on it. A failure must report false whichever way it arrives: + * releasing the shared latch alone still leaves the subscription that issued the failed create + * unable to retry for as long as its closure lives, which is the same suppression one level up. + */ +export async function dispatchWebRuntimeInitialTerminalBootstrap( + environmentId: string, + worktreeId: string +): Promise { + if (!beginWebRuntimeInitialTerminalBootstrap(environmentId, worktreeId)) { + return false + } + let outcome: WebRuntimeTerminalCreateOutcome + try { + outcome = await createWebRuntimeSessionTerminal({ worktreeId, environmentId, activate: true }) + } catch (error) { + endWebRuntimeInitialTerminalBootstrap(environmentId, worktreeId) + throw error + } + // Why check the outcome: the create reports RPC and network failures as `{ status: 'failed' }` + // rather than throwing, so the catch above never sees them. Both arms report the same way. + if (outcome.status === 'failed') { + endWebRuntimeInitialTerminalBootstrap(environmentId, worktreeId) + return false + } + if (Object.hasOwn(useAppStore.getState().tabsByWorktree, worktreeId)) { + endWebRuntimeInitialTerminalBootstrap(environmentId, worktreeId) + } else { + markWebRuntimeInitialTerminalBootstrapAwaitingMirror(environmentId, worktreeId) + } + return true +} diff --git a/src/renderer/src/runtime/web-runtime-initial-terminal-bootstrap.ts b/src/renderer/src/runtime/web-runtime-initial-terminal-bootstrap.ts new file mode 100644 index 000000000000..961a96fbdc6c --- /dev/null +++ b/src/renderer/src/runtime/web-runtime-initial-terminal-bootstrap.ts @@ -0,0 +1,103 @@ +/** + * Which runtime-owned workspaces already have an initial-terminal bootstrap in flight. + * + * The bootstrap used to be latched by a `let` inside the session-tabs subscription closure, which + * made "one focus creates at most one terminal" true only for as long as that one closure lived. + * Its effect re-runs whenever the environment, connection generation, pairing revision, or + * session-ready flag settles — all of which move during a workspace switch — so a second closure + * re-armed the flag while the first create was still in flight and seeded a second terminal + * (STA-6173). This latch outlives the closures. + * + * It is keyed by environment AND worktree, not worktree alone: a worktree id is `repoId::path` with + * no host component, so the same id can be live on two paired runtimes at once (STA-4343). Keying + * per environment lets a per-environment teardown release only its own in-flight keys — clearing + * every environment's latch would release a sibling environment's pending create and let a new + * subscription for it seed a duplicate, which is this very bug through another door. + * + * `creating` blocks every other closure while the create RPC is in flight. `awaiting-mirror` is a + * create that resolved without the mirror yet holding a row for the worktree: the host may have the + * tab and the frame simply has not landed, so the latch stays held — releasing here is what let the + * next empty frame seed a duplicate. The next frame the mirror accepts for that worktree is its + * answer either way (a row now exists and the predicate declines on its own, or the host genuinely + * has no terminal and a retry is right), so that frame releases it. Without that release a create + * whose frame never lands would suppress every later auto-seed until environment teardown. + */ +type InitialTerminalBootstrapPhase = 'creating' | 'awaiting-mirror' + +const phaseByWorktreeByEnvironment = new Map>() + +export function isWebRuntimeInitialTerminalBootstrapInFlight( + environmentId: string, + worktreeId: string +): boolean { + return phaseByWorktreeByEnvironment.get(environmentId)?.has(worktreeId) ?? false +} + +/** Claims the bootstrap for this environment's worktree; false when another closure already holds it. */ +export function beginWebRuntimeInitialTerminalBootstrap( + environmentId: string, + worktreeId: string +): boolean { + const phases = phaseByWorktreeByEnvironment.get(environmentId) + if (phases?.has(worktreeId)) { + return false + } + if (phases) { + phases.set(worktreeId, 'creating') + } else { + phaseByWorktreeByEnvironment.set(environmentId, new Map([[worktreeId, 'creating']])) + } + return true +} + +/** The create resolved but no mirrored row exists yet; hold until the mirror answers. */ +export function markWebRuntimeInitialTerminalBootstrapAwaitingMirror( + environmentId: string, + worktreeId: string +): void { + const phases = phaseByWorktreeByEnvironment.get(environmentId) + if (phases?.has(worktreeId)) { + phases.set(worktreeId, 'awaiting-mirror') + } +} + +export function endWebRuntimeInitialTerminalBootstrap( + environmentId: string, + worktreeId: string +): void { + const phases = phaseByWorktreeByEnvironment.get(environmentId) + if (!phases) { + return + } + phases.delete(worktreeId) + if (phases.size === 0) { + phaseByWorktreeByEnvironment.delete(environmentId) + } +} + +/** + * Release a bootstrap that was only waiting on the mirror. A create still in flight keeps its claim: + * releasing it on a frame is exactly the re-armed-closure race this latch exists to close. + */ +export function releaseWebRuntimeInitialTerminalBootstrapOnMirrorFrame( + environmentId: string, + worktreeId: string +): void { + if (phaseByWorktreeByEnvironment.get(environmentId)?.get(worktreeId) === 'awaiting-mirror') { + endWebRuntimeInitialTerminalBootstrap(environmentId, worktreeId) + } +} + +export function clearWebRuntimeInitialTerminalBootstrapsForEnvironment( + environmentId: string +): void { + phaseByWorktreeByEnvironment.delete(environmentId) +} + +export function clearAllWebRuntimeInitialTerminalBootstraps(): void { + phaseByWorktreeByEnvironment.clear() +} + +export function resetWebRuntimeInitialTerminalBootstrapForTests(): void { + clearAllWebRuntimeInitialTerminalBootstraps() +} diff --git a/src/renderer/src/runtime/web-session-tabs-sync-emptied-workspace-tombstone.test.ts b/src/renderer/src/runtime/web-session-tabs-sync-emptied-workspace-tombstone.test.ts new file mode 100644 index 000000000000..bde50529e64a --- /dev/null +++ b/src/renderer/src/runtime/web-session-tabs-sync-emptied-workspace-tombstone.test.ts @@ -0,0 +1,138 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { + UNPUBLISHED_WORKTREE_PUBLICATION_EPOCH, + type RuntimeMobileSessionTabsRemovedResult +} from '../../../shared/runtime-types' +import type { TerminalTab } from '../../../shared/terminal-tab-types' +import { applyWebSessionTabsSnapshot, type WebSessionTabsSyncState } from './web-session-tabs-sync' +import { + ENV, + HOST_SURFACE_ID, + LEAF_ID, + NOW, + WT, + makeSnapshot, + makeState, + resetWebSessionTabsSyncTestState +} from './web-session-tabs-sync-test-harness' + +vi.mock('../store', () => ({ + useAppStore: { + setState: vi.fn() + } +})) + +const hostTerminalSnapshot = (): ReturnType => + makeSnapshot([ + { + type: 'terminal', + id: HOST_SURFACE_ID, + parentTabId: 'host-tab-1', + leafId: LEAF_ID, + title: 'Terminal', + status: 'ready', + terminal: 'terminal-1', + isActive: true + } + ]) + +const emptySnapshot = ( + overrides: Partial> = {} +): ReturnType => + makeSnapshot([], { + snapshotVersion: 2, + activeGroupId: null, + activeTabId: null, + activeTabType: null, + ...overrides + }) + +/** + * STA-6173. The seeders read a missing terminal row as "never initialized" and an explicit empty + * one as "the user closed the last terminal" (initial-terminal.ts). The mirror used to delete the + * key, so a runtime-owned workspace could never record the second state and was re-seeded on every + * focus. + */ +describe('applyWebSessionTabsSnapshot emptied-workspace tombstone', () => { + beforeEach(resetWebSessionTabsSyncTestState) + + it('leaves an explicit empty row when the host retracts the last terminal', () => { + const mirrored = applyWebSessionTabsSnapshot( + makeState(), + hostTerminalSnapshot(), + ENV, + NOW + ) as Partial + const mirroredTabs = mirrored.tabsByWorktree?.[WT] as TerminalTab[] + expect(mirroredTabs).toHaveLength(1) + + const patch = applyWebSessionTabsSnapshot( + makeState({ tabsByWorktree: { [WT]: mirroredTabs } }), + emptySnapshot(), + ENV, + NOW + ) as Partial + + expect(patch.tabsByWorktree).toBeDefined() + expect(Object.hasOwn(patch.tabsByWorktree!, WT)).toBe(true) + expect(patch.tabsByWorktree![WT]).toEqual([]) + }) + + it('does not invent a row for a workspace the host has never had terminals in', () => { + const patch = applyWebSessionTabsSnapshot( + makeState(), + emptySnapshot(), + ENV, + NOW + ) as Partial + + expect(patch.tabsByWorktree === undefined || !Object.hasOwn(patch.tabsByWorktree, WT)).toBe( + true + ) + }) + + it('drops the row on a worktree removal frame instead of tombstoning a workspace that is gone', () => { + const mirrored = applyWebSessionTabsSnapshot( + makeState(), + hostTerminalSnapshot(), + ENV, + NOW + ) as Partial + const mirroredTabs = mirrored.tabsByWorktree?.[WT] as TerminalTab[] + + const patch = applyWebSessionTabsSnapshot( + makeState({ tabsByWorktree: { [WT]: mirroredTabs } }), + { ...emptySnapshot(), removed: true } as RuntimeMobileSessionTabsRemovedResult, + ENV, + NOW + ) as Partial + + expect(patch.tabsByWorktree).toBeDefined() + expect(Object.hasOwn(patch.tabsByWorktree!, WT)).toBe(false) + }) + + // Why: a runtime that has published nothing for a worktree still answers a forced snapshot with a + // synthesized empty frame. That is "ask me later", not "the user emptied this". + it('does not tombstone from a synthesized unpublished frame', () => { + const mirrored = applyWebSessionTabsSnapshot( + makeState(), + hostTerminalSnapshot(), + ENV, + NOW + ) as Partial + const mirroredTabs = mirrored.tabsByWorktree?.[WT] as TerminalTab[] + + const patch = applyWebSessionTabsSnapshot( + makeState({ tabsByWorktree: { [WT]: mirroredTabs } }), + emptySnapshot({ + publicationEpoch: UNPUBLISHED_WORKTREE_PUBLICATION_EPOCH, + snapshotVersion: 0 + }), + ENV, + NOW + ) as Partial + + expect(patch.tabsByWorktree).toBeDefined() + expect(Object.hasOwn(patch.tabsByWorktree!, WT)).toBe(false) + }) +}) diff --git a/src/renderer/src/runtime/web-session-tabs-sync-initial-terminal-relaunch.test.tsx b/src/renderer/src/runtime/web-session-tabs-sync-initial-terminal-relaunch.test.tsx new file mode 100644 index 000000000000..e0c6f2243112 --- /dev/null +++ b/src/renderer/src/runtime/web-session-tabs-sync-initial-terminal-relaunch.test.tsx @@ -0,0 +1,315 @@ +// @vitest-environment happy-dom + +import { act, cleanup, renderHook } from '@testing-library/react' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { RuntimeMobileSessionTabsResult } from '../../../shared/runtime-types' +import { resetStaleDocumentVisibilityForTesting } from '@/components/terminal-pane/stale-document-visibility' +import type * as WorktreeRuntimeOwnerModule from '@/lib/worktree-runtime-owner' +import type * as WebRuntimeSessionModule from './web-runtime-session' + +const mocks = vi.hoisted(() => ({ + createTerminal: vi.fn(), + getExplicitRuntimeEnvironmentIdForWorktree: vi.fn(), + recoverSnapshot: vi.fn(), + runtimeSessionMirrorEnvironmentKey: vi.fn(), + runtimeSessionMirrorEnvironmentKeys: vi.fn(() => ({ + environmentKey: null, + resubscribeSignal: 0 + })) +})) + +vi.mock('./use-runtime-session-mirror-environment-key', () => ({ + useRuntimeSessionMirrorEnvironmentKey: mocks.runtimeSessionMirrorEnvironmentKey, + useRuntimeSessionMirrorEnvironmentKeys: mocks.runtimeSessionMirrorEnvironmentKeys +})) + +vi.mock('@/lib/worktree-runtime-owner', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + getExplicitRuntimeEnvironmentIdForWorktree: mocks.getExplicitRuntimeEnvironmentIdForWorktree + } +}) + +vi.mock('./web-session-terminal-orphan-recovery', () => ({ + recoverWebSessionTerminalOrphansBeforeApply: mocks.recoverSnapshot +})) + +vi.mock('./web-runtime-session', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, createWebRuntimeSessionTerminal: mocks.createTerminal } +}) + +import { useAppStore } from '@/store' +import type { PublicKnownRuntimeEnvironment } from '../../../shared/runtime-environments' +import type { AppState } from '@/store/types' +import { replaceRuntimeEnvironmentRevisions } from './runtime-environment-revision' +import { + resetWebSessionTabsSnapshotFreshnessForTests, + useWebSessionTabsSync +} from './web-session-tabs-sync' +import { clearRuntimeEnvironmentConnectionGenerationsForTests } from '@/store/slices/runtime-status' +import { resetWebRuntimeInitialTerminalBootstrapForTests } from './web-runtime-initial-terminal-bootstrap' + +const ENV = 'env-a' +const WORKTREE = 'repo-a::worktree-a' +const REVISION = 101 +const MIRROR_KEY = 'env-a::runtime-a::1' +const initialState = useAppStore.getInitialState() + +type RuntimeSubscribe = typeof window.api.runtimeEnvironments.subscribe +type RuntimeSubscription = { + request: Parameters[0] + callbacks: Parameters[1] + unsubscribe: ReturnType +} + +const subscriptions: RuntimeSubscription[] = [] +const runtimeCall = vi.fn(async () => ({ + id: 'list-all', + ok: true as const, + result: { snapshots: [] }, + _meta: { runtimeId: 'runtime-a' } +})) +const runtimeSubscribe = vi.fn(async (request, callbacks) => { + const unsubscribe = vi.fn() + subscriptions.push({ request, callbacks, unsubscribe }) + return { unsubscribe, sendBinary: vi.fn() } +}) + +function createDeferred(): { promise: Promise; resolve: (value: T) => void } { + let resolve = (_value: T): void => {} + const promise = new Promise((done) => { + resolve = done + }) + return { promise, resolve } +} + +async function settle(): Promise { + await Promise.resolve() + await Promise.resolve() + await Promise.resolve() +} + +function emptyActiveSnapshot(snapshotVersion: number): RuntimeMobileSessionTabsResult { + return { + worktree: WORKTREE, + publicationEpoch: 'epoch-1', + snapshotVersion, + activeGroupId: null, + activeTabId: null, + activeTabType: null, + tabs: [] + } +} + +function findActiveSubscription(occurrence: number): RuntimeSubscription { + const matches = subscriptions.filter(({ request }) => request.method === 'session.tabs.subscribe') + const subscription = matches[occurrence] + if (!subscription) { + throw new Error(`Missing active subscription ${occurrence}`) + } + return subscription +} + +async function publish(subscription: RuntimeSubscription, result: unknown): Promise { + await act(async () => { + subscription.callbacks.onResponse({ + id: 'subscription-event', + ok: true as const, + result, + _meta: { runtimeId: 'runtime-a' } + } as never) + await settle() + }) +} + +function runtimeStatusMap(connectionGeneration: number): AppState['runtimeStatusByEnvironmentId'] { + return new Map([ + [ENV, { status: { runtimeId: 'runtime-a' }, connectionGeneration }] + ]) as AppState['runtimeStatusByEnvironmentId'] +} + +function seedRemoteMirrorState(connectionGeneration: number): void { + const runtimeEnvironments = [ + { id: ENV, createdAt: 100, pairingRevision: REVISION } + ] as PublicKnownRuntimeEnvironment[] + replaceRuntimeEnvironmentRevisions(runtimeEnvironments) + useAppStore.setState( + { + ...initialState, + activeWorktreeId: WORKTREE, + workspaceSessionReady: true, + runtimeEnvironments, + runtimeStatusByEnvironmentId: runtimeStatusMap(connectionGeneration) + }, + true + ) +} + +describe('useWebSessionTabsSync initial-terminal bootstrap across an effect re-run', () => { + beforeEach(() => { + subscriptions.length = 0 + runtimeCall.mockClear() + runtimeSubscribe.mockClear() + mocks.createTerminal.mockReset() + mocks.recoverSnapshot.mockReset().mockImplementation(async (_state, snapshot) => snapshot) + mocks.getExplicitRuntimeEnvironmentIdForWorktree.mockReset().mockReturnValue(ENV) + mocks.runtimeSessionMirrorEnvironmentKey.mockReset().mockReturnValue(MIRROR_KEY) + Object.defineProperty(window, 'api', { + configurable: true, + value: { runtimeEnvironments: { call: runtimeCall, subscribe: runtimeSubscribe } } + }) + resetWebSessionTabsSnapshotFreshnessForTests() + resetWebRuntimeInitialTerminalBootstrapForTests() + seedRemoteMirrorState(1) + }) + + afterEach(() => { + cleanup() + useAppStore.setState(initialState, true) + replaceRuntimeEnvironmentRevisions([]) + resetWebSessionTabsSnapshotFreshnessForTests() + resetWebRuntimeInitialTerminalBootstrapForTests() + clearRuntimeEnvironmentConnectionGenerationsForTests() + resetStaleDocumentVisibilityForTesting() + }) + + // STA-6173, second defect. Returning to an emptied runtime-owned workspace re-runs the active + // session-tabs effect (its environment / connection-generation / pairing / session-ready deps all + // settle during a switch), installing a fresh subscription closure. The old per-closure + // `requestedInitialTerminal` flag reset to false in the new closure, so a second empty frame + // seeded a second terminal while the first create was still in flight. The module-scoped latch + // outlives the closures, so exactly one create is issued. + it('does not seed a second terminal when the effect re-runs before the first create settles', async () => { + const pendingCreate = createDeferred() + mocks.createTerminal.mockReturnValue(pendingCreate.promise) + + const hook = renderHook(() => useWebSessionTabsSync()) + await act(settle) + + await publish(findActiveSubscription(0), { type: 'snapshot', ...emptyActiveSnapshot(1) }) + expect(mocks.createTerminal).toHaveBeenCalledTimes(1) + + // Force the active effect to tear down and reinstall a fresh closure, exactly as switching back + // to the workspace does, while the first create is still unresolved. + act(() => { + useAppStore.setState({ runtimeStatusByEnvironmentId: runtimeStatusMap(2) }) + }) + await act(settle) + expect( + subscriptions.filter(({ request }) => request.method === 'session.tabs.subscribe') + ).toHaveLength(2) + + await publish(findActiveSubscription(1), { type: 'snapshot', ...emptyActiveSnapshot(2) }) + expect(mocks.createTerminal).toHaveBeenCalledTimes(1) + + pendingCreate.resolve(undefined) + await act(settle) + hook.unmount() + }) + + // CodeRabbit review: the create awaits its own snapshot refresh, but that refresh can resolve on + // an empty, unconfirmed frame that leaves no `tabsByWorktree` row. Releasing the latch on settle + // then would let the next effect re-run seed a second terminal even though the first create + // succeeded. The latch is held until a row exists, so the re-run declines. + it('does not seed again after a create that resolved without mirroring a row', async () => { + // The create succeeds but writes no tabsByWorktree row (host has not published the tab yet). + mocks.createTerminal.mockResolvedValue({ status: 'created' }) + + const hook = renderHook(() => useWebSessionTabsSync()) + await act(settle) + + await publish(findActiveSubscription(0), { type: 'snapshot', ...emptyActiveSnapshot(1) }) + expect(mocks.createTerminal).toHaveBeenCalledTimes(1) + expect(useAppStore.getState().tabsByWorktree[WORKTREE]).toBeUndefined() + + // Re-run the active effect (workspace switch back) after the create already settled. + act(() => { + useAppStore.setState({ runtimeStatusByEnvironmentId: runtimeStatusMap(2) }) + }) + await act(settle) + + await publish(findActiveSubscription(1), { type: 'snapshot', ...emptyActiveSnapshot(2) }) + expect(mocks.createTerminal).toHaveBeenCalledTimes(1) + hook.unmount() + }) + + // pullfrog review: `createWebRuntimeSessionTerminal` reports RPC and network failures as a + // returned `{ status: 'failed' }`, never a throw. A latch released only in `catch` or once a row + // exists therefore stays held after a failed create and suppresses every later auto-seed for the + // worktree until teardown. A returned failure must release it so the next focus can retry. + it('retries the bootstrap on the next focus after a create that returned failed', async () => { + mocks.createTerminal.mockResolvedValue({ status: 'failed', message: 'host unreachable' }) + + const hook = renderHook(() => useWebSessionTabsSync()) + await act(settle) + + await publish(findActiveSubscription(0), { type: 'snapshot', ...emptyActiveSnapshot(1) }) + expect(mocks.createTerminal).toHaveBeenCalledTimes(1) + expect(useAppStore.getState().tabsByWorktree[WORKTREE]).toBeUndefined() + + // The next focus installs a fresh closure; with the latch released, it may seed again. + act(() => { + useAppStore.setState({ runtimeStatusByEnvironmentId: runtimeStatusMap(2) }) + }) + await act(settle) + + await publish(findActiveSubscription(1), { type: 'snapshot', ...emptyActiveSnapshot(2) }) + expect(mocks.createTerminal).toHaveBeenCalledTimes(2) + hook.unmount() + }) + + // Third suppression path (review): the returned-failure release above frees the *module* latch, + // but the caller sets its closure-local `requestedInitialTerminal` whenever the dispatch reports + // it owned the create — including when that create returned `{ status: 'failed' }`. A thrown + // failure never sets it and retries on the very next frame; since the create reports every RPC + // and network failure as a return rather than a throw, the live path was the one that suppressed + // the whole subscription. A failed create must leave the closure free to retry, exactly like a + // thrown one. + it('retries on the next frame of the same subscription after a create that returned failed', async () => { + mocks.createTerminal.mockResolvedValue({ status: 'failed', message: 'host unreachable' }) + + const hook = renderHook(() => useWebSessionTabsSync()) + await act(settle) + + const subscription = findActiveSubscription(0) + await publish(subscription, { type: 'snapshot', ...emptyActiveSnapshot(1) }) + expect(mocks.createTerminal).toHaveBeenCalledTimes(1) + expect(useAppStore.getState().tabsByWorktree[WORKTREE]).toBeUndefined() + + // Same closure, next accepted frame: nothing was created, so the workspace is still + // never-initialized and the retry is the only thing that will ever give it a terminal. + await publish(subscription, { type: 'snapshot', ...emptyActiveSnapshot(2) }) + expect(mocks.createTerminal).toHaveBeenCalledTimes(2) + hook.unmount() + }) + + // Readiness review: the inverse hazard of the test above. A create that succeeds but whose frame + // never lands (host accepted, the mirror never got a row) must not hold the latch until environment + // teardown. The frame right after the settle is the mirror's answer and may not seed (pre-mirror + // window); the one after it decides on real state — no row, host affirms empty — and retries. + it('retries after a successful create that never mirrored a row, once the mirror has answered', async () => { + mocks.createTerminal.mockResolvedValue({ status: 'created' }) + + const hook = renderHook(() => useWebSessionTabsSync()) + await act(settle) + + await publish(findActiveSubscription(0), { type: 'snapshot', ...emptyActiveSnapshot(1) }) + expect(mocks.createTerminal).toHaveBeenCalledTimes(1) + expect(useAppStore.getState().tabsByWorktree[WORKTREE]).toBeUndefined() + + // The mirror's answer: still empty. Releases the parked bootstrap; must not itself seed. + await publish(findActiveSubscription(0), { type: 'snapshot', ...emptyActiveSnapshot(2) }) + expect(mocks.createTerminal).toHaveBeenCalledTimes(1) + + // A later focus installs a fresh closure; with the latch released, its empty frame may retry. + act(() => { + useAppStore.setState({ runtimeStatusByEnvironmentId: runtimeStatusMap(2) }) + }) + await act(settle) + await publish(findActiveSubscription(1), { type: 'snapshot', ...emptyActiveSnapshot(3) }) + expect(mocks.createTerminal).toHaveBeenCalledTimes(2) + hook.unmount() + }) +}) diff --git a/src/renderer/src/runtime/web-session-tabs-sync-terminal-bootstrap.test.ts b/src/renderer/src/runtime/web-session-tabs-sync-terminal-bootstrap.test.ts index b34a86ebd3e7..07c4007013bd 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync-terminal-bootstrap.test.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync-terminal-bootstrap.test.ts @@ -6,6 +6,7 @@ import { shouldRespawnWebRuntimeTerminalAfterWake, shouldSyncRuntimeSessionTabs } from './web-session-tabs-sync' +import { UNPUBLISHED_WORKTREE_PUBLICATION_EPOCH } from '../../../shared/runtime-types' import { ENV, HOST_SURFACE_ID, @@ -14,6 +15,19 @@ import { makeSnapshot, resetWebSessionTabsSyncTestState } from './web-session-tabs-sync-test-harness' +import { + beginWebRuntimeInitialTerminalBootstrap, + endWebRuntimeInitialTerminalBootstrap, + isWebRuntimeInitialTerminalBootstrapInFlight, + markWebRuntimeInitialTerminalBootstrapAwaitingMirror, + releaseWebRuntimeInitialTerminalBootstrapOnMirrorFrame +} from './web-runtime-initial-terminal-bootstrap' + +const OTHER_ENV = 'web-env-2' +import { + clearWebSessionTabsTrackingForWorktree, + clearWebSessionTabsTrackingForEnvironment +} from './web-session-tabs-sync/tracking-lifecycle' vi.mock('../store', () => ({ useAppStore: { @@ -55,7 +69,8 @@ describe('applyWebSessionTabsSnapshot', () => { activeWorktreeId: WT, requestedInitialTerminal: false, snapshotIsFresh: staleIsFresh, - localTerminalCount: 0 + localTerminalCount: 0, + hasPersistedTerminalState: false }) ).toBe(false) }) @@ -73,11 +88,171 @@ describe('applyWebSessionTabsSnapshot', () => { activeWorktreeId: WT, requestedInitialTerminal: false, snapshotIsFresh: true, - localTerminalCount: 1 + localTerminalCount: 1, + hasPersistedTerminalState: true + }) + ).toBe(false) + }) + + // Why: the workspace the user emptied on purpose. STA-6173 — the mirror used to delete the row, + // which reads back as "never initialized", so every focus of a runtime-owned workspace seeded a + // terminal the local path had long since stopped seeding. + it('does not bootstrap a terminal when an explicit empty row records the closed last terminal', () => { + const freshEmpty = makeSnapshot([], { + activeGroupId: null, + activeTabId: null, + activeTabType: null + }) + + expect( + shouldBootstrapInitialWebRuntimeTerminal({ + event: { type: 'snapshot', ...freshEmpty }, + activeWorktreeId: WT, + requestedInitialTerminal: false, + snapshotIsFresh: true, + localTerminalCount: 0, + hasPersistedTerminalState: true + }) + ).toBe(false) + }) + + it('bootstraps a terminal for a workspace that has no terminal row at all', () => { + const freshEmpty = makeSnapshot([], { + activeGroupId: null, + activeTabId: null, + activeTabType: null + }) + + expect( + shouldBootstrapInitialWebRuntimeTerminal({ + event: { type: 'snapshot', ...freshEmpty }, + activeWorktreeId: WT, + requestedInitialTerminal: false, + snapshotIsFresh: true, + localTerminalCount: 0, + hasPersistedTerminalState: false + }) + ).toBe(true) + }) + + // Why: a runtime that has published nothing for a worktree still answers a forced snapshot with a + // synthesized empty frame (`UNPUBLISHED_WORKTREE_PUBLICATION_EPOCH` at version 0). That is "ask me + // later", not a host with zero terminals — seeding on it can duplicate a pane the host is about to + // republish after a restart, so it must decline even when no terminal row exists yet. + it('does not bootstrap on a synthesized unpublished frame with no terminal row', () => { + const unpublished = makeSnapshot([], { + publicationEpoch: UNPUBLISHED_WORKTREE_PUBLICATION_EPOCH, + snapshotVersion: 0, + activeGroupId: null, + activeTabId: null, + activeTabType: null + }) + + expect( + shouldBootstrapInitialWebRuntimeTerminal({ + event: { type: 'snapshot', ...unpublished }, + activeWorktreeId: WT, + requestedInitialTerminal: false, + snapshotIsFresh: true, + localTerminalCount: 0, + hasPersistedTerminalState: false }) ).toBe(false) }) + // Why: the shared latch outlives the subscription closures, so a create RPC that never settles + // during a disconnect would leave the per-worktree key set and suppress the next bootstrap after + // reconnect. Tracking teardown must release it, mirroring the wake-respawn latch. + it('releases the in-flight bootstrap latch when worktree tracking is cleared', () => { + expect(beginWebRuntimeInitialTerminalBootstrap(ENV, WT)).toBe(true) + expect(isWebRuntimeInitialTerminalBootstrapInFlight(ENV, WT)).toBe(true) + + clearWebSessionTabsTrackingForWorktree(ENV, WT) + + expect(isWebRuntimeInitialTerminalBootstrapInFlight(ENV, WT)).toBe(false) + expect(beginWebRuntimeInitialTerminalBootstrap(ENV, WT)).toBe(true) + }) + + it('releases the torn-down environment latch but preserves a sibling environment mid-create', () => { + // A create for OTHER_ENV is in flight when ENV is torn down. Root cause of the cross-environment + // duplicate: the latch was keyed by worktree alone, so ENV's teardown released OTHER_ENV's key + // and a fresh OTHER_ENV subscription could seed a second terminal (same STA-6173 defect, other + // door). Per-environment keying keeps the sibling latch held. + expect(beginWebRuntimeInitialTerminalBootstrap(ENV, WT)).toBe(true) + expect(beginWebRuntimeInitialTerminalBootstrap(OTHER_ENV, WT)).toBe(true) + + clearWebSessionTabsTrackingForEnvironment(ENV) + + expect(isWebRuntimeInitialTerminalBootstrapInFlight(ENV, WT)).toBe(false) + expect(isWebRuntimeInitialTerminalBootstrapInFlight(OTHER_ENV, WT)).toBe(true) + + // A freshly installed OTHER_ENV closure passes its own flag false, so only the surviving latch + // stops it — proving the sibling create cannot be duplicated by ENV's teardown. + const freshEmpty = makeSnapshot([], { + activeGroupId: null, + activeTabId: null, + activeTabType: null + }) + expect( + shouldBootstrapInitialWebRuntimeTerminal({ + event: { type: 'snapshot', ...freshEmpty }, + activeWorktreeId: WT, + requestedInitialTerminal: isWebRuntimeInitialTerminalBootstrapInFlight(OTHER_ENV, WT), + snapshotIsFresh: true, + localTerminalCount: 0, + hasPersistedTerminalState: false + }) + ).toBe(false) + }) + + // Why: a mirror frame is the host's answer, but only for a create that already resolved. Releasing + // a still-pending create on a frame is the re-armed-closure race this latch exists to close. + it('releases an awaiting-mirror bootstrap on a mirror frame but never a pending create', () => { + expect(beginWebRuntimeInitialTerminalBootstrap(ENV, WT)).toBe(true) + releaseWebRuntimeInitialTerminalBootstrapOnMirrorFrame(ENV, WT) + expect(isWebRuntimeInitialTerminalBootstrapInFlight(ENV, WT)).toBe(true) + + markWebRuntimeInitialTerminalBootstrapAwaitingMirror(ENV, WT) + expect(isWebRuntimeInitialTerminalBootstrapInFlight(ENV, WT)).toBe(true) + releaseWebRuntimeInitialTerminalBootstrapOnMirrorFrame(ENV, WT) + expect(isWebRuntimeInitialTerminalBootstrapInFlight(ENV, WT)).toBe(false) + }) + + it('does not park a bootstrap that was never claimed', () => { + markWebRuntimeInitialTerminalBootstrapAwaitingMirror(ENV, WT) + expect(isWebRuntimeInitialTerminalBootstrapInFlight(ENV, WT)).toBe(false) + }) + + // Why: the second half of STA-6173. One focus re-runs the subscription effect (environment, + // connection generation and session-ready all settle during a workspace switch), and the old + // closure-local flag re-armed with it, so both closures seeded before either create mirrored. + it('declines a second bootstrap while one is already in flight for the worktree', () => { + const freshEmpty = makeSnapshot([], { + activeGroupId: null, + activeTabId: null, + activeTabType: null + }) + const decide = (): boolean => + shouldBootstrapInitialWebRuntimeTerminal({ + event: { type: 'snapshot', ...freshEmpty }, + activeWorktreeId: WT, + // What a freshly installed closure passes: its own flag is false, so only the shared latch + // can stop it. + requestedInitialTerminal: isWebRuntimeInitialTerminalBootstrapInFlight(ENV, WT), + snapshotIsFresh: true, + localTerminalCount: 0, + hasPersistedTerminalState: false + }) + + expect(decide()).toBe(true) + expect(beginWebRuntimeInitialTerminalBootstrap(ENV, WT)).toBe(true) + expect(beginWebRuntimeInitialTerminalBootstrap(ENV, WT)).toBe(false) + expect(decide()).toBe(false) + + endWebRuntimeInitialTerminalBootstrap(ENV, WT) + expect(decide()).toBe(true) + }) + it('does not respawn after wake when activation already requested a respawn', () => { const freshEmpty = makeSnapshot([], { activeGroupId: null, diff --git a/src/renderer/src/runtime/web-session-tabs-sync-test-harness.ts b/src/renderer/src/runtime/web-session-tabs-sync-test-harness.ts index 76477706f38f..78e0205117a3 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync-test-harness.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync-test-harness.ts @@ -4,6 +4,7 @@ import { resetWebSessionFocusIntentForTests } from './web-session-focus-intent' import { resetWebSessionCloseIntentForTests } from './web-session-close-intent' import { resetWebSessionReorderIntentForTests } from './web-session-reorder-intent' import { resetWebAgentSessionHandoffsForTests } from './web-agent-session-handoff' +import { resetWebRuntimeInitialTerminalBootstrapForTests } from './web-runtime-initial-terminal-bootstrap' import { resetWebSessionTabsSnapshotFreshnessForTests, type WebSessionTabsSyncState @@ -24,6 +25,7 @@ export function resetWebSessionTabsSyncTestState(): void { resetWebSessionCloseIntentForTests() resetWebSessionReorderIntentForTests() resetWebAgentSessionHandoffsForTests() + resetWebRuntimeInitialTerminalBootstrapForTests() } export function layoutHasGroup(layout: TabGroupLayoutNode | undefined, groupId: string): boolean { diff --git a/src/renderer/src/runtime/web-session-tabs-sync/active-session-subscription.ts b/src/renderer/src/runtime/web-session-tabs-sync/active-session-subscription.ts index 391953c4f285..5a61b0311e84 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync/active-session-subscription.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync/active-session-subscription.ts @@ -35,6 +35,11 @@ import { shouldSkipWebRuntimeWakeTerminalRespawn } from '../web-runtime-wake-terminal-respawn' import { createWebRuntimeSessionTerminal } from '../web-runtime-session' +import { + isWebRuntimeInitialTerminalBootstrapInFlight, + releaseWebRuntimeInitialTerminalBootstrapOnMirrorFrame +} from '../web-runtime-initial-terminal-bootstrap' +import { dispatchWebRuntimeInitialTerminalBootstrap } from '../web-runtime-initial-terminal-bootstrap-dispatch' import { toRuntimeWorktreeSelector } from '../runtime-worktree-selector' import type { SessionTabsStreamEvent } from './state' @@ -112,7 +117,6 @@ export function installActiveSessionTabsSubscription({ environmentId, { expectedEnvironmentPairingRevision: activeWorktreeRuntimePairingRevision, - expectedRuntimeId: runtimeId, getCurrentState: () => useAppStore.getState() } ) @@ -140,16 +144,21 @@ export function installActiveSessionTabsSubscription({ const hasLiveLocalPty = localTabs.some( (tab) => (syncState.ptyIdsByTabId[tab.id] ?? []).length > 0 ) - const skipAutomaticTerminal = shouldSkipWebRuntimeWakeTerminalRespawn(activeWorktreeId) - const bootstrap = - !skipAutomaticTerminal && - shouldBootstrapInitialWebRuntimeTerminal({ - event: recoveredEvent, - activeWorktreeId, - requestedInitialTerminal, - snapshotIsFresh: decision.apply, - localTerminalCount - }) + const bootstrap = shouldBootstrapInitialWebRuntimeTerminal({ + event: recoveredEvent, + activeWorktreeId, + // Why both: the closure flag records that this subscription already owned a create, so the + // same closure never seeds twice even once the shared latch is released; a create that threw + // never set it, so a later frame may retry. The shared latch is what survives the effect + // re-runs a workspace switch triggers — without it a second closure seeds a second terminal + // while the first create is still in flight (STA-6173). + requestedInitialTerminal: + requestedInitialTerminal || + isWebRuntimeInitialTerminalBootstrapInFlight(environmentId, activeWorktreeId), + snapshotIsFresh: decision.apply, + localTerminalCount, + hasPersistedTerminalState: Object.hasOwn(syncState.tabsByWorktree, activeWorktreeId) + }) const respawn = shouldRespawnWebRuntimeTerminalAfterWake({ event: recoveredEvent, activeWorktreeId, @@ -157,7 +166,7 @@ export function installActiveSessionTabsSubscription({ snapshotIsFresh: decision.apply, localTerminalCount, hasLiveLocalPty, - skipWakeRespawn: skipAutomaticTerminal + skipWakeRespawn: shouldSkipWebRuntimeWakeTerminalRespawn(activeWorktreeId) }) let settle: HostSessionMirrorSettle | null = decision.apply ? null @@ -186,15 +195,18 @@ export function installActiveSessionTabsSubscription({ event.type === 'updated' && !replayed ) visibilitySnapshotAccepted.current(environmentId, recovered, receivedFrame, runtimeId) + // Why here and only on an applied frame: this is the mirror's answer about the worktree. A + // bootstrap whose create resolved without a row was parked until that answer arrived; a + // pending create keeps its claim (see the latch module). The predicate above already ran with + // the latch held, so this frame never seeds — the next one decides on real state. + releaseWebRuntimeInitialTerminalBootstrapOnMirrorFrame(environmentId, recovered.worktree) } try { if (isCurrent() && bootstrap) { - requestedInitialTerminal = true - await createWebRuntimeSessionTerminal({ - worktreeId: activeWorktreeId, - environmentId, - activate: true - }) + // Latch claim, create, and row-conditional release live in the dispatch helper. + if (await dispatchWebRuntimeInitialTerminalBootstrap(environmentId, activeWorktreeId)) { + requestedInitialTerminal = true + } } else if (isCurrent() && respawn && beginWebRuntimeWakeTerminalRespawn(activeWorktreeId)) { requestedRespawnAfterWake = true await createWebRuntimeSessionTerminal({ diff --git a/src/renderer/src/runtime/web-session-tabs-sync/apply-preparation-base.ts b/src/renderer/src/runtime/web-session-tabs-sync/apply-preparation-base.ts index 0b64a584dd6b..0ff3f0ecb662 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync/apply-preparation-base.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync/apply-preparation-base.ts @@ -29,6 +29,7 @@ import { shouldReplaceTerminalTab } from './terminal-surfaces' import { buildMirroredTerminalTabs } from './terminal-build' +import { hostSnapshotAffirmsWorktreeContents } from '../host-session-snapshot-authority' export function prepareWebSessionTabsSnapshotBase( state: WebSessionTabsSyncState, @@ -182,10 +183,19 @@ export function prepareWebSessionTabsSnapshotBase( ) const mirroredTerminalTabEntries = mirroredTerminalTabs.map((entry) => entry.tab) const retainedTerminalIds = new Set(retainedTerminalTabs.map((tab) => tab.id)) + // Why an empty row rather than a deleted key: an explicit empty row is the closed-last-terminal + // tombstone every seeder reads (initial-terminal.ts), and dropping the key reads back as "never + // initialized", which re-seeds the workspace on every focus (STA-6173). `sameTerminalTabs` treats + // a missing row and an empty one as equal, so a worktree that never had a terminal still gets no + // row. A removal frame really is gone, and a synthesized unpublished frame is "ask me later", not + // evidence the user emptied anything — neither may leave a tombstone behind. const nextTerminalTabs = retainedTerminalTabs.length + mirroredTerminalTabEntries.length > 0 ? [...retainedTerminalTabs, ...mirroredTerminalTabEntries] - : null + : isWebSessionTabsWorktreeRemovalFrame(snapshot) || + !hostSnapshotAffirmsWorktreeContents(snapshot) + ? null + : [] const mirroredTerminalIds = new Set(mirroredTerminalTabEntries.map((tab) => tab.id)) const removedTerminalIds = new Set( currentTerminalTabs.filter((tab) => !retainedTerminalIds.has(tab.id)).map((tab) => tab.id) diff --git a/src/renderer/src/runtime/web-session-tabs-sync/tracking-decisions.ts b/src/renderer/src/runtime/web-session-tabs-sync/tracking-decisions.ts index a5a4c851b5dd..9bd90ba78b13 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync/tracking-decisions.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync/tracking-decisions.ts @@ -18,9 +18,10 @@ import { trackWebSessionTabsWorktree, recordAcceptedWebSessionTabsEnvironment } from './tracking' -import { hostSnapshotAffirmsWorktreeContents } from '../host-session-snapshot-authority' import { clearWebSessionTabsTrackingForWorktree } from './tracking-lifecycle' import { queueAcceptedWebSessionTerminalSnapshot } from '../web-session-terminal-handle-events' +import { shouldAutoCreateInitialTerminal } from '@/components/terminal/initial-terminal' +import { hostSnapshotAffirmsWorktreeContents } from '../host-session-snapshot-authority' /** A frame's fate, paired with whether that fate is host evidence for the worktree. */ export type WebSessionTabsSnapshotDecision = { @@ -137,12 +138,21 @@ export function shouldBootstrapInitialWebRuntimeTerminal(args: { requestedInitialTerminal: boolean snapshotIsFresh: boolean localTerminalCount: number + hasPersistedTerminalState: boolean }): boolean { return ( args.snapshotIsFresh && args.event.type === 'snapshot' && + // Why: a synthesized unpublished frame (`UNPUBLISHED_WORKTREE_PUBLICATION_EPOCH` at version 0) + // is the runtime saying "ask me later", not a host with zero terminals. Seeding on it can + // duplicate a pane the host is about to republish after a restart — the same "ask me later" + // frame the tombstone write already refuses to treat as the user emptying the workspace. + hostSnapshotAffirmsWorktreeContents(args.event) && args.event.tabs.length === 0 && - args.localTerminalCount === 0 && + // Why the shared predicate: the host owning the terminals does not change what an empty + // workspace means. A missing row is "never initialized", an explicit empty row is "the user + // closed the last terminal", and only the local seeder used to read the difference (STA-6173). + shouldAutoCreateInitialTerminal(args.localTerminalCount, args.hasPersistedTerminalState) && !args.requestedInitialTerminal && args.activeWorktreeId === args.event.worktree ) diff --git a/src/renderer/src/runtime/web-session-tabs-sync/tracking-lifecycle.ts b/src/renderer/src/runtime/web-session-tabs-sync/tracking-lifecycle.ts index 81ed68aea296..0c32f9539867 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync/tracking-lifecycle.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync/tracking-lifecycle.ts @@ -21,6 +21,10 @@ import { clearWebRuntimeWakeTerminalRespawnForWorktree, clearAllWebRuntimeWakeTerminalRespawn } from '../web-runtime-wake-terminal-respawn' +import { + endWebRuntimeInitialTerminalBootstrap, + clearWebRuntimeInitialTerminalBootstrapsForEnvironment +} from '../web-runtime-initial-terminal-bootstrap' import { clearWebSessionReorderIntentsForWorktree } from '../web-session-reorder-intent' import { clearWebSessionCloseIntentsForWorktree } from '../web-session-close-intent' import { @@ -138,6 +142,7 @@ export function clearWebSessionTabsTrackingForWorktree( lastHostTerminalTabCountByWorktree.delete(key) sessionTabsInventoryOmissionsByWorktree.delete(key) clearWebRuntimeWakeTerminalRespawnForWorktree(worktreeId) + endWebRuntimeInitialTerminalBootstrap(environmentId, worktreeId) clearWebSessionReorderIntentsForWorktree({ environmentId }, worktreeId) clearWebSessionCloseIntentsForWorktree({ environmentId }, worktreeId) clearWebAgentSessionHandoffsForWorktree(environmentId, worktreeId) @@ -214,6 +219,7 @@ export function clearWebSessionTabsTrackingForEnvironment(environmentId: string) clearHostSessionMirrorHydration(trimmedEnvironmentId) clearHostMirrorHandleGapVerdictsForEnvironment(trimmedEnvironmentId) clearAllWebRuntimeWakeTerminalRespawn() + clearWebRuntimeInitialTerminalBootstrapsForEnvironment(trimmedEnvironmentId) } export function getWebSessionTabsTrackingGeneration(environmentId: string): number { diff --git a/src/renderer/src/store/terminals/workspace-terminal-hydration-patch.ts b/src/renderer/src/store/terminals/workspace-terminal-hydration-patch.ts index 52150e926250..e2713203a41d 100644 --- a/src/renderer/src/store/terminals/workspace-terminal-hydration-patch.ts +++ b/src/renderer/src/store/terminals/workspace-terminal-hydration-patch.ts @@ -184,11 +184,15 @@ export function targetScopedWorkspaceHydrationPatch( hydrated.lastVisitedAtByWorktreeId, workspaceKeys ), - defaultTerminalTabsAppliedByWorktreeId: replaceHydratedRecordKeys( - state.defaultTerminalTabsAppliedByWorktreeId, - hydrated.defaultTerminalTabsAppliedByWorktreeId, - workspaceKeys - ), + // Why local marks win: same write-once rule as mergeDirectSshRemoteWorkspaceSession. + defaultTerminalTabsAppliedByWorktreeId: { + ...replaceHydratedRecordKeys( + state.defaultTerminalTabsAppliedByWorktreeId, + hydrated.defaultTerminalTabsAppliedByWorktreeId, + workspaceKeys + ), + ...state.defaultTerminalTabsAppliedByWorktreeId + }, // Why passed through whole: hydration already unioned it with live store state, and the map is // keyed by tab id rather than by workspace key so replaceHydratedRecordKeys has nothing to match. closedTerminalTabTombstonesByTabId: hydrated.closedTerminalTabTombstonesByTabId, diff --git a/src/renderer/src/store/terminals/workspace-terminal-hydration.ts b/src/renderer/src/store/terminals/workspace-terminal-hydration.ts index 9abafd4a7f6e..db278e05d925 100644 --- a/src/renderer/src/store/terminals/workspace-terminal-hydration.ts +++ b/src/renderer/src/store/terminals/workspace-terminal-hydration.ts @@ -185,8 +185,12 @@ export function createWorkspaceTerminalHydrationActions( worktreesByRepo, // Why: restore the focus-recency map; pruning is deferred to App.tsx (post-hydration) because SSH worktrees may still be appearing in worktreesByRepo. lastVisitedAtByWorktreeId: session.lastVisitedAtByWorktreeId ?? {}, - defaultTerminalTabsAppliedByWorktreeId: - session.defaultTerminalTabsAppliedByWorktreeId ?? {}, + // Why union: a persist snapshot that omits the write-once marker is a writer that never + // stamped it, not a report that default tabs were never applied (#18117). + defaultTerminalTabsAppliedByWorktreeId: { + ...session.defaultTerminalTabsAppliedByWorktreeId, + ...s.defaultTerminalTabsAppliedByWorktreeId + }, // Why replace and not union: both callers hand over a map they derived from this store // synchronously (the pull merge) or from disk before the store had one (startup), so there // is no local tombstone to lose — and a union would resurrect the ones the merge just