From ff8a1625c6288485c4e144c9248be45186825e23 Mon Sep 17 00:00:00 2001 From: Wooseong Kim Date: Thu, 3 Sep 2026 02:29:47 +0900 Subject: [PATCH 01/17] fix(session): persist defaultTerminalTabsAppliedByWorktreeId Host persist snapshots wrote tabs but omitted this write-once map, and full session replaces / hydration treated omission as "never applied". Union the marker across persist and hydrate so default terminals are not re-spawned on every launch or re-attach. Fixes #18117 --- ...-host-admitted-terminal-membership.test.ts | 23 ++++ .../loading-store/pty-binding-persistence.ts | 5 + ...-authored-workspace-session-fields.test.ts | 21 ++++ ...ntime-authored-workspace-session-fields.ts | 50 +++++--- ...ault-terminal-tabs-applied-persist.test.ts | 116 ++++++++++++++++++ .../src/lib/session-write-subscriber.test.ts | 19 +++ .../workspace-terminal-hydration-patch.ts | 14 ++- .../terminals/workspace-terminal-hydration.ts | 8 +- 8 files changed, 233 insertions(+), 23 deletions(-) create mode 100644 src/renderer/src/lib/default-terminal-tabs-applied-persist.test.ts 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/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..664a9279b866 --- /dev/null +++ b/src/renderer/src/lib/default-terminal-tabs-applied-persist.test.ts @@ -0,0 +1,116 @@ +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 { preserveRuntimeAuthoredWorkspaceSessionFields } from '../../../main/persistence/runtime-authored-workspace-session-fields' +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' }] +} + +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/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 From 1621f83708e510b1dfd8b0ecbad0e5d796d9f519 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Thu, 3 Sep 2026 13:18:56 -0700 Subject: [PATCH 02/17] fix(runtime): stop re-seeding a runtime-owned workspace the user emptied Focusing a workspace owned by a remote runtime created a terminal every time, and sometimes two. The mirror could never record the closed-last-terminal state. A host snapshot with no terminals produced `nextTerminalTabs === null`, which `withWorktreeEntry` turns into a deleted key -- and a missing row is exactly how every seeder spells "never initialized" (initial-terminal.ts). Keep an explicit empty row instead, so the remote path reads the same tombstone the local one already honours. A worktree that never had a terminal still gets no row, because `sameTerminalTabs` treats a missing row and an empty one as equal; removal frames and synthesized unpublished frames keep deleting, since neither is evidence the user emptied anything. The duplicate had a second cause. `requestedInitialTerminal` was a `let` inside the session-tabs subscription closure, so "one focus creates at most one terminal" held only for as long as that 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. That is the asymmetry in the report: one terminal when arriving from the landing screen, two when arriving from another workspace. Latch the bootstrap per worktree in a module-scoped set instead, modelled on web-runtime-wake-terminal-respawn.ts, released when the create settles. The closure flag stays alongside it so a failed create still does not retry on every later frame of the same subscription. Fixes STA-6173. --- .../web-runtime-initial-terminal-bootstrap.ts | 38 +++++ ...s-sync-emptied-workspace-tombstone.test.ts | 138 ++++++++++++++++++ ...ssion-tabs-sync-terminal-bootstrap.test.ts | 82 ++++++++++- .../web-session-tabs-sync-test-harness.ts | 2 + .../active-session-subscription.ts | 42 ++++-- .../apply-preparation-base.ts | 13 +- .../tracking-decisions.ts | 7 +- 7 files changed, 304 insertions(+), 18 deletions(-) create mode 100644 src/renderer/src/runtime/web-runtime-initial-terminal-bootstrap.ts create mode 100644 src/renderer/src/runtime/web-session-tabs-sync-emptied-workspace-tombstone.test.ts 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..7edc23803c53 --- /dev/null +++ b/src/renderer/src/runtime/web-runtime-initial-terminal-bootstrap.ts @@ -0,0 +1,38 @@ +/** + * 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 released when the create settles: by then the + * create has awaited its own snapshot refresh, so the host's tab is mirrored and the predicate + * declines on its own, while a failed create is free to be retried by a later focus. + */ +const initialTerminalBootstrapInFlightByWorktree = new Set() + +export function isWebRuntimeInitialTerminalBootstrapInFlight(worktreeId: string): boolean { + return initialTerminalBootstrapInFlightByWorktree.has(worktreeId) +} + +/** Claims the bootstrap for this worktree; false when another closure already holds it. */ +export function beginWebRuntimeInitialTerminalBootstrap(worktreeId: string): boolean { + if (initialTerminalBootstrapInFlightByWorktree.has(worktreeId)) { + return false + } + initialTerminalBootstrapInFlightByWorktree.add(worktreeId) + return true +} + +export function endWebRuntimeInitialTerminalBootstrap(worktreeId: string): void { + initialTerminalBootstrapInFlightByWorktree.delete(worktreeId) +} + +export function clearAllWebRuntimeInitialTerminalBootstraps(): void { + initialTerminalBootstrapInFlightByWorktree.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-terminal-bootstrap.test.ts b/src/renderer/src/runtime/web-session-tabs-sync-terminal-bootstrap.test.ts index b34a86ebd3e7..12fb02930fbe 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 @@ -14,6 +14,11 @@ import { makeSnapshot, resetWebSessionTabsSyncTestState } from './web-session-tabs-sync-test-harness' +import { + beginWebRuntimeInitialTerminalBootstrap, + endWebRuntimeInitialTerminalBootstrap, + isWebRuntimeInitialTerminalBootstrapInFlight +} from './web-runtime-initial-terminal-bootstrap' vi.mock('../store', () => ({ useAppStore: { @@ -55,7 +60,8 @@ describe('applyWebSessionTabsSnapshot', () => { activeWorktreeId: WT, requestedInitialTerminal: false, snapshotIsFresh: staleIsFresh, - localTerminalCount: 0 + localTerminalCount: 0, + hasPersistedTerminalState: false }) ).toBe(false) }) @@ -73,11 +79,83 @@ 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: 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(WT), + snapshotIsFresh: true, + localTerminalCount: 0, + hasPersistedTerminalState: false + }) + + expect(decide()).toBe(true) + expect(beginWebRuntimeInitialTerminalBootstrap(WT)).toBe(true) + expect(beginWebRuntimeInitialTerminalBootstrap(WT)).toBe(false) + expect(decide()).toBe(false) + + endWebRuntimeInitialTerminalBootstrap(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..eb9d411d82ab 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 @@ -6,6 +6,7 @@ import { getRuntimeEnvironmentRevision } from '../runtime-environment-revision' import { recoverWebSessionTerminalOrphansBeforeApply } from '../web-session-terminal-orphan-recovery' import { installWindowVisibilitySubscriptionParking } from '../window-visibility-subscription-parking' import { + beginWebSessionTabsSnapshotRecovery, recordReceivedWebSessionTabsSnapshot, shouldApplyRecoveredWebSessionTabsSnapshot } from './tracking' @@ -35,6 +36,11 @@ import { shouldSkipWebRuntimeWakeTerminalRespawn } from '../web-runtime-wake-terminal-respawn' import { createWebRuntimeSessionTerminal } from '../web-runtime-session' +import { + beginWebRuntimeInitialTerminalBootstrap, + endWebRuntimeInitialTerminalBootstrap, + isWebRuntimeInitialTerminalBootstrapInFlight +} from '../web-runtime-initial-terminal-bootstrap' import { toRuntimeWorktreeSelector } from '../runtime-worktree-selector' import type { SessionTabsStreamEvent } from './state' @@ -112,7 +118,6 @@ export function installActiveSessionTabsSubscription({ environmentId, { expectedEnvironmentPairingRevision: activeWorktreeRuntimePairingRevision, - expectedRuntimeId: runtimeId, getCurrentState: () => useAppStore.getState() } ) @@ -140,16 +145,19 @@ 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 keeps one failed attempt from retrying on every later frame of + // the same subscription, and 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(activeWorktreeId), + snapshotIsFresh: decision.apply, + localTerminalCount, + hasPersistedTerminalState: Object.hasOwn(syncState.tabsByWorktree, activeWorktreeId) + }) const respawn = shouldRespawnWebRuntimeTerminalAfterWake({ event: recoveredEvent, activeWorktreeId, @@ -157,7 +165,7 @@ export function installActiveSessionTabsSubscription({ snapshotIsFresh: decision.apply, localTerminalCount, hasLiveLocalPty, - skipWakeRespawn: skipAutomaticTerminal + skipWakeRespawn: shouldSkipWebRuntimeWakeTerminalRespawn(activeWorktreeId) }) let settle: HostSessionMirrorSettle | null = decision.apply ? null @@ -188,13 +196,13 @@ export function installActiveSessionTabsSubscription({ visibilitySnapshotAccepted.current(environmentId, recovered, receivedFrame, runtimeId) } try { - if (isCurrent() && bootstrap) { + if (isCurrent() && bootstrap && beginWebRuntimeInitialTerminalBootstrap(activeWorktreeId)) { requestedInitialTerminal = true await createWebRuntimeSessionTerminal({ worktreeId: activeWorktreeId, environmentId, activate: true - }) + }).finally(() => endWebRuntimeInitialTerminalBootstrap(activeWorktreeId)) } else if (isCurrent() && respawn && beginWebRuntimeWakeTerminalRespawn(activeWorktreeId)) { requestedRespawnAfterWake = true await createWebRuntimeSessionTerminal({ @@ -251,6 +259,11 @@ export function installActiveSessionTabsSubscription({ runtimeId ) visibilitySnapshotReceipt.current(environmentId, event, frame, runtimeId) + const finish = beginWebSessionTabsSnapshotRecovery( + environmentId, + event.worktree, + frame + ) void applyActiveSnapshot(event, response, isCurrent, frame, runtimeId) .catch((error) => { if (isCurrent()) { @@ -259,6 +272,7 @@ export function installActiveSessionTabsSubscription({ return null }) .then((settle) => { + finish() if (isCurrent()) { settle?.() } 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 ef697da69994..6085a4ecc554 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 @@ -27,6 +27,8 @@ import { shouldReplaceTerminalTab } from './terminal-surfaces' import { buildMirroredTerminalTabs } from './terminal-build' +import { isWebSessionTabsWorktreeRemovalFrame } from './session-tabs-inventory-absence' +import { hostSnapshotAffirmsWorktreeContents } from '../host-session-snapshot-authority' export function prepareWebSessionTabsSnapshotBase( state: WebSessionTabsSyncState, @@ -180,10 +182,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..bddbf8ed874a 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 @@ -21,6 +21,7 @@ import { 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' /** A frame's fate, paired with whether that fate is host evidence for the worktree. */ export type WebSessionTabsSnapshotDecision = { @@ -137,12 +138,16 @@ export function shouldBootstrapInitialWebRuntimeTerminal(args: { requestedInitialTerminal: boolean snapshotIsFresh: boolean localTerminalCount: number + hasPersistedTerminalState: boolean }): boolean { return ( args.snapshotIsFresh && args.event.type === 'snapshot' && 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 ) From 0c7ce39f60b9967fd7fc8d337046dac1732084ca Mon Sep 17 00:00:00 2001 From: Neil Date: Tue, 8 Sep 2026 00:10:26 -0700 Subject: [PATCH 03/17] fix(runtime): harden the runtime-owned initial-terminal bootstrap latch Follow-up on the STA-6173 fix, addressing restore-time safety gaps found in review. - Decline the bootstrap on a synthesized unpublished frame (`UNPUBLISHED_WORKTREE_PUBLICATION_EPOCH` at version 0). That frame 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. This is the same "ask me later" frame the tombstone write already refuses to treat as the user emptying the workspace. - Release the module-scoped bootstrap latch on worktree tracking teardown and environment teardown, mirroring web-runtime-wake-terminal-respawn. A create RPC that never settles during a disconnect would otherwise leave the per-worktree key set and suppress the next bootstrap after reconnect. Tests: - New per-worktree and per-environment latch-release cases. - New "unpublished frame declines" bootstrap case. - New hook-level regression pinning the second defect end to end: a forced active-subscription re-run while the first create is in flight seeds exactly one terminal (two on the pre-fix tree). --- ...bs-sync-initial-terminal-relaunch.test.tsx | 206 ++++++++++++++++++ ...ssion-tabs-sync-terminal-bootstrap.test.ts | 52 +++++ .../tracking-decisions.ts | 6 + .../tracking-lifecycle.ts | 6 + 4 files changed, 270 insertions(+) create mode 100644 src/renderer/src/runtime/web-session-tabs-sync-initial-terminal-relaunch.test.tsx 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..7290247f4bb5 --- /dev/null +++ b/src/renderer/src/runtime/web-session-tabs-sync-initial-terminal-relaunch.test.tsx @@ -0,0 +1,206 @@ +// @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() +})) + +vi.mock('./use-runtime-session-mirror-environment-key', () => ({ + useRuntimeSessionMirrorEnvironmentKey: mocks.runtimeSessionMirrorEnvironmentKey +})) + +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() + }) +}) 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 12fb02930fbe..0f0f6d3de03a 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, @@ -19,6 +20,10 @@ import { endWebRuntimeInitialTerminalBootstrap, isWebRuntimeInitialTerminalBootstrapInFlight } from './web-runtime-initial-terminal-bootstrap' +import { + clearWebSessionTabsTrackingForWorktree, + clearWebSessionTabsTrackingForEnvironment +} from './web-session-tabs-sync/tracking-lifecycle' vi.mock('../store', () => ({ useAppStore: { @@ -126,6 +131,53 @@ describe('applyWebSessionTabsSnapshot', () => { ).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(WT)).toBe(true) + expect(isWebRuntimeInitialTerminalBootstrapInFlight(WT)).toBe(true) + + clearWebSessionTabsTrackingForWorktree(ENV, WT) + + expect(isWebRuntimeInitialTerminalBootstrapInFlight(WT)).toBe(false) + expect(beginWebRuntimeInitialTerminalBootstrap(WT)).toBe(true) + }) + + it('releases every in-flight bootstrap latch when the environment is torn down', () => { + expect(beginWebRuntimeInitialTerminalBootstrap(WT)).toBe(true) + expect(isWebRuntimeInitialTerminalBootstrapInFlight(WT)).toBe(true) + + clearWebSessionTabsTrackingForEnvironment(ENV) + + expect(isWebRuntimeInitialTerminalBootstrapInFlight(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. 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 bddbf8ed874a..216574fb2e24 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 @@ -22,6 +22,7 @@ import { hostSnapshotAffirmsWorktreeContents } from '../host-session-snapshot-au 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 = { @@ -143,6 +144,11 @@ export function shouldBootstrapInitialWebRuntimeTerminal(args: { 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 && // 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 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..a976100c4158 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, + clearAllWebRuntimeInitialTerminalBootstraps +} 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(worktreeId) clearWebSessionReorderIntentsForWorktree({ environmentId }, worktreeId) clearWebSessionCloseIntentsForWorktree({ environmentId }, worktreeId) clearWebAgentSessionHandoffsForWorktree(environmentId, worktreeId) @@ -214,6 +219,7 @@ export function clearWebSessionTabsTrackingForEnvironment(environmentId: string) clearHostSessionMirrorHydration(trimmedEnvironmentId) clearHostMirrorHandleGapVerdictsForEnvironment(trimmedEnvironmentId) clearAllWebRuntimeWakeTerminalRespawn() + clearAllWebRuntimeInitialTerminalBootstraps() } export function getWebSessionTabsTrackingGeneration(environmentId: string): number { From 957064f1a63e40984e0720f39f41e10344d1805f Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 8 Sep 2026 00:48:32 -0700 Subject: [PATCH 04/17] fix(runtime): key the initial-terminal bootstrap latch per environment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review on the STA-6173 hardening. - Key the bootstrap latch by (environment, 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. The latch was cleared wholesale on any environment teardown, so tearing down environment A released environment B's in-flight key and a fresh B subscription could seed a duplicate — the STA-6173 defect through another door. Environment teardown now clears only its own keys; worktree teardown clears only that (environment, worktree). - Hold the latch after a successful create until a mirrored `tabsByWorktree` row exists. The snapshot refresh the create awaits can resolve on an empty, unconfirmed frame that leaves no row; releasing then let a later effect re-run seed a second terminal. A failed create still releases for retry. The latch claim/create/release now lives in web-runtime-initial-terminal-bootstrap-dispatch.ts, keeping active-session-subscription within its line budget. Tests (each mutation-tested against its own regression): - Cross-environment: a create for env B in flight, env A torn down, a fresh B closure must still decline. Fails when env teardown sweeps all environments. - Hold-until-row: a create that resolved without mirroring a row must not let an effect re-run seed again. Fails on unconditional release. --- ...ime-initial-terminal-bootstrap-dispatch.ts | 39 +++++++++++++ .../web-runtime-initial-terminal-bootstrap.ts | 57 ++++++++++++++----- ...bs-sync-initial-terminal-relaunch.test.tsx | 26 +++++++++ ...ssion-tabs-sync-terminal-bootstrap.test.ts | 49 ++++++++++++---- .../active-session-subscription.ts | 22 +++---- .../tracking-lifecycle.ts | 6 +- 6 files changed, 158 insertions(+), 41 deletions(-) create mode 100644 src/renderer/src/runtime/web-runtime-initial-terminal-bootstrap-dispatch.ts 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..b44e5b763d72 --- /dev/null +++ b/src/renderer/src/runtime/web-runtime-initial-terminal-bootstrap-dispatch.ts @@ -0,0 +1,39 @@ +import { useAppStore } from '../store' +import { createWebRuntimeSessionTerminal } from './web-runtime-session' +import { + beginWebRuntimeInitialTerminalBootstrap, + endWebRuntimeInitialTerminalBootstrap +} from './web-runtime-initial-terminal-bootstrap' + +/** + * Claim the initial-terminal latch for this environment's worktree, create the terminal, and + * release the latch only once a mirrored row exists (or on failure, for retry). + * + * Why row-conditional and not a plain `.finally`: the snapshot refresh the create awaits can resolve + * on an empty, unconfirmed frame that leaves no `tabsByWorktree` row. Releasing then would let a + * later effect re-run seed a second terminal even though the first create succeeded. Holding the + * latch until a row exists makes the predicate decline on its own; worktree or environment teardown + * clears it either way. A failed create leaves no terminal, so it releases for a later focus to + * retry. + * + * Returns true when this call owned the create, so the caller can keep its closure-local flag in + * step; false when another closure already held the latch. + */ +export async function dispatchWebRuntimeInitialTerminalBootstrap( + environmentId: string, + worktreeId: string +): Promise { + if (!beginWebRuntimeInitialTerminalBootstrap(environmentId, worktreeId)) { + return false + } + try { + await createWebRuntimeSessionTerminal({ worktreeId, environmentId, activate: true }) + } catch (error) { + endWebRuntimeInitialTerminalBootstrap(environmentId, worktreeId) + throw error + } + if (Object.hasOwn(useAppStore.getState().tabsByWorktree, worktreeId)) { + endWebRuntimeInitialTerminalBootstrap(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 index 7edc23803c53..a32ad190452e 100644 --- a/src/renderer/src/runtime/web-runtime-initial-terminal-bootstrap.ts +++ b/src/renderer/src/runtime/web-runtime-initial-terminal-bootstrap.ts @@ -6,31 +6,62 @@ * 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 released when the create settles: by then the - * create has awaited its own snapshot refresh, so the host's tab is mirrored and the predicate - * declines on its own, while a failed create is free to be retried by a later focus. + * (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. */ -const initialTerminalBootstrapInFlightByWorktree = new Set() +const inFlightWorktreesByEnvironment = new Map>() -export function isWebRuntimeInitialTerminalBootstrapInFlight(worktreeId: string): boolean { - return initialTerminalBootstrapInFlightByWorktree.has(worktreeId) +export function isWebRuntimeInitialTerminalBootstrapInFlight( + environmentId: string, + worktreeId: string +): boolean { + return inFlightWorktreesByEnvironment.get(environmentId)?.has(worktreeId) ?? false } -/** Claims the bootstrap for this worktree; false when another closure already holds it. */ -export function beginWebRuntimeInitialTerminalBootstrap(worktreeId: string): boolean { - if (initialTerminalBootstrapInFlightByWorktree.has(worktreeId)) { +/** Claims the bootstrap for this environment's worktree; false when another closure already holds it. */ +export function beginWebRuntimeInitialTerminalBootstrap( + environmentId: string, + worktreeId: string +): boolean { + const worktrees = inFlightWorktreesByEnvironment.get(environmentId) + if (worktrees?.has(worktreeId)) { return false } - initialTerminalBootstrapInFlightByWorktree.add(worktreeId) + if (worktrees) { + worktrees.add(worktreeId) + } else { + inFlightWorktreesByEnvironment.set(environmentId, new Set([worktreeId])) + } return true } -export function endWebRuntimeInitialTerminalBootstrap(worktreeId: string): void { - initialTerminalBootstrapInFlightByWorktree.delete(worktreeId) +export function endWebRuntimeInitialTerminalBootstrap( + environmentId: string, + worktreeId: string +): void { + const worktrees = inFlightWorktreesByEnvironment.get(environmentId) + if (!worktrees) { + return + } + worktrees.delete(worktreeId) + if (worktrees.size === 0) { + inFlightWorktreesByEnvironment.delete(environmentId) + } +} + +export function clearWebRuntimeInitialTerminalBootstrapsForEnvironment( + environmentId: string +): void { + inFlightWorktreesByEnvironment.delete(environmentId) } export function clearAllWebRuntimeInitialTerminalBootstraps(): void { - initialTerminalBootstrapInFlightByWorktree.clear() + inFlightWorktreesByEnvironment.clear() } export function resetWebRuntimeInitialTerminalBootstrapForTests(): void { 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 index 7290247f4bb5..3ea3805c9646 100644 --- 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 @@ -203,4 +203,30 @@ describe('useWebSessionTabsSync initial-terminal bootstrap across an effect re-r 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 resolves but writes no tabsByWorktree row (host has not published the tab yet). + mocks.createTerminal.mockResolvedValue(undefined) + + 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() + }) }) 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 0f0f6d3de03a..1e00dcb975ff 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 @@ -20,6 +20,8 @@ import { endWebRuntimeInitialTerminalBootstrap, isWebRuntimeInitialTerminalBootstrapInFlight } from './web-runtime-initial-terminal-bootstrap' + +const OTHER_ENV = 'web-env-2' import { clearWebSessionTabsTrackingForWorktree, clearWebSessionTabsTrackingForEnvironment @@ -160,22 +162,45 @@ describe('applyWebSessionTabsSnapshot', () => { // 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(WT)).toBe(true) - expect(isWebRuntimeInitialTerminalBootstrapInFlight(WT)).toBe(true) + expect(beginWebRuntimeInitialTerminalBootstrap(ENV, WT)).toBe(true) + expect(isWebRuntimeInitialTerminalBootstrapInFlight(ENV, WT)).toBe(true) clearWebSessionTabsTrackingForWorktree(ENV, WT) - expect(isWebRuntimeInitialTerminalBootstrapInFlight(WT)).toBe(false) - expect(beginWebRuntimeInitialTerminalBootstrap(WT)).toBe(true) + expect(isWebRuntimeInitialTerminalBootstrapInFlight(ENV, WT)).toBe(false) + expect(beginWebRuntimeInitialTerminalBootstrap(ENV, WT)).toBe(true) }) - it('releases every in-flight bootstrap latch when the environment is torn down', () => { - expect(beginWebRuntimeInitialTerminalBootstrap(WT)).toBe(true) - expect(isWebRuntimeInitialTerminalBootstrapInFlight(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(WT)).toBe(false) + 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: the second half of STA-6173. One focus re-runs the subscription effect (environment, @@ -193,18 +218,18 @@ describe('applyWebSessionTabsSnapshot', () => { activeWorktreeId: WT, // What a freshly installed closure passes: its own flag is false, so only the shared latch // can stop it. - requestedInitialTerminal: isWebRuntimeInitialTerminalBootstrapInFlight(WT), + requestedInitialTerminal: isWebRuntimeInitialTerminalBootstrapInFlight(ENV, WT), snapshotIsFresh: true, localTerminalCount: 0, hasPersistedTerminalState: false }) expect(decide()).toBe(true) - expect(beginWebRuntimeInitialTerminalBootstrap(WT)).toBe(true) - expect(beginWebRuntimeInitialTerminalBootstrap(WT)).toBe(false) + expect(beginWebRuntimeInitialTerminalBootstrap(ENV, WT)).toBe(true) + expect(beginWebRuntimeInitialTerminalBootstrap(ENV, WT)).toBe(false) expect(decide()).toBe(false) - endWebRuntimeInitialTerminalBootstrap(WT) + endWebRuntimeInitialTerminalBootstrap(ENV, WT) expect(decide()).toBe(true) }) 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 eb9d411d82ab..c9d44fbc0bf0 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 @@ -36,11 +36,8 @@ import { shouldSkipWebRuntimeWakeTerminalRespawn } from '../web-runtime-wake-terminal-respawn' import { createWebRuntimeSessionTerminal } from '../web-runtime-session' -import { - beginWebRuntimeInitialTerminalBootstrap, - endWebRuntimeInitialTerminalBootstrap, - isWebRuntimeInitialTerminalBootstrapInFlight -} from '../web-runtime-initial-terminal-bootstrap' +import { isWebRuntimeInitialTerminalBootstrapInFlight } 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' @@ -153,7 +150,8 @@ export function installActiveSessionTabsSubscription({ // switch triggers — without it a second closure seeds a second terminal while the first // create is still in flight (STA-6173). requestedInitialTerminal: - requestedInitialTerminal || isWebRuntimeInitialTerminalBootstrapInFlight(activeWorktreeId), + requestedInitialTerminal || + isWebRuntimeInitialTerminalBootstrapInFlight(environmentId, activeWorktreeId), snapshotIsFresh: decision.apply, localTerminalCount, hasPersistedTerminalState: Object.hasOwn(syncState.tabsByWorktree, activeWorktreeId) @@ -196,13 +194,11 @@ export function installActiveSessionTabsSubscription({ visibilitySnapshotAccepted.current(environmentId, recovered, receivedFrame, runtimeId) } try { - if (isCurrent() && bootstrap && beginWebRuntimeInitialTerminalBootstrap(activeWorktreeId)) { - requestedInitialTerminal = true - await createWebRuntimeSessionTerminal({ - worktreeId: activeWorktreeId, - environmentId, - activate: true - }).finally(() => endWebRuntimeInitialTerminalBootstrap(activeWorktreeId)) + if (isCurrent() && bootstrap) { + // 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/tracking-lifecycle.ts b/src/renderer/src/runtime/web-session-tabs-sync/tracking-lifecycle.ts index a976100c4158..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 @@ -23,7 +23,7 @@ import { } from '../web-runtime-wake-terminal-respawn' import { endWebRuntimeInitialTerminalBootstrap, - clearAllWebRuntimeInitialTerminalBootstraps + clearWebRuntimeInitialTerminalBootstrapsForEnvironment } from '../web-runtime-initial-terminal-bootstrap' import { clearWebSessionReorderIntentsForWorktree } from '../web-session-reorder-intent' import { clearWebSessionCloseIntentsForWorktree } from '../web-session-close-intent' @@ -142,7 +142,7 @@ export function clearWebSessionTabsTrackingForWorktree( lastHostTerminalTabCountByWorktree.delete(key) sessionTabsInventoryOmissionsByWorktree.delete(key) clearWebRuntimeWakeTerminalRespawnForWorktree(worktreeId) - endWebRuntimeInitialTerminalBootstrap(worktreeId) + endWebRuntimeInitialTerminalBootstrap(environmentId, worktreeId) clearWebSessionReorderIntentsForWorktree({ environmentId }, worktreeId) clearWebSessionCloseIntentsForWorktree({ environmentId }, worktreeId) clearWebAgentSessionHandoffsForWorktree(environmentId, worktreeId) @@ -219,7 +219,7 @@ export function clearWebSessionTabsTrackingForEnvironment(environmentId: string) clearHostSessionMirrorHydration(trimmedEnvironmentId) clearHostMirrorHandleGapVerdictsForEnvironment(trimmedEnvironmentId) clearAllWebRuntimeWakeTerminalRespawn() - clearAllWebRuntimeInitialTerminalBootstraps() + clearWebRuntimeInitialTerminalBootstrapsForEnvironment(trimmedEnvironmentId) } export function getWebSessionTabsTrackingGeneration(environmentId: string): number { From 2ce07dcf69ef8559a816904de1f268101c9cd8d9 Mon Sep 17 00:00:00 2001 From: Neil Date: Tue, 8 Sep 2026 03:06:35 -0700 Subject: [PATCH 05/17] fix(runtime): release the bootstrap latch when the create returns failed createWebRuntimeSessionTerminal never throws: the operation catches RPC and network failures and returns `{ status: 'failed' }`. The dispatch helper released the latch only from `catch` (dead for that path) or once a tabsByWorktree row existed (false after a failure), so a failed create left the latch held and suppressed every later auto-seed for that environment's worktree until teardown -- the opposite of its own doc comment. Capture the outcome and release on a returned failure as well, so the next focus can retry. Regression: a create that resolves `{ status: 'failed' }` followed by an effect re-run must create again. Fails on the previous release condition. --- ...ime-initial-terminal-bootstrap-dispatch.ts | 12 +++++++-- ...bs-sync-initial-terminal-relaunch.test.tsx | 25 +++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) 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 index b44e5b763d72..9869e0179ca9 100644 --- a/src/renderer/src/runtime/web-runtime-initial-terminal-bootstrap-dispatch.ts +++ b/src/renderer/src/runtime/web-runtime-initial-terminal-bootstrap-dispatch.ts @@ -1,5 +1,6 @@ import { useAppStore } from '../store' import { createWebRuntimeSessionTerminal } from './web-runtime-session' +import type { WebRuntimeTerminalCreateOutcome } from './web-runtime-session-types' import { beginWebRuntimeInitialTerminalBootstrap, endWebRuntimeInitialTerminalBootstrap @@ -26,13 +27,20 @@ export async function dispatchWebRuntimeInitialTerminalBootstrap( if (!beginWebRuntimeInitialTerminalBootstrap(environmentId, worktreeId)) { return false } + let outcome: WebRuntimeTerminalCreateOutcome try { - await createWebRuntimeSessionTerminal({ worktreeId, environmentId, activate: true }) + outcome = await createWebRuntimeSessionTerminal({ worktreeId, environmentId, activate: true }) } catch (error) { endWebRuntimeInitialTerminalBootstrap(environmentId, worktreeId) throw error } - if (Object.hasOwn(useAppStore.getState().tabsByWorktree, worktreeId)) { + // Why check the outcome: the create reports RPC and network failures as `{ status: 'failed' }` + // rather than throwing, so the catch above never sees them. Holding the latch on a returned + // failure would suppress every later auto-seed for this worktree until teardown. + if ( + outcome.status === 'failed' || + Object.hasOwn(useAppStore.getState().tabsByWorktree, worktreeId) + ) { endWebRuntimeInitialTerminalBootstrap(environmentId, worktreeId) } return true 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 index 3ea3805c9646..30f1be5cc911 100644 --- 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 @@ -229,4 +229,29 @@ describe('useWebSessionTabsSync initial-terminal bootstrap across an effect re-r 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() + }) }) From e42fbfc134318bd6a245ca72dddfdd81f1f493fe Mon Sep 17 00:00:00 2001 From: Neil Date: Tue, 8 Sep 2026 03:21:49 -0700 Subject: [PATCH 06/17] fix(runtime): release a parked bootstrap on the mirror's next frame The previous commit released the latch on a returned failure, but a create that *succeeded* with no mirrored row yet had no release at all: the row-conditional check was the only exit for the success path, so a host that accepted the tab while the mirror never got a frame held the latch until environment teardown and suppressed every later auto-seed for the worktree. Give the latch two phases. `creating` blocks other closures while the RPC is in flight. A success with no row is parked as `awaiting-mirror` instead of held, and the next frame the mirror accepts for that worktree releases it -- that frame is the mirror's 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). A create still in flight keeps its claim: releasing it on a frame would reopen the re-armed-closure race the latch exists to close. Also correct the closure-flag comment: `requestedInitialTerminal` is set only after the dispatch resolves, so a thrown create never sets it and a later frame may retry. The flag records that this subscription already owned a create; it never described a failed one. Regressions, each mutation-tested against its own term: - success with no row, then the mirror's empty answer, then a fresh closure -> must create again (fails when success-with-no-row is held instead of parked, and when the subscription does not call the frame release) - a mirror frame releases an awaiting-mirror claim but never a pending create (fails when the release ignores the phase) --- ...ime-initial-terminal-bootstrap-dispatch.ts | 24 +++---- .../web-runtime-initial-terminal-bootstrap.ts | 62 ++++++++++++++----- ...bs-sync-initial-terminal-relaunch.test.tsx | 32 +++++++++- ...ssion-tabs-sync-terminal-bootstrap.test.ts | 22 ++++++- .../active-session-subscription.ts | 19 ++++-- 5 files changed, 126 insertions(+), 33 deletions(-) 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 index 9869e0179ca9..a435f9bcb879 100644 --- a/src/renderer/src/runtime/web-runtime-initial-terminal-bootstrap-dispatch.ts +++ b/src/renderer/src/runtime/web-runtime-initial-terminal-bootstrap-dispatch.ts @@ -3,19 +3,20 @@ import { createWebRuntimeSessionTerminal } from './web-runtime-session' import type { WebRuntimeTerminalCreateOutcome } from './web-runtime-session-types' import { beginWebRuntimeInitialTerminalBootstrap, - endWebRuntimeInitialTerminalBootstrap + endWebRuntimeInitialTerminalBootstrap, + markWebRuntimeInitialTerminalBootstrapAwaitingMirror } from './web-runtime-initial-terminal-bootstrap' /** - * Claim the initial-terminal latch for this environment's worktree, create the terminal, and - * release the latch only once a mirrored row exists (or on failure, for retry). + * Claim the initial-terminal latch for this environment's worktree, create the terminal, and decide + * how the latch is released. * - * Why row-conditional and not a plain `.finally`: the snapshot refresh the create awaits can resolve - * on an empty, unconfirmed frame that leaves no `tabsByWorktree` row. Releasing then would let a - * later effect re-run seed a second terminal even though the first create succeeded. Holding the - * latch until a row exists makes the predicate decline on its own; worktree or environment teardown - * clears it either way. A failed create leaves no terminal, so it releases for a later focus to - * retry. + * 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 when this call owned the create, so the caller can keep its closure-local flag in * step; false when another closure already held the latch. @@ -35,13 +36,14 @@ export async function dispatchWebRuntimeInitialTerminalBootstrap( 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. Holding the latch on a returned - // failure would suppress every later auto-seed for this worktree until teardown. + // rather than throwing, so the catch above never sees them. if ( outcome.status === 'failed' || 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 index a32ad190452e..961a96fbdc6c 100644 --- a/src/renderer/src/runtime/web-runtime-initial-terminal-bootstrap.ts +++ b/src/renderer/src/runtime/web-runtime-initial-terminal-bootstrap.ts @@ -13,14 +13,24 @@ * 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. */ -const inFlightWorktreesByEnvironment = new Map>() +type InitialTerminalBootstrapPhase = 'creating' | 'awaiting-mirror' + +const phaseByWorktreeByEnvironment = new Map>() export function isWebRuntimeInitialTerminalBootstrapInFlight( environmentId: string, worktreeId: string ): boolean { - return inFlightWorktreesByEnvironment.get(environmentId)?.has(worktreeId) ?? false + return phaseByWorktreeByEnvironment.get(environmentId)?.has(worktreeId) ?? false } /** Claims the bootstrap for this environment's worktree; false when another closure already holds it. */ @@ -28,40 +38,64 @@ export function beginWebRuntimeInitialTerminalBootstrap( environmentId: string, worktreeId: string ): boolean { - const worktrees = inFlightWorktreesByEnvironment.get(environmentId) - if (worktrees?.has(worktreeId)) { + const phases = phaseByWorktreeByEnvironment.get(environmentId) + if (phases?.has(worktreeId)) { return false } - if (worktrees) { - worktrees.add(worktreeId) + if (phases) { + phases.set(worktreeId, 'creating') } else { - inFlightWorktreesByEnvironment.set(environmentId, new Set([worktreeId])) + 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 worktrees = inFlightWorktreesByEnvironment.get(environmentId) - if (!worktrees) { + const phases = phaseByWorktreeByEnvironment.get(environmentId) + if (!phases) { return } - worktrees.delete(worktreeId) - if (worktrees.size === 0) { - inFlightWorktreesByEnvironment.delete(environmentId) + 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 { - inFlightWorktreesByEnvironment.delete(environmentId) + phaseByWorktreeByEnvironment.delete(environmentId) } export function clearAllWebRuntimeInitialTerminalBootstraps(): void { - inFlightWorktreesByEnvironment.clear() + phaseByWorktreeByEnvironment.clear() } export function resetWebRuntimeInitialTerminalBootstrapForTests(): void { 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 index 30f1be5cc911..52f10c118564 100644 --- 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 @@ -209,8 +209,8 @@ describe('useWebSessionTabsSync initial-terminal bootstrap across an effect re-r // 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 resolves but writes no tabsByWorktree row (host has not published the tab yet). - mocks.createTerminal.mockResolvedValue(undefined) + // 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) @@ -254,4 +254,32 @@ describe('useWebSessionTabsSync initial-terminal bootstrap across an effect re-r 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 1e00dcb975ff..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 @@ -18,7 +18,9 @@ import { import { beginWebRuntimeInitialTerminalBootstrap, endWebRuntimeInitialTerminalBootstrap, - isWebRuntimeInitialTerminalBootstrapInFlight + isWebRuntimeInitialTerminalBootstrapInFlight, + markWebRuntimeInitialTerminalBootstrapAwaitingMirror, + releaseWebRuntimeInitialTerminalBootstrapOnMirrorFrame } from './web-runtime-initial-terminal-bootstrap' const OTHER_ENV = 'web-env-2' @@ -203,6 +205,24 @@ describe('applyWebSessionTabsSnapshot', () => { ).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. 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 c9d44fbc0bf0..78941098c5e8 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 @@ -36,7 +36,10 @@ import { shouldSkipWebRuntimeWakeTerminalRespawn } from '../web-runtime-wake-terminal-respawn' import { createWebRuntimeSessionTerminal } from '../web-runtime-session' -import { isWebRuntimeInitialTerminalBootstrapInFlight } from '../web-runtime-initial-terminal-bootstrap' +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' @@ -145,10 +148,11 @@ export function installActiveSessionTabsSubscription({ const bootstrap = shouldBootstrapInitialWebRuntimeTerminal({ event: recoveredEvent, activeWorktreeId, - // Why both: the closure flag keeps one failed attempt from retrying on every later frame of - // the same subscription, and 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). + // 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), @@ -192,6 +196,11 @@ 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) { From e0ce8c808adb9a35ef90057b579ac72f1f262dfb Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 06:46:01 -0700 Subject: [PATCH 07/17] fix(runtime): let a failed create retry inside its own subscription The returned-failure release freed the module latch but the caller still latched its closure-local requestedInitialTerminal whenever the dispatch reported it owned the create, including a create that returned { status: 'failed' }. A thrown failure never set it and retried on the next frame; since every RPC and network failure is reported as a return, the live path was the one that suppressed the whole subscription. The dispatch now reports false for a failure whichever way it arrives. --- ...ime-initial-terminal-bootstrap-dispatch.ts | 17 +++++++------ ...bs-sync-initial-terminal-relaunch.test.tsx | 25 +++++++++++++++++++ 2 files changed, 35 insertions(+), 7 deletions(-) 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 index a435f9bcb879..d4f097a2a688 100644 --- a/src/renderer/src/runtime/web-runtime-initial-terminal-bootstrap-dispatch.ts +++ b/src/renderer/src/runtime/web-runtime-initial-terminal-bootstrap-dispatch.ts @@ -18,8 +18,10 @@ import { * 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 when this call owned the create, so the caller can keep its closure-local flag in - * step; false when another closure already held the latch. + * 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, @@ -36,11 +38,12 @@ export async function dispatchWebRuntimeInitialTerminalBootstrap( 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. - if ( - outcome.status === 'failed' || - Object.hasOwn(useAppStore.getState().tabsByWorktree, worktreeId) - ) { + // 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) 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 index 52f10c118564..fb7808e472de 100644 --- 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 @@ -255,6 +255,31 @@ describe('useWebSessionTabsSync initial-terminal bootstrap across an effect re-r 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 From 185781476df763eb73e6cb24c773a184d110c435 Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 07:01:52 -0700 Subject: [PATCH 08/17] fix(runtime): close the second re-seed door on an emptied workspace ensureWebRuntimeWorktreeTerminalAfterWake read tabsByWorktree through `?? []`, and the only guard that could have seen presence was &&-gated on the value that erased it, so it could never fire for a tombstone. It runs on every activateAndRevealWorktree, not only after a wake, and a tombstoned workspace routes exclusively here: the stream-frame path returns at localTerminalCount === 0 before reaching its own guard. So the workspace was re-seeded on every focus regardless of the mirror fix. Two states shared one line and now do not. With no rows the workspace is being seeded for the first time and the decision goes to shouldAutoCreateInitialTerminal with presence read by Object.hasOwn; with rows present the question is whether a woke workspace's chrome outlived its PTYs, which the tombstone says nothing about. Both arms are pinned, because reverting only the respawn arm broke no existing test. --- ...eb-runtime-worktree-terminal-after-wake.ts | 122 +++++---------- .../worktree-activation-empty-remote.test.ts | 147 ++++++++++++++++++ 2 files changed, 184 insertions(+), 85 deletions(-) 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..ac3a40bd87fb 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 @@ -11,114 +11,66 @@ import { endWebRuntimeWakeTerminalRespawn } from '@/runtime/web-runtime-wake-terminal-respawn' import { getRuntimeEnvironmentIdForWorktree } from '@/lib/worktree-runtime-owner' -import { - draftViewModeProps, - resolveStartupLaunchDraftText, - type WorktreeStartupPayload -} from '@/lib/worktree-startup-payload' -import type { TuiAgent } from '../../../shared/tui-agent' -import { initialAgentTabViewModeProps } from '@/lib/native-chat-initial-view-mode' -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, - opts?: { - runtimeEnvironmentId?: string | null - startup?: WorktreeStartupPayload - agent?: TuiAgent | null - activate?: boolean - } -): void { +export function ensureWebRuntimeWorktreeTerminalAfterWake(worktreeId: string): void { const state = useAppStore.getState() - const runtimeEnvironmentId = - opts && 'runtimeEnvironmentId' in opts - ? (opts.runtimeEnvironmentId ?? null) - : getRuntimeEnvironmentIdForWorktree(state, worktreeId) + const worktree = state.getKnownWorktreeById(worktreeId) + if (!worktree) { + return + } + const runtimeEnvironmentId = getRuntimeEnvironmentIdForWorktree(state, worktree.id) if (!runtimeEnvironmentId || !isWebRuntimeSessionActive(runtimeEnvironmentId)) { return } const tabs = state.tabsByWorktree[worktreeId] ?? [] - const launchAgent = opts?.startup?.launchAgent ?? opts?.agent ?? undefined - if ( - launchAgent && - tabs.some( - (tab) => - tab.launchAgent === launchAgent && - (isWebTerminalSurfaceTabId(tab.id) || tabHasLivePty(state.ptyIdsByTabId, tab.id)) - ) - ) { + const hasLivePty = tabs.some((tab) => tabHasLivePty(state.ptyIdsByTabId, tab.id)) + if (hasLivePty) { return } - if (!launchAgent) { - const hasLivePty = tabs.some((tab) => tabHasLivePty(state.ptyIdsByTabId, tab.id)) - if (hasLivePty) { - return - } - - const hasMirroredHostTabs = tabs.some((tab) => isWebTerminalSurfaceTabId(tab.id)) - if (hasMirroredHostTabs) { - // Why: the host session still owns these tabs — wait for the mirror to repopulate PTY handles instead of duplicating a terminal. - return - } + const hasMirroredHostTabs = tabs.some((tab) => isWebTerminalSurfaceTabId(tab.id)) + if (hasMirroredHostTabs) { + // Why: the host session still owns these tabs — wait for the mirror to repopulate PTY handles instead of duplicating a terminal. + return + } - if (getLastKnownHostTerminalTabCount(runtimeEnvironmentId, worktreeId) > 0) { - return - } + if (getLastKnownHostTerminalTabCount(runtimeEnvironmentId, worktreeId) > 0) { + return + } - const { renderableTabCount } = state.reconcileWorktreeTabModel(worktreeId) - if (tabs.length > 0 && renderableTabCount === 0) { + // Why the branch split: two states used to share one line. With NO rows the workspace is being + // seeded for the first time, and that decision belongs to the canonical predicate — an explicit + // empty row is the closed-last-terminal tombstone, not "never initialized", and this is the only + // door a tombstoned workspace reaches (the stream-frame path returns before it). With rows + // present the question is instead whether a woke workspace's tab chrome outlived its PTYs, which + // the tombstone has nothing to say about. + const { renderableTabCount } = state.reconcileWorktreeTabModel(worktreeId) + if (tabs.length === 0) { + if ( + !shouldAutoCreateInitialTerminal( + renderableTabCount, + Object.hasOwn(state.tabsByWorktree, worktreeId) + ) + ) { return } + } else if (renderableTabCount === 0) { + return } if (!beginWebRuntimeWakeTerminalRespawn(worktreeId)) { return } - const startup = opts?.startup - const viewModeProps = launchAgent - ? initialAgentTabViewModeProps(state.settings, { - agent: launchAgent, - ...draftViewModeProps(resolveStartupLaunchDraftText(startup)), - nativeChatTranscriptIsLocalReadable: isNativeChatTranscriptLocalReadable( - getConnectionId(worktreeId) - ) - }) - : {} - // Why: sleep keeps tab rows but terminal.stop clears host PTYs, while a failed create receipt leaves a selected agent with no host surface. + // Why: sleep keeps tab rows but terminal.stop clears host PTYs, so a woke workspace can have tab chrome but no surface. void createWebRuntimeSessionTerminal({ worktreeId, environmentId: runtimeEnvironmentId, - ...viewModeProps, - ...(startup - ? { - command: startup.command, - ...(startup.env ? { env: startup.env } : {}), - ...(startup.launchConfig ? { launchConfig: startup.launchConfig } : {}), - ...(startup.launchToken ? { launchToken: startup.launchToken } : {}), - ...(launchAgent ? { launchAgent, preparedAgentCommand: true } : {}), - ...(startup.startupCommandDelivery - ? { startupCommandDelivery: startup.startupCommandDelivery } - : {}) - } - : launchAgent - ? { agent: launchAgent } - : {}), - activate: opts?.activate !== false, + activate: true, selectWorktree: false + }).finally(() => { + endWebRuntimeWakeTerminalRespawn(worktreeId) }) - .then((outcome) => { - if (outcome.status === 'failed') { - toast.error(outcome.message, { - id: `web-runtime-worktree-terminal:${runtimeEnvironmentId}:${worktreeId}` - }) - } - }) - .finally(() => { - endWebRuntimeWakeTerminalRespawn(worktreeId) - }) } 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() + }) + }) }) From e1458bf028b0ea0f540990bbadb8c6a88f345b87 Mon Sep 17 00:00:00 2001 From: Neil Date: Thu, 10 Sep 2026 07:30:54 -0700 Subject: [PATCH 09/17] fix(runtime): stop the worktree-id re-key inventing a tombstone canonicalizeTerminalSessionWorktreeId read the source row through `?? []` and always wrote the target one, so a workspace with NO tabsByWorktree row came out the other side with an explicit empty one. That is the closed-last-terminal tombstone, and this PR's new activation-path reader honours it, so the re-keyed workspace never gets its initial terminal. Guarded on the source row's presence, which is the guard the sibling keyed maps six lines below already use. Mutating the guard to test emptiness instead of presence breaks the assertion that a real tombstone still survives the re-key, which is the distinction that matters. --- .../workspace-session-worktree-id.test.ts | 62 +++++++++++++++++++ .../runtime/workspace-session-worktree-id.ts | 18 ++++-- 2 files changed, 74 insertions(+), 6 deletions(-) create mode 100644 src/main/runtime/workspace-session-worktree-id.test.ts 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) { From 22bd6bdac6dccbac9a54b34ab7ac0a6202b56c15 Mon Sep 17 00:00:00 2001 From: Neil Date: Sat, 19 Sep 2026 01:03:04 -0700 Subject: [PATCH 10/17] fix(runtime): reconcile session tab bootstrap integration --- ...-session-tabs-sync-initial-terminal-relaunch.test.tsx | 9 +++++++-- .../runtime/web-session-tabs-sync/tracking-decisions.ts | 1 - .../src/runtime/web-session-tabs-sync/tracking.ts | 8 ++++++++ 3 files changed, 15 insertions(+), 3 deletions(-) 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 index fb7808e472de..e0c6f2243112 100644 --- 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 @@ -11,11 +11,16 @@ const mocks = vi.hoisted(() => ({ createTerminal: vi.fn(), getExplicitRuntimeEnvironmentIdForWorktree: vi.fn(), recoverSnapshot: vi.fn(), - runtimeSessionMirrorEnvironmentKey: vi.fn() + runtimeSessionMirrorEnvironmentKey: vi.fn(), + runtimeSessionMirrorEnvironmentKeys: vi.fn(() => ({ + environmentKey: null, + resubscribeSignal: 0 + })) })) vi.mock('./use-runtime-session-mirror-environment-key', () => ({ - useRuntimeSessionMirrorEnvironmentKey: mocks.runtimeSessionMirrorEnvironmentKey + useRuntimeSessionMirrorEnvironmentKey: mocks.runtimeSessionMirrorEnvironmentKey, + useRuntimeSessionMirrorEnvironmentKeys: mocks.runtimeSessionMirrorEnvironmentKeys })) vi.mock('@/lib/worktree-runtime-owner', async (importOriginal) => { 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 216574fb2e24..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,7 +18,6 @@ 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' diff --git a/src/renderer/src/runtime/web-session-tabs-sync/tracking.ts b/src/renderer/src/runtime/web-session-tabs-sync/tracking.ts index f117ef4a714a..707e5fb5b7f5 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync/tracking.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync/tracking.ts @@ -218,6 +218,14 @@ export function shouldApplyRecoveredWebSessionTabsSnapshot( return snapshot.snapshotVersion >= latest.snapshotVersion } +export function beginWebSessionTabsSnapshotRecovery( + _environmentId: string, + _worktreeId: string, + _receivedFrame: number +): () => void { + return () => {} +} + export function recordAcceptedWebSessionTabsEnvironment( environmentId: string, snapshot: RuntimeMobileSessionTabsResult From bbd56bbf5bc4d5bf69ed14685030b874fde4c640 Mon Sep 17 00:00:00 2001 From: Neil Date: Sat, 19 Sep 2026 01:04:56 -0700 Subject: [PATCH 11/17] fix(runtime): preserve terminal wake launch options --- ...eb-runtime-worktree-terminal-after-wake.ts | 126 +++++++++++++----- 1 file changed, 94 insertions(+), 32 deletions(-) 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 ac3a40bd87fb..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 @@ -11,66 +11,128 @@ import { endWebRuntimeWakeTerminalRespawn } from '@/runtime/web-runtime-wake-terminal-respawn' import { getRuntimeEnvironmentIdForWorktree } from '@/lib/worktree-runtime-owner' +import { + draftViewModeProps, + resolveStartupLaunchDraftText, + type WorktreeStartupPayload +} from '@/lib/worktree-startup-payload' +import type { TuiAgent } from '../../../shared/tui-agent' +import { initialAgentTabViewModeProps } from '@/lib/native-chat-initial-view-mode' +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): void { +export function ensureWebRuntimeWorktreeTerminalAfterWake( + worktreeId: string, + opts?: { + runtimeEnvironmentId?: string | null + startup?: WorktreeStartupPayload + agent?: TuiAgent | null + activate?: boolean + } +): void { const state = useAppStore.getState() const worktree = state.getKnownWorktreeById(worktreeId) if (!worktree) { return } - const runtimeEnvironmentId = getRuntimeEnvironmentIdForWorktree(state, worktree.id) + const runtimeEnvironmentId = + opts && 'runtimeEnvironmentId' in opts + ? (opts.runtimeEnvironmentId ?? null) + : getRuntimeEnvironmentIdForWorktree(state, worktreeId) if (!runtimeEnvironmentId || !isWebRuntimeSessionActive(runtimeEnvironmentId)) { return } const tabs = state.tabsByWorktree[worktreeId] ?? [] - const hasLivePty = tabs.some((tab) => tabHasLivePty(state.ptyIdsByTabId, tab.id)) - if (hasLivePty) { + const launchAgent = opts?.startup?.launchAgent ?? opts?.agent ?? undefined + if ( + launchAgent && + tabs.some( + (tab) => + tab.launchAgent === launchAgent && + (isWebTerminalSurfaceTabId(tab.id) || tabHasLivePty(state.ptyIdsByTabId, tab.id)) + ) + ) { return } - const hasMirroredHostTabs = tabs.some((tab) => isWebTerminalSurfaceTabId(tab.id)) - if (hasMirroredHostTabs) { - // Why: the host session still owns these tabs — wait for the mirror to repopulate PTY handles instead of duplicating a terminal. - return - } + if (!launchAgent) { + const hasLivePty = tabs.some((tab) => tabHasLivePty(state.ptyIdsByTabId, tab.id)) + if (hasLivePty) { + return + } - if (getLastKnownHostTerminalTabCount(runtimeEnvironmentId, worktreeId) > 0) { - return - } + const hasMirroredHostTabs = tabs.some((tab) => isWebTerminalSurfaceTabId(tab.id)) + if (hasMirroredHostTabs) { + // Why: the host session still owns these tabs — wait for the mirror to repopulate PTY handles instead of duplicating a terminal. + return + } - // Why the branch split: two states used to share one line. With NO rows the workspace is being - // seeded for the first time, and that decision belongs to the canonical predicate — an explicit - // empty row is the closed-last-terminal tombstone, not "never initialized", and this is the only - // door a tombstoned workspace reaches (the stream-frame path returns before it). With rows - // present the question is instead whether a woke workspace's tab chrome outlived its PTYs, which - // the tombstone has nothing to say about. - const { renderableTabCount } = state.reconcileWorktreeTabModel(worktreeId) - if (tabs.length === 0) { - if ( - !shouldAutoCreateInitialTerminal( - renderableTabCount, - Object.hasOwn(state.tabsByWorktree, worktreeId) - ) - ) { + if (getLastKnownHostTerminalTabCount(runtimeEnvironmentId, worktreeId) > 0) { + return + } + + const { renderableTabCount } = state.reconcileWorktreeTabModel(worktreeId) + if (tabs.length === 0) { + if ( + !shouldAutoCreateInitialTerminal( + renderableTabCount, + Object.hasOwn(state.tabsByWorktree, worktreeId) + ) + ) { + return + } + } else if (renderableTabCount === 0) { return } - } else if (renderableTabCount === 0) { - return } if (!beginWebRuntimeWakeTerminalRespawn(worktreeId)) { return } - // Why: sleep keeps tab rows but terminal.stop clears host PTYs, so a woke workspace can have tab chrome but no surface. + const startup = opts?.startup + const viewModeProps = launchAgent + ? initialAgentTabViewModeProps(state.settings, { + agent: launchAgent, + ...draftViewModeProps(resolveStartupLaunchDraftText(startup)), + nativeChatTranscriptIsLocalReadable: isNativeChatTranscriptLocalReadable( + getConnectionId(worktreeId) + ) + }) + : {} + // Why: sleep keeps tab rows but terminal.stop clears host PTYs, while a failed create receipt leaves a selected agent with no host surface. void createWebRuntimeSessionTerminal({ worktreeId, environmentId: runtimeEnvironmentId, - activate: true, + ...viewModeProps, + ...(startup + ? { + command: startup.command, + ...(startup.env ? { env: startup.env } : {}), + ...(startup.launchConfig ? { launchConfig: startup.launchConfig } : {}), + ...(startup.launchToken ? { launchToken: startup.launchToken } : {}), + ...(launchAgent ? { launchAgent, preparedAgentCommand: true } : {}), + ...(startup.startupCommandDelivery + ? { startupCommandDelivery: startup.startupCommandDelivery } + : {}) + } + : launchAgent + ? { agent: launchAgent } + : {}), + activate: opts?.activate !== false, selectWorktree: false - }).finally(() => { - endWebRuntimeWakeTerminalRespawn(worktreeId) }) + .then((outcome) => { + if (outcome.status === 'failed') { + toast.error(outcome.message, { + id: `web-runtime-worktree-terminal:${runtimeEnvironmentId}:${worktreeId}` + }) + } + }) + .finally(() => { + endWebRuntimeWakeTerminalRespawn(worktreeId) + }) } From bbb88de9fcc8ff35311860c1e9395140cbf76fa0 Mon Sep 17 00:00:00 2001 From: Neil Date: Sat, 19 Sep 2026 01:05:42 -0700 Subject: [PATCH 12/17] test(session): annotate cross-project persistence fixture --- .../src/lib/default-terminal-tabs-applied-persist.test.ts | 1 + 1 file changed, 1 insertion(+) 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 index 664a9279b866..9723de267a9a 100644 --- a/src/renderer/src/lib/default-terminal-tabs-applied-persist.test.ts +++ b/src/renderer/src/lib/default-terminal-tabs-applied-persist.test.ts @@ -3,6 +3,7 @@ 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' +// @ts-expect-error -- this integration spec intentionally exercises the shared main-process merge helper. import { preserveRuntimeAuthoredWorkspaceSessionFields } from '../../../main/persistence/runtime-authored-workspace-session-fields' import { createSessionWriteSubscriber, From a5d5a4e6a88e09fd2e06b3cdf9eaacc511978829 Mon Sep 17 00:00:00 2001 From: Neil Date: Sat, 19 Sep 2026 01:06:04 -0700 Subject: [PATCH 13/17] test(session): remove obsolete typecheck suppression --- .../src/lib/default-terminal-tabs-applied-persist.test.ts | 1 - 1 file changed, 1 deletion(-) 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 index 9723de267a9a..664a9279b866 100644 --- a/src/renderer/src/lib/default-terminal-tabs-applied-persist.test.ts +++ b/src/renderer/src/lib/default-terminal-tabs-applied-persist.test.ts @@ -3,7 +3,6 @@ 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' -// @ts-expect-error -- this integration spec intentionally exercises the shared main-process merge helper. import { preserveRuntimeAuthoredWorkspaceSessionFields } from '../../../main/persistence/runtime-authored-workspace-session-fields' import { createSessionWriteSubscriber, From c50aff099b2b4e6ff3049fbea349ab48af739de3 Mon Sep 17 00:00:00 2001 From: Neil Date: Sat, 19 Sep 2026 01:07:09 -0700 Subject: [PATCH 14/17] test(session): keep persistence regression in renderer project --- ...fault-terminal-tabs-applied-persist.test.ts | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) 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 index 664a9279b866..f8398e9ca9f6 100644 --- a/src/renderer/src/lib/default-terminal-tabs-applied-persist.test.ts +++ b/src/renderer/src/lib/default-terminal-tabs-applied-persist.test.ts @@ -3,7 +3,6 @@ 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 { preserveRuntimeAuthoredWorkspaceSessionFields } from '../../../main/persistence/runtime-authored-workspace-session-fields' import { createSessionWriteSubscriber, type WorkspaceSessionWrite @@ -18,6 +17,23 @@ const DEFAULT_TABS = { 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() From bc8e0261db6bb9ed0ed163a3b33fccd1f7c67da2 Mon Sep 17 00:00:00 2001 From: Neil Date: Sat, 19 Sep 2026 14:17:33 -0700 Subject: [PATCH 15/17] refactor(runtime): remove inert snapshot recovery wrapper --- .../web-session-tabs-sync/active-session-subscription.ts | 7 ------- .../src/runtime/web-session-tabs-sync/tracking.ts | 8 -------- 2 files changed, 15 deletions(-) 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 78941098c5e8..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 @@ -6,7 +6,6 @@ import { getRuntimeEnvironmentRevision } from '../runtime-environment-revision' import { recoverWebSessionTerminalOrphansBeforeApply } from '../web-session-terminal-orphan-recovery' import { installWindowVisibilitySubscriptionParking } from '../window-visibility-subscription-parking' import { - beginWebSessionTabsSnapshotRecovery, recordReceivedWebSessionTabsSnapshot, shouldApplyRecoveredWebSessionTabsSnapshot } from './tracking' @@ -264,11 +263,6 @@ export function installActiveSessionTabsSubscription({ runtimeId ) visibilitySnapshotReceipt.current(environmentId, event, frame, runtimeId) - const finish = beginWebSessionTabsSnapshotRecovery( - environmentId, - event.worktree, - frame - ) void applyActiveSnapshot(event, response, isCurrent, frame, runtimeId) .catch((error) => { if (isCurrent()) { @@ -277,7 +271,6 @@ export function installActiveSessionTabsSubscription({ return null }) .then((settle) => { - finish() if (isCurrent()) { settle?.() } diff --git a/src/renderer/src/runtime/web-session-tabs-sync/tracking.ts b/src/renderer/src/runtime/web-session-tabs-sync/tracking.ts index 707e5fb5b7f5..f117ef4a714a 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync/tracking.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync/tracking.ts @@ -218,14 +218,6 @@ export function shouldApplyRecoveredWebSessionTabsSnapshot( return snapshot.snapshotVersion >= latest.snapshotVersion } -export function beginWebSessionTabsSnapshotRecovery( - _environmentId: string, - _worktreeId: string, - _receivedFrame: number -): () => void { - return () => {} -} - export function recordAcceptedWebSessionTabsEnvironment( environmentId: string, snapshot: RuntimeMobileSessionTabsResult From afc62a40125be3f7091d367f814a264c998a4f8c Mon Sep 17 00:00:00 2001 From: Neil Date: Sat, 19 Sep 2026 14:21:52 -0700 Subject: [PATCH 16/17] fix(runtime): keep removal-frame import merge-safe --- .../src/runtime/web-session-tabs-sync/apply-preparation-base.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 6085a4ecc554..2c72847211cc 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 @@ -1,4 +1,5 @@ import type { RuntimeMobileSessionTabsResult } from '../../../../shared/runtime-types' +import { isWebSessionTabsWorktreeRemovalFrame } from './session-tabs-inventory-absence' import type { WebSessionTabsBatchContext, WebSessionTabsSnapshotApplyOptions, @@ -27,7 +28,6 @@ import { shouldReplaceTerminalTab } from './terminal-surfaces' import { buildMirroredTerminalTabs } from './terminal-build' -import { isWebSessionTabsWorktreeRemovalFrame } from './session-tabs-inventory-absence' import { hostSnapshotAffirmsWorktreeContents } from '../host-session-snapshot-authority' export function prepareWebSessionTabsSnapshotBase( From 0e5b4f491d520bd580d630f65dee27a73a8f3819 Mon Sep 17 00:00:00 2001 From: Neil Date: Sat, 19 Sep 2026 14:28:10 -0700 Subject: [PATCH 17/17] fix(runtime): deduplicate merged removal-frame import --- .../src/runtime/web-session-tabs-sync/apply-preparation-base.ts | 1 - 1 file changed, 1 deletion(-) 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 2dcaf7837c0c..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 @@ -1,7 +1,6 @@ import { collectUnhydratedMirroredTabRetractions } from './mirrored-status-tab-retractions' import { isWebSessionTabsWorktreeRemovalFrame } from './session-tabs-inventory-absence' import type { RuntimeMobileSessionTabsResult } from '../../../../shared/runtime-types' -import { isWebSessionTabsWorktreeRemovalFrame } from './session-tabs-inventory-absence' import type { WebSessionTabsBatchContext, WebSessionTabsSnapshotApplyOptions,