diff --git a/src/renderer/src/hooks/remote-workspace-deferred-placement-retry.ts b/src/renderer/src/hooks/remote-workspace-deferred-placement-retry.ts new file mode 100644 index 00000000000..904ae273234 --- /dev/null +++ b/src/renderer/src/hooks/remote-workspace-deferred-placement-retry.ts @@ -0,0 +1,93 @@ +import type { DirectSshAuthority } from '../../../shared/ssh-types' +import type { RemoteWorkspaceObservedSnapshot } from '../../../shared/remote-workspace-types' +import { directSshAuthoritiesEqual } from './direct-ssh-reconnect-tokens' +import { + waitForSnapshotWorktreePlacement, + type RemoteWorkspaceSnapshotPlacementStore +} from './remote-workspace-snapshot-placement' + +/** How long a conflicted target keeps watching the catalog for the rows it could not place. The + * catalog for a remote host often only fills in when the user opens the worktree, which is minutes + * after connect, and `conflict` has no other exit: it suppresses uploads and holds terminal + * authority at `unverifiable` until a reconnect or an unsolicited host push happens to arrive. */ +const DEFERRED_SNAPSHOT_PLACEMENT_TIMEOUT_MS = 600_000 + +export type DeferredSnapshotPlacementRetryDeps = { + store: RemoteWorkspaceSnapshotPlacementStore + getCurrentAuthority: (targetId: string) => DirectSshAuthority | null + getSnapshot: (targetId: string) => Promise + applySnapshot: (targetId: string, snapshot: RemoteWorkspaceObservedSnapshot) => Promise +} + +export type DeferredSnapshotPlacementRetries = { + /** Empty `worktreePaths` retires the target's outstanding watch instead of arming one. */ + watch: (authority: DirectSshAuthority, worktreePaths: readonly string[]) => void + stop: () => void +} + +/** + * Re-pull once the local catalog can place the host paths an apply had to drop. + * + * Why a fresh pull rather than replaying the snapshot already held: that snapshot is only evidence + * of what the host had when it was taken. The host owns execution state, so the answer acted on has + * to be its current one — see docs/reference/ssh-execution-boundary.md. A pull that comes back + * empty or fails is left alone: it is `unverifiable`, and moving the target off `conflict` on it + * would re-authorise uploads and sleeping-agent resume from a picture known to be incomplete. + */ +export function createDeferredSnapshotPlacementRetries( + deps: DeferredSnapshotPlacementRetryDeps +): DeferredSnapshotPlacementRetries { + const watchers = new Map() + let stopped = false + + const watch = (authority: DirectSshAuthority, worktreePaths: readonly string[]): void => { + // Always retire the previous watch: an apply that placed everything passes no paths, and that + // is exactly when the outstanding watch is obsolete. + watchers.get(authority.targetId)?.abort() + watchers.delete(authority.targetId) + if (stopped || worktreePaths.length === 0) { + return + } + const controller = new AbortController() + watchers.set(authority.targetId, controller) + const isCurrent = (): boolean => + !stopped && + watchers.get(authority.targetId) === controller && + directSshAuthoritiesEqual(deps.getCurrentAuthority(authority.targetId), authority) + void (async () => { + try { + const placed = await waitForSnapshotWorktreePlacement( + deps.store, + authority, + worktreePaths, + isCurrent, + controller.signal, + DEFERRED_SNAPSHOT_PLACEMENT_TIMEOUT_MS + ) + if (!placed || !isCurrent()) { + return + } + const snapshot = await deps.getSnapshot(authority.targetId) + if (!snapshot || snapshot.revision <= 0 || !isCurrent()) { + return + } + await deps.applySnapshot(authority.targetId, snapshot) + } finally { + if (watchers.get(authority.targetId) === controller) { + watchers.delete(authority.targetId) + } + } + })() + } + + return { + watch, + stop: () => { + stopped = true + for (const controller of watchers.values()) { + controller.abort() + } + watchers.clear() + } + } +} diff --git a/src/renderer/src/hooks/remote-workspace-snapshot-apply.ts b/src/renderer/src/hooks/remote-workspace-snapshot-apply.ts index 024fb3f51c0..af2e845ab51 100644 --- a/src/renderer/src/hooks/remote-workspace-snapshot-apply.ts +++ b/src/renderer/src/hooks/remote-workspace-snapshot-apply.ts @@ -79,6 +79,12 @@ type RemoteWorkspaceSnapshotApplyInput = { isPreparationTokenCurrent: (token: DirectSshPreparationToken) => boolean waitForWorkspaceSessionReady: (signal?: AbortSignal) => Promise finalizeHydratedTerminals: (authority: DirectSshAuthority) => number + /** + * Host paths still carrying terminal tabs when this apply gave up placing them. `unverifiable`, + * never proof the rows are not ours, so the caller owns getting back to a placed picture — this + * apply itself has no way back once the bounded in-apply wait expires. + */ + onUnplacedTabWorktreePaths?: (worktreePaths: readonly string[]) => void } export type RemoteWorkspaceSnapshotApplyResult = 'applied' | 'stale' | 'failed' @@ -115,7 +121,8 @@ export async function applyDirectSshRemoteWorkspaceSnapshot({ isArrivalCurrent, isPreparationTokenCurrent, waitForWorkspaceSessionReady, - finalizeHydratedTerminals + finalizeHydratedTerminals, + onUnplacedTabWorktreePaths }: RemoteWorkspaceSnapshotApplyInput): Promise { const { authority } = token if (!isArrivalCurrent(authority.targetId, arrival)) { @@ -181,6 +188,7 @@ export async function applyDirectSshRemoteWorkspaceSnapshot({ return 'stale' } const hasUnplacedTerminalTabs = unplacedTabWorktreePaths.length > 0 + onUnplacedTabWorktreePaths?.([...unplacedTabWorktreePaths]) snapshotApplyDepth += 1 try { const currentStore = store.getState() diff --git a/src/renderer/src/hooks/remote-workspace-snapshot-placement.ts b/src/renderer/src/hooks/remote-workspace-snapshot-placement.ts index 0d280bc6d9e..29925baa4cb 100644 --- a/src/renderer/src/hooks/remote-workspace-snapshot-placement.ts +++ b/src/renderer/src/hooks/remote-workspace-snapshot-placement.ts @@ -39,6 +39,23 @@ export function resolveDirectSshSnapshotWorktreeIds( return worktreeIds } +/** Only the rows the snapshot is allowed to replace, with no host-qualified widening. */ +export function resolveExactDirectSshTargetWorktreeIds( + state: AppState, + authority: DirectSshAuthority +): Set { + return resolveDirectSshTargetScope({ + targetId: authority.targetId, + catalogRevision: 0, + repos: state.repos, + worktreesByRepo: state.worktreesByRepo, + detectedWorktreesByRepo: state.detectedWorktreesByRepo, + folderWorkspaces: state.folderWorkspaces, + projectGroups: state.projectGroups, + restoredRuntimeHostIdByWorkspaceSessionKey: state.restoredRuntimeHostIdByWorkspaceSessionKey + }).gitWorktreeIds +} + function snapshotPathsArePlaceable( state: AppState, authority: DirectSshAuthority, @@ -85,7 +102,8 @@ export async function waitForSnapshotWorktreePlacement( authority: DirectSshAuthority, worktreePaths: readonly string[], isCurrent: () => boolean, - signal?: AbortSignal + signal?: AbortSignal, + timeoutMs: number = SNAPSHOT_WORKTREE_PLACEMENT_TIMEOUT_MS ): Promise { if (signal?.aborted || !isCurrent()) { return false @@ -117,7 +135,7 @@ export async function waitForSnapshotWorktreePlacement( resolve(placed) } const onAbort = (): void => finish(false) - timer = setTimeout(() => finish(false), SNAPSHOT_WORKTREE_PLACEMENT_TIMEOUT_MS) + timer = setTimeout(() => finish(false), timeoutMs) signal?.addEventListener('abort', onAbort, { once: true }) const subscribedUnsubscribe = store.subscribe((state) => { if (!isCurrent()) { diff --git a/src/renderer/src/hooks/remote-workspace-snapshot-unplaced-tab-adoption.test.ts b/src/renderer/src/hooks/remote-workspace-snapshot-unplaced-tab-adoption.test.ts index 6be1721438a..13daf0c9f20 100644 --- a/src/renderer/src/hooks/remote-workspace-snapshot-unplaced-tab-adoption.test.ts +++ b/src/renderer/src/hooks/remote-workspace-snapshot-unplaced-tab-adoption.test.ts @@ -12,8 +12,10 @@ * * The oracle is the promotion, not the drop. Without a local catalog there is nowhere to put the * rows, and that is fine and recoverable — what is not recoverable is declaring the empty result - * authoritative, because nothing re-pulls after the lineage lands. An unplaceable row is - * `unverifiable`, never `exited` (docs/reference/ssh-execution-boundary.md). + * authoritative. An unplaceable row is `unverifiable`, never `exited` + * (docs/reference/ssh-execution-boundary.md). Getting back to a placed picture is the caller's job: + * this apply has no way back once its bounded wait expires, so it reports the paths it dropped and + * remote-workspace-target-sync.ts re-pulls when the catalog can place them. * * The catalog-present case is pinned alongside it so the gate cannot be satisfied by never * hydrating anything. @@ -275,7 +277,8 @@ describe('a host snapshot whose terminal tabs cannot be placed locally', () => { expect(adoptedTabIds(store), 'no local worktree row exists to hang the host tabs on').toEqual( [] ) - // Not recoverable: promoting that to truth. Nothing re-pulls once the lineage lands. + // Not recoverable: promoting that to truth. This apply never re-pulls on its own; the deferred + // placement watch in remote-workspace-target-sync.ts is what re-pulls once the lineage lands. expect( isHydrated(store), 'the host named 3 terminals and this client placed none of them, so the target is not hydrated' @@ -335,7 +338,7 @@ describe('a host snapshot whose terminal tabs cannot be placed locally', () => { const store = createStore() // First pass: the lineage read was degraded, so nothing places and the target is left - // un-hydrated on purpose. With the retry chain gone, this is the only way back. + // un-hydrated on purpose. A later snapshot is how this apply, on its own, gets back. await applySnapshot(store, snapshot(1)) expect(adoptedTabIds(store)).toEqual([]) expect(isHydrated(store)).toBe(false) diff --git a/src/renderer/src/hooks/remote-workspace-target-sync-types.ts b/src/renderer/src/hooks/remote-workspace-target-sync-types.ts new file mode 100644 index 00000000000..40d5b488c41 --- /dev/null +++ b/src/renderer/src/hooks/remote-workspace-target-sync-types.ts @@ -0,0 +1,45 @@ +import type { + RemoteWorkspaceObservedPatchResult, + RemoteWorkspaceObservedSnapshot +} from '../../../shared/remote-workspace-types' +import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' +import type { DirectSshAuthority } from '../../../shared/ssh-types' +import type { + DirectSshPreparationInput, + DirectSshPreparationOutcome, + DirectSshPreparationToken +} from './direct-ssh-reconnect-coordinator' +import type { RemoteWorkspaceSnapshotPlacementStore } from './remote-workspace-snapshot-placement' + +export type RemoteWorkspaceApi = { + get: (args: { targetId: string }) => Promise + setForConnectedTargets: (args: { + session?: WorkspaceSessionState + hydratedTargetIds?: string[] + expectedRevisionsByTargetId: Record + expectedHostObservationTokensByTargetId: Record + }) => Promise<{ targetId: string; result: RemoteWorkspaceObservedPatchResult }[]> +} + +export type RemoteWorkspaceTargetSyncDeps = { + store: RemoteWorkspaceSnapshotPlacementStore + remoteWorkspace: RemoteWorkspaceApi + getCurrentAuthority: (targetId: string) => DirectSshAuthority | null + isPreparationTokenCurrent: (token: DirectSshPreparationToken) => boolean + capturePreparationInput: ( + authority: DirectSshAuthority, + reason: 'workspace-snapshot', + snapshotRevision: number + ) => Promise + prepareOnly: (input: DirectSshPreparationInput) => Promise + finalizeHydratedTerminals: (authority: DirectSshAuthority) => number +} + +export type RemoteWorkspaceTargetSync = { + syncAfterConnect: (token: DirectSshPreparationToken) => Promise + applyUnsolicitedSnapshot: ( + targetId: string, + snapshot: RemoteWorkspaceObservedSnapshot + ) => Promise + stop: () => void +} diff --git a/src/renderer/src/hooks/remote-workspace-target-sync.test.ts b/src/renderer/src/hooks/remote-workspace-target-sync.test.ts index b3242077052..bc0082c03ce 100644 --- a/src/renderer/src/hooks/remote-workspace-target-sync.test.ts +++ b/src/renderer/src/hooks/remote-workspace-target-sync.test.ts @@ -457,6 +457,63 @@ describe('createRemoteWorkspaceTargetSync', () => { expect(clearRemoteWorkspaceHydrated).toHaveBeenCalledWith('target-a') }) + it('re-pulls a conflicted target once the catalog can place the host tabs it dropped', async () => { + vi.useFakeTimers() + const hydrateTabsSession = vi.fn() + const markRemoteWorkspaceHydrated = vi.fn() + const state = appState({ + worktreesByRepo: {}, + hydrateTabsSession, + markRemoteWorkspaceHydrated + }) + const incoming = snapshot(12, { + '/remote/work': [ + { + id: 'host-tab', + worktreePath: '/remote/work', + ptyId: 'ssh:target-a@@pty-1' + } as RemoteWorkspaceSnapshot['session']['tabsByWorktreePath'][string][number] + ] + }) + const get = vi.fn(async () => incoming) + const harness = createHarness(state, get) + try { + const pending = harness.sync.applyUnsolicitedSnapshot('target-a', incoming) + await flush() + // The bounded in-apply wait expires with the host's path still unplaceable. + await vi.advanceTimersByTimeAsync(10_000) + await pending + expect( + markRemoteWorkspaceHydrated, + 'adopting none of the host tabs is not the host picture' + ).not.toHaveBeenCalled() + expect( + get, + 'nothing should re-pull while the path is still unplaceable' + ).not.toHaveBeenCalled() + + // Minutes later the user opens the worktree and its catalog row finally lands. + state.worktreesByRepo = appState().worktreesByRepo + harness.publishState() + await vi.advanceTimersByTimeAsync(0) + await flush() + await vi.advanceTimersByTimeAsync(0) + + expect(get).toHaveBeenCalledWith({ targetId: 'target-a' }) + expect( + hydrateTabsSession.mock.calls + .at(-1)?.[0] + .tabsByWorktree['repo-a::/remote/work'].map((tab: { id: string }) => tab.id), + 'the host tab dropped by the cold-catalog pull was never hydrated' + ).toEqual(['host-tab']) + // Hydration is what lifts the upload suppression, so the host ledger stops going stale too. + expect(markRemoteWorkspaceHydrated).toHaveBeenCalledWith('target-a') + } finally { + harness.sync.stop() + vi.useRealTimers() + } + }) + it('keeps only the latest placement waiter and fences a burst to the newest snapshot', async () => { vi.useFakeTimers() const hydrateTabsSession = vi.fn() diff --git a/src/renderer/src/hooks/remote-workspace-target-sync.ts b/src/renderer/src/hooks/remote-workspace-target-sync.ts index a6c2a1a3e77..c3d99224dbc 100644 --- a/src/renderer/src/hooks/remote-workspace-target-sync.ts +++ b/src/renderer/src/hooks/remote-workspace-target-sync.ts @@ -1,78 +1,40 @@ -import type { StoreApi } from 'zustand' -import type { - RemoteWorkspaceObservedPatchResult, - RemoteWorkspaceObservedSnapshot -} from '../../../shared/remote-workspace-types' -import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' +import type { RemoteWorkspaceObservedSnapshot } from '../../../shared/remote-workspace-types' import type { DirectSshAuthority } from '../../../shared/ssh-types' import { translate } from '@/i18n/i18n' import { buildWorkspaceSessionPayload } from '../lib/workspace-session' -import type { AppState } from '../store/types' import type { - DirectSshPreparationInput, - DirectSshPreparationOutcome, DirectSshPreparationToken, DirectSshSnapshotApplyToken } from './direct-ssh-reconnect-coordinator' import { buildDirectSshSnapshotApplyToken } from './direct-ssh-reconnect-coordinator' -import { resolveDirectSshTargetScope } from '../lib/direct-ssh-target-scope' +import { resolveExactDirectSshTargetWorktreeIds } from './remote-workspace-snapshot-placement' import { applyDirectSshRemoteWorkspaceSnapshot } from './remote-workspace-snapshot-apply' import { createRemoteWorkspaceSnapshotArrivalCoordinator } from './remote-workspace-snapshot-arrival-coordinator' +import { createDeferredSnapshotPlacementRetries } from './remote-workspace-deferred-placement-retry' import { applyRemoteWorkspacePushStatus } from './remote-workspace-push-status' import { waitForRemoteWorkspaceSessionReady } from './remote-workspace-session-readiness' +import type { + RemoteWorkspaceTargetSync, + RemoteWorkspaceTargetSyncDeps +} from './remote-workspace-target-sync-types' + +export type { + RemoteWorkspaceTargetSync, + RemoteWorkspaceTargetSyncDeps +} from './remote-workspace-target-sync-types' const MAX_SNAPSHOT_APPLY_ATTEMPTS = 3 -type RemoteWorkspaceApi = { - get: (args: { targetId: string }) => Promise - setForConnectedTargets: (args: { - session?: WorkspaceSessionState - hydratedTargetIds?: string[] - expectedRevisionsByTargetId: Record - expectedHostObservationTokensByTargetId: Record - }) => Promise<{ targetId: string; result: RemoteWorkspaceObservedPatchResult }[]> -} - -export type RemoteWorkspaceTargetSyncDeps = { - store: Pick, 'getState'> & Partial, 'subscribe'>> - remoteWorkspace: RemoteWorkspaceApi - getCurrentAuthority: (targetId: string) => DirectSshAuthority | null - isPreparationTokenCurrent: (token: DirectSshPreparationToken) => boolean - capturePreparationInput: ( - authority: DirectSshAuthority, - reason: 'workspace-snapshot', - snapshotRevision: number - ) => Promise - prepareOnly: (input: DirectSshPreparationInput) => Promise - finalizeHydratedTerminals: (authority: DirectSshAuthority) => number -} - -export type RemoteWorkspaceTargetSync = { - syncAfterConnect: (token: DirectSshPreparationToken) => Promise - applyUnsolicitedSnapshot: ( - targetId: string, - snapshot: RemoteWorkspaceObservedSnapshot - ) => Promise - stop: () => void -} - -function exactTargetWorktreeIds(state: AppState, authority: DirectSshAuthority): Set { - return resolveDirectSshTargetScope({ - targetId: authority.targetId, - catalogRevision: 0, - repos: state.repos, - worktreesByRepo: state.worktreesByRepo, - detectedWorktreesByRepo: state.detectedWorktreesByRepo, - folderWorkspaces: state.folderWorkspaces, - projectGroups: state.projectGroups, - restoredRuntimeHostIdByWorkspaceSessionKey: state.restoredRuntimeHostIdByWorkspaceSessionKey - }).gitWorktreeIds -} - export function createRemoteWorkspaceTargetSync( deps: RemoteWorkspaceTargetSyncDeps ): RemoteWorkspaceTargetSync { const arrivals = createRemoteWorkspaceSnapshotArrivalCoordinator() + const deferredPlacementRetries = createDeferredSnapshotPlacementRetries({ + store: deps.store, + getCurrentAuthority: deps.getCurrentAuthority, + getSnapshot: (targetId) => deps.remoteWorkspace.get({ targetId }), + applySnapshot: (targetId, snapshot) => applyUnsolicitedSnapshot(targetId, snapshot) + }) const isArrivalCurrent = arrivals.isCurrent @@ -104,6 +66,7 @@ export function createRemoteWorkspaceTargetSync( ): Promise => { let applyToken = initialToken for (let attempt = 0; attempt < MAX_SNAPSHOT_APPLY_ATTEMPTS; attempt += 1) { + let unplacedTabWorktreePaths: readonly string[] = [] const result = await applyDirectSshRemoteWorkspaceSnapshot({ store: deps.store, snapshot, @@ -114,8 +77,14 @@ export function createRemoteWorkspaceTargetSync( isPreparationTokenCurrent: deps.isPreparationTokenCurrent, waitForWorkspaceSessionReady: (signal) => waitForRemoteWorkspaceSessionReady(deps.store, signal), - finalizeHydratedTerminals: deps.finalizeHydratedTerminals + finalizeHydratedTerminals: deps.finalizeHydratedTerminals, + onUnplacedTabWorktreePaths: (worktreePaths) => { + unplacedTabWorktreePaths = worktreePaths + } }) + if (result === 'applied') { + deferredPlacementRetries.watch(authority, unplacedTabWorktreePaths) + } if (result !== 'stale' || !isArrivalCurrent(authority.targetId, arrival)) { return } @@ -172,7 +141,7 @@ export function createRemoteWorkspaceTargetSync( return } const stateBeforeGet = deps.store.getState() - const worktreeIds = exactTargetWorktreeIds(stateBeforeGet, authority) + const worktreeIds = resolveExactDirectSshTargetWorktreeIds(stateBeforeGet, authority) const hasLocalTabs = [...worktreeIds].some( (worktreeId) => (stateBeforeGet.tabsByWorktree[worktreeId] ?? []).length > 0 ) @@ -307,6 +276,7 @@ export function createRemoteWorkspaceTargetSync( syncAfterConnect, applyUnsolicitedSnapshot, stop: () => { + deferredPlacementRetries.stop() arrivals.stop() } } diff --git a/src/renderer/src/runtime/web-session-terminal-orphan-recovery-inventory.ts b/src/renderer/src/runtime/web-session-terminal-orphan-recovery-inventory.ts index f314dfb19d1..422a741aec8 100644 --- a/src/renderer/src/runtime/web-session-terminal-orphan-recovery-inventory.ts +++ b/src/renderer/src/runtime/web-session-terminal-orphan-recovery-inventory.ts @@ -202,7 +202,15 @@ export async function resolveTerminalOrphanInventory(args: { ) { disposition = 'retain' } else if (surface.pending && terminal.ptyId !== surface.expectedPtyId) { - disposition = 'remove' + // Rebind, never retire. `expectedPtyId` is the ptyId of the *snapshot frame's* pending row; + // `terminal.ptyId` is the answer to a `terminal.list` with `requireFreshPtyLiveness: true`, so + // the host has just attested this handle is live under a different PTY. A PTY id changing + // across a host relaunch is the normal case, not evidence of death (#11495). Retaining emits a + // ready row bound to the host's handle, which is the rebind — the handle is the identity, the + // ptyId behind it is the host's business. Removal here needs what the two branches around it + // already require: an explicit `retiredTerminalSurfaces` entry, or two authoritative + // inventories omitting the identity. See docs/reference/ssh-execution-boundary.md. + disposition = 'retain' } else if (!hasStrongOrphanIdentity(terminal, surface, snapshot.worktree)) { disposition = 'retain' } else { diff --git a/src/renderer/src/runtime/web-session-terminal-orphan-recovery-regressions.test.ts b/src/renderer/src/runtime/web-session-terminal-orphan-recovery-regressions.test.ts index 9e995de8776..5ff58f9913a 100644 --- a/src/renderer/src/runtime/web-session-terminal-orphan-recovery-regressions.test.ts +++ b/src/renderer/src/runtime/web-session-terminal-orphan-recovery-regressions.test.ts @@ -23,7 +23,16 @@ describe('web session terminal orphan recovery regressions', () => { const ROOTLESS_SOLE_MAP_LEAF = '22222222-2222-4222-8222-222222222222' const ROOTLESS_OFF_TREE_LEAF = '33333333-3333-4333-8333-333333333333' - it('removes only a PTY-mismatched leaf while retaining an unresolved sibling and other tabs', async () => { + // Retargeted (#11495). This previously asserted the mismatched leaf was REMOVED, and that was + // deliberate — it kept a leaf whose pending row named a ptyId the host no longer reported from + // lingering. But `terminal.list` ran with `requireFreshPtyLiveness: true` and answered with the + // handle live under `pty-replacement`, so the only thing the old assertion proved was that the + // host had relaunched the PTY. Stale-leaf accumulation is already covered by the two host-attested + // branches in the same file (`retiredTerminalSurfaces` proof, and two authoritative inventories + // omitting the identity), which is why this branch is the outlier. Removing on it is destructive: + // shouldReplaceTerminalTab rebuilds the whole mirror from one frame, so a dropped leaf takes + // ptyIdsByTabId and terminalLayoutsByTabId with it — the only surviving record of how to rebind. + it('rebinds a PTY-mismatched leaf to the handle the host still reports live', async () => { const worktree = 'repo::mismatch' const leaves = [ { leafId: 'leaf-bad', handle: 'term-bad' }, @@ -70,6 +79,12 @@ describe('web session terminal orphan recovery regressions', () => { expect(recovered?.tabs).toEqual([ browser, + expect.objectContaining({ + parentTabId: tabId, + leafId: 'leaf-bad', + status: 'ready', + terminal: 'term-bad' + }), expect.objectContaining({ parentTabId: tabId, leafId: 'leaf-hold', diff --git a/src/renderer/src/runtime/web-session-terminal-pending-handle-recovery.test.ts b/src/renderer/src/runtime/web-session-terminal-pending-handle-recovery.test.ts index 48bd556e958..1411200b14c 100644 --- a/src/renderer/src/runtime/web-session-terminal-pending-handle-recovery.test.ts +++ b/src/renderer/src/runtime/web-session-terminal-pending-handle-recovery.test.ts @@ -564,7 +564,13 @@ describe('web session pending terminal handle recovery', () => { ) }) - it('quarantines a cached handle that now names a different PTY', async () => { + // Retargeted (#11495): this pinned the quarantine, and the quarantine was the bug. The listing + // ran with `requireFreshPtyLiveness: true` and came back `orphaned: true` under a replacement + // PTY, which is a host attestation that the handle is LIVE — a PTY id rotating across a host + // relaunch is the normal case, not evidence of death. The row stays bound to the handle, which is + // the identity the host answers on; the stale `ptyId` on the carried-over row settles on the next + // frame. See web-session-terminal-orphan-recovery-inventory.ts. + it('rebinds a cached handle the host now serves from a different PTY', async () => { const snapshot = pendingSnapshot() const call = vi.fn(async () => ({ ok: true as const, @@ -589,7 +595,18 @@ describe('web session pending terminal handle recovery', () => { ENVIRONMENT_ID, { call: call as never } ) - ).resolves.toEqual(expect.objectContaining({ tabs: [] })) + ).resolves.toEqual( + expect.objectContaining({ + tabs: [ + expect.objectContaining({ + parentTabId: HOST_TAB_ID, + leafId: LEAF_ID, + status: 'ready', + terminal: TERMINAL_HANDLE + }) + ] + }) + ) expect(call).toHaveBeenCalledOnce() }) diff --git a/src/shared/remote-workspace-session-projection.test.ts b/src/shared/remote-workspace-session-projection.test.ts index a22f4123d25..fdccd75b8e9 100644 --- a/src/shared/remote-workspace-session-projection.test.ts +++ b/src/shared/remote-workspace-session-projection.test.ts @@ -200,6 +200,72 @@ describe('remote workspace session projection', () => { expect(Object.keys(session.tabsByWorktree)).toEqual(['repo-b::/srv/app']) }) + it('unions two local repo rows that collapse onto one host path instead of clobbering', () => { + // Duplicate repo rows for one remote checkout are the normal state while a host catalog + // reconciles. `worktreePathFromId` drops the repoId, so both keys project onto '/srv/app'; a + // freshly created empty row iterating last used to publish an empty tab list for a workspace + // the user had panes open in, and the upload is a wholesale replace-session (#15484). + const session = { + ...getDefaultWorkspaceSession(), + activeWorktreeId: 'repo-old::/srv/app', + activeTabId: 'tab-live', + tabsByWorktree: { + 'repo-old::/srv/app': [ + { + id: 'tab-live', + ptyId: 'pty-live', + worktreeId: 'repo-old::/srv/app', + title: 'Agent', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + ], + 'repo-new::/srv/app': [] + }, + activeTabIdByWorktree: { + 'repo-old::/srv/app': 'tab-live', + 'repo-new::/srv/app': null + }, + remoteSessionIdsByTabId: { 'tab-live': 'pty-live' } + } + + const projected = exportRemoteWorkspaceSession(session, { + isTargetWorktree: () => true + }) + + expect( + projected.tabsByWorktreePath['/srv/app']?.map((tab) => tab.id), + 'the empty twin row erased a live pane from the host ledger' + ).toEqual(['tab-live']) + expect(projected.activeTabIdByWorktreePath?.['/srv/app']).toBe('tab-live') + expect(projected.remoteSessionIdsByTabId).toEqual({ 'tab-live': 'pty-live' }) + }) + + it('keeps one row per tab id when colliding local keys share a tab', () => { + const tab = { + id: 'tab-shared', + ptyId: 'pty-shared', + title: 'Agent', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } + const session = { + ...getDefaultWorkspaceSession(), + tabsByWorktree: { + 'repo-old::/srv/app': [{ ...tab, worktreeId: 'repo-old::/srv/app' }], + 'repo-new::/srv/app': [{ ...tab, worktreeId: 'repo-new::/srv/app' }] + } + } + + const projected = exportRemoteWorkspaceSession(session, { isTargetWorktree: () => true }) + + expect(projected.tabsByWorktreePath['/srv/app']?.map((row) => row.id)).toEqual(['tab-shared']) + }) + it('does not report unplaced tabs when every host path resolves', () => { const unplaced: string[] = [] diff --git a/src/shared/remote-workspace-session-projection.ts b/src/shared/remote-workspace-session-projection.ts index 0b0066c1ed0..7f923050d36 100644 --- a/src/shared/remote-workspace-session-projection.ts +++ b/src/shared/remote-workspace-session-projection.ts @@ -59,10 +59,25 @@ export function exportRemoteWorkspaceSession( if (!worktreePath) { continue } - tabsByWorktreePath[worktreePath] = tabs.map((tab) => { + // Why union rather than assignment: `worktreePathFromId` drops the repoId, so two local keys + // for one host path — duplicate repo rows for the same remote checkout, which is the normal + // state while a host catalog reconciles — collapse onto one entry here. Assignment let + // whichever key came last win outright, and an empty twin published an empty tab list for a + // workspace the user had panes open in (#15484). This projection is uploaded as a wholesale + // replace-session, so a clobbered entry deletes those tabs from the host snapshot. The host has + // one workspace at that path, so the union deduped by tab id is the only lossless answer. Same + // collision the `Math.max` below folds for `lastVisitedAtByWorktreePath`. + const merged = tabsByWorktreePath[worktreePath] ?? [] + const alreadyProjected = new Set(merged.map((tab) => tab.id)) + for (const tab of tabs) { + if (alreadyProjected.has(tab.id)) { + continue + } + alreadyProjected.add(tab.id) terminalTabIds.add(tab.id) - return tabToRemote(tab, worktreePath) - }) + merged.push(tabToRemote(tab, worktreePath)) + } + tabsByWorktreePath[worktreePath] = merged } const activeWorktreePath = @@ -80,7 +95,11 @@ export function exportRemoteWorkspaceSession( } const worktreePath = worktreePathFromId(worktreeId) if (worktreePath) { - activeTabIdByWorktreePath[worktreePath] = tabId && terminalTabIds.has(tabId) ? tabId : null + // Same path collision as the tab lists above: a colliding key's null must not erase the + // active tab the other key named. + const resolved = tabId && terminalTabIds.has(tabId) ? tabId : null + activeTabIdByWorktreePath[worktreePath] = + resolved ?? activeTabIdByWorktreePath[worktreePath] ?? null } }