From 3e7e49d77b611e2b19f5586d787f48c8bb62d897 Mon Sep 17 00:00:00 2001 From: Lesley Murfin Date: Mon, 21 Sep 2026 11:17:19 -0600 Subject: [PATCH 01/23] test(GAP-03): red tests for offline-client tab resurrection on reconnect Reproduces the additive-merge bug in adoptStrandedHostPartitionSession (src/shared/workspace-session-stranded-partition-adoption.ts): when a worktree's tabs are all closed server-side while a client is offline, the client's base tabsByWorktree row for that worktree becomes empty, so workspacesTheBaseOwns() no longer protects it and the stale cached ssh:* partition's closed tabs are unconditionally re-adopted into the merged session on reconnect. - RED: closed tabs must not reappear after reconnect (fails today) - GREEN: a genuinely offline-created tab (never synced) must survive reconnect -- the #12721 non-regression constraint the downstream fix must not break Fixes stablyai/orca#22038. No fix code included; implementation of reconcileLocalTabsWithServerManifest()/hasPendingReconnectionHandshake is a separate downstream task. --- ...ace-session-host-offline-reconnect.test.ts | 109 ++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 src/renderer/src/lib/workspace-session-host-offline-reconnect.test.ts diff --git a/src/renderer/src/lib/workspace-session-host-offline-reconnect.test.ts b/src/renderer/src/lib/workspace-session-host-offline-reconnect.test.ts new file mode 100644 index 000000000000..674daf85f349 --- /dev/null +++ b/src/renderer/src/lib/workspace-session-host-offline-reconnect.test.ts @@ -0,0 +1,109 @@ +/** + * GAP-03: offline paired clients retain stale tabs in their cached host partition and re-inject + * them into the merged session on reconnect. + * + * `adoptStrandedHostPartitionSession` (src/shared/workspace-session-stranded-partition-adoption.ts) + * treats a `base` worktree as "owned" — and therefore protected from adoption — only when it holds + * a NON-EMPTY `tabsByWorktree` row (`workspacesTheBaseOwns`). When every tab for a worktree has + * been closed server-side while a client was disconnected, the base's row for that worktree goes + * to empty/absent, so it is no longer "owned". The stranded/cached host partition — the client's + * stale on-disk mirror of a runtime/ssh host session from before it went offline — still carries + * the closed tabs, and `adoptStrandedHostPartitionSession` unconditionally folds them back into the + * merged session, re-injecting tabs the user (or another client) explicitly closed while this + * client was away. That merged session is then the payload the next + * `persistWorkspaceSessionByHost`/`patchWorkspaceSessionByHost` call round-trips back to the host + * partition and broadcasts to every other connected client — the "ghost tab" resurrection reported + * in revive_labs#962 and filed upstream as stablyai/orca#22038. + * + * Contrast with the sibling `workspace-session-ssh-partition-ownership.test.ts` suite: those tests + * pin the #12721 protection (an empty PARTITION row is not proof of absence, and a base's own + * unsaved draft must survive when the OTHER side is empty). This suite pins the opposite gap: an + * empty BASE row is treated as "nothing to protect", so a stale but non-empty host row wins even + * though it names tabs the base has no record of intentionally reopening. + */ +import { describe, expect, it } from 'vitest' +import { getDefaultWorkspaceSession } from '../../../shared/constants' +import type { ExecutionHostId } from '../../../shared/execution-host' +import { normalizeExecutionHostId } from '../../../shared/execution-host' +import type { TerminalTab } from '../../../shared/terminal-tab-types' +import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' +import { fetchWorkspaceSessionWithRuntimeHostOwners } from './workspace-session-host-hydration' + +const TARGET_ID = 'target-1' +const SSH_HOST_ID: ExecutionHostId = `ssh:${TARGET_ID}` +const REPO_ID = 'repo-remote' +const WORKTREE_ID = `${REPO_ID}::/remote/checkout` + +function session(overrides: Partial): WorkspaceSessionState { + return { ...getDefaultWorkspaceSession(), ...overrides } +} + +function tab(id: string, worktreeId: string): TerminalTab { + return { + id, + ptyId: `pty-${id}`, + worktreeId, + title: id, + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } +} + +/** A session read whose partitions are exactly what persistence holds, plus its census. */ +function partitionedApi(partitions: Partial>) { + return { + get: async (hostId?: ExecutionHostId) => + partitions[hostId ?? 'local'] ?? getDefaultWorkspaceSession(), + listHostIds: async () => + Object.keys(partitions).flatMap((hostId) => normalizeExecutionHostId(hostId) ?? []) + } +} + +describe('GAP-03: offline client reconnect must not resurrect server-closed tabs', () => { + it('does not re-adopt a tab that was closed on the host while this client was offline', async () => { + // The client's LOCAL read reflects current truth: the worktree's tabs were all closed while + // this client was disconnected, so its `tabsByWorktree` row is now empty/absent. + // The `ssh:` partition is this client's STALE on-disk cache from before it went + // offline: it still names the two now-closed tabs. + const read = await fetchWorkspaceSessionWithRuntimeHostOwners( + partitionedApi({ + local: session({}), + [SSH_HOST_ID]: session({ + tabsByWorktree: { + [WORKTREE_ID]: [tab('tab-2', WORKTREE_ID), tab('tab-3', WORKTREE_ID)] + } + }) + }), + [{ id: REPO_ID, connectionId: TARGET_ID, executionHostId: SSH_HOST_ID }] + ) + + // Expected: reconnect purges the stale cached tabs instead of reinjecting them into the + // merged session (and, downstream, back into the host partition and every other client). + expect(read.session.tabsByWorktree[WORKTREE_ID] ?? []).toEqual([]) + }) +}) + +describe('GAP-03 non-regression: #12721 offline-created local work must survive reconnect', () => { + it('keeps a genuinely offline-created tab that was never synced to the host partition', async () => { + // This client created a tab entirely offline — the host partition (mirroring the last state + // synced from the server before disconnect) has nothing for this worktree at all, because the + // tab never existed server-side. `hostHasNothingFor` / the empty-host-row rule from #12721 is + // exactly what must keep protecting this: an empty host row is never adopted, so it cannot + // out-rank the base's real, populated row. + const read = await fetchWorkspaceSessionWithRuntimeHostOwners( + partitionedApi({ + local: session({ + tabsByWorktree: { [WORKTREE_ID]: [tab('tab-offline-draft', WORKTREE_ID)] } + }), + [SSH_HOST_ID]: session({}) + }), + [{ id: REPO_ID, connectionId: TARGET_ID, executionHostId: SSH_HOST_ID }] + ) + + expect(read.session.tabsByWorktree[WORKTREE_ID]?.map((entry) => entry.id)).toEqual([ + 'tab-offline-draft' + ]) + }) +}) From 932bf33cb3f14bd5e757f14b32f72dfca3e5e748 Mon Sep 17 00:00:00 2001 From: Lesley Murfin Date: Mon, 21 Sep 2026 11:51:07 -0600 Subject: [PATCH 02/23] fix(GAP-03): stop reconnect from re-adopting server-closed tabs adoptStrandedHostPartitionSession's workspacesTheBaseOwns() only ever protected a worktree from stranded-partition adoption when the base held a non-empty tabsByWorktree row. Once every tab for a worktree closed server-side while a client was offline, the base's row went empty/absent, so the repair no longer protected it and unconditionally folded in whatever tabs were still sitting in the client's stale on-disk ssh:/runtime cache from before it disconnected -- resurrecting closed tabs on reconnect and round-tripping them back out via persistWorkspaceSessionByHost to the host partition and every other connected client (stablyai/orca#22038, revive_labs#962). Fix: sshPartitionCatalogAttribution (extracted into a new workspace-session-host-reconciliation.ts module alongside unownedSessionKeys, to keep workspace-session-host-hydration.ts under this repo's 300-line file budget) now also tracks, per SSH host, the worktree ids the repo catalog positively and *currently* confirms belong to exactly that host -- via the row's own executionHostId field, not a bare connectionId fallback. That fallback already treats a legacy catalog row as resolved, which is exactly the pre-#12723 shape the one-shot gap-fill repair exists for, so it stays on the unconditional path. adoptStrandedHostPartitionSession takes the confirmed set as reconciledWorktreeIds: for one of these ids, an absent tabsByWorktree row on the base is no longer read as an unverifiable #12721 gap -- it's the server's current truth of zero tabs -- so a host row that still names real (non-empty) tabs is declined rather than adopted. An id the catalog also marks foreign for another host (residue proving a genuine migration) is exempted, so a freshly-moved connection's real tabs still adopt normally. Declining never destroys anything: the id simply drops out of adoptedWorkspaceIds, so partitionRowsTheWriteWontReturn parks the stale row in the write-side shadow the same way a contested id already does. One regression fixture (workspace-session-ssh-partition-ownership.test .ts's "keeps a declined row..." case) gave its sibling worktree a bare `local: session({})` with no tabsByWorktree entry at all for the sibling -- structurally identical to the GAP-03 bug shape itself. Gave it the same known-worktree explicit-empty-row shape already used throughout the round-trip suite (`strandedPartitions()`), which is what that test's own "sibling is adopted and routes back" premise actually depends on. Tests: - workspace-session-host-offline-reconnect.test.ts: RED -> GREEN (closed tab no longer resurrected); #12721-safety scenario (offline- created, never-synced tab) stays GREEN. - workspace-session-ssh-partition-ownership.test.ts, workspace-session-ssh-partition-round-trip.test.ts, workspace-session-host-split.test.ts, workspace-session-partition-owner.test.ts, ssh-host-partition-session-export.test.ts: all still GREEN, zero regressions. - Also re-ran workspace-session-host-contention.test.ts, workspace-session-host-persistence.test.ts and main/ipc/remote-workspace.test.ts (other direct callers/dependents of the touched functions): all green. - 115/115 across all 9 files above. No PR opened; stops at a locally-ready, committed branch per this slice's constraints. --- .../lib/workspace-session-host-hydration.ts | 135 +----------- .../workspace-session-host-reconciliation.ts | 200 ++++++++++++++++++ ...ce-session-ssh-partition-ownership.test.ts | 6 +- ...ace-session-stranded-partition-adoption.ts | 32 +++ 4 files changed, 245 insertions(+), 128 deletions(-) create mode 100644 src/renderer/src/lib/workspace-session-host-reconciliation.ts diff --git a/src/renderer/src/lib/workspace-session-host-hydration.ts b/src/renderer/src/lib/workspace-session-host-hydration.ts index 9852dd563859..4138760da1f6 100644 --- a/src/renderer/src/lib/workspace-session-host-hydration.ts +++ b/src/renderer/src/lib/workspace-session-host-hydration.ts @@ -6,21 +6,19 @@ import { parseExecutionHostId, type ExecutionHostId } from '../../../shared/execution-host' -import { normalizeWorkspaceSessionKeyToWorkspaceId } from '../../../shared/workspace-scope' -import { getRepoIdFromWorktreeId } from '../../../shared/worktree/id' -import { - createRepoRowExecutionHostLookup, - resolveWorktreeExecutionHost -} from '../../../shared/worktree-execution-host-resolution' import { adoptStrandedHostPartitionSession, - partitionRowsTheWriteWontReturn, - workspaceIdsNamedByPartition + partitionRowsTheWriteWontReturn } from '../../../shared/workspace-session-stranded-partition-adoption' import { mergeWorkspaceSessionsWithHostShadow, normalizeWorkspaceSessionKeyToWorktreeId } from './workspace-session-host-contention' +import { + reconciledWorktreeIdsForHost, + sshPartitionCatalogAttribution, + unownedSessionKeys +} from './workspace-session-host-reconciliation' import { nonLocalHostSessionEntries, type HostSessionSlices } from './workspace-session-host-split' type SessionReadApi = { @@ -227,7 +225,8 @@ export async function fetchWorkspaceSessionWithRuntimeHostOwners( session, attribution.contestedSessionKeys, attribution.foreignSessionKeysByHostId.get(hostId) - ) + ), + reconciledWorktreeIds: reconciledWorktreeIdsForHost(attribution, hostId) }) session = adoption.session if (slice) { @@ -270,121 +269,3 @@ async function listPersistedSshPartitionHostIds(api: SessionReadApi): Promise`; that pair IS the - * shape the repair exists for, and reading it as a collision disables the repair: - * - the local/runtime rivalry the contention split already arbitrated; - * - two SSH partitions both naming the id, which that split never sees; - * - a repo id the catalog registers on more than one host. - * - * `foreign` — the catalog positively resolves the id to a different host. That is residue, not a - * rival claim: adopting it would show a stale row in front of the live one and then route it into - * the live partition. The same "positively says otherwise" rule `catalogReattributedAwayFrom` uses - * — an id the catalog cannot speak for is neither contested nor foreign, so a boot whose repos have - * not hydrated still reunites its rows. - */ -function sshPartitionCatalogAttribution( - repos: readonly Pick[], - sshPartitions: readonly (readonly [ExecutionHostId, WorkspaceSessionState | null])[], - mergedContested: ReadonlySet -) { - const contestedSessionKeys = new Set(mergedContested) - const foreignSessionKeysByHostId = new Map>() - // Ids the catalog resolves to the ssh partition that names them. - const ownedSessionKeysByHostId = new Map>() - const repoLookup = createRepoRowExecutionHostLookup(repos) - const ownedByHostId = new Map>() - for (const [hostId, slice] of sshPartitions) { - if (!slice) { - continue - } - const foreign = new Set() - const owned = new Set() - const catalogOwned = new Set() - for (const workspaceId of workspaceIdsNamedByPartition(slice)) { - // A folder key carries no repo id at all, and `getRepoIdFromWorktreeId` hands back the whole - // key rather than nothing, so the catalog would be asked about `folder:` and answer - // `unknown`. Right verdict, wasted resolution; skip it by shape instead. - const resolution = isWorktreeSessionKey(workspaceId) - ? resolveWorktreeExecutionHost(repoLookup, { - repoId: getRepoIdFromWorktreeId(workspaceId), - hostId: null - }) - : null - if (resolution?.kind === 'resolved' && resolution.hostId !== hostId) { - foreign.add(workspaceId) - continue - } - if (resolution?.kind === 'resolved') { - catalogOwned.add(workspaceId) - } else if (resolution?.reason === 'ambiguous') { - contestedSessionKeys.add(workspaceId) - } - owned.add(workspaceId) - } - if (foreign.size > 0) { - foreignSessionKeysByHostId.set(hostId, foreign) - } - ownedByHostId.set(hostId, owned) - ownedSessionKeysByHostId.set(hostId, catalogOwned) - } - // Why co-presence is asked only of the ids left after the catalog has spoken: one partition - // holding residue the catalog attributes elsewhere is a single owner plus a leftover, not a - // collision. Counting the leftover would withhold the real owner's rows from the write. - for (const workspaceId of sessionKeysHeldByMultiplePartitionSets([...ownedByHostId.values()])) { - contestedSessionKeys.add(workspaceId) - } - return { contestedSessionKeys, foreignSessionKeysByHostId, ownedSessionKeysByHostId } -} - -/** Ids that appear in more than one of these per-partition sets. */ -function sessionKeysHeldByMultiplePartitionSets(sets: readonly ReadonlySet[]): Set { - const held = new Set() - const contested = new Set() - for (const keys of sets) { - for (const key of keys) { - if (held.has(key)) { - contested.add(key) - } - held.add(key) - } - } - return contested -} - -/** - * Keys this partition must not contribute: the ones the catalog gave to another host, plus a - * contested id the assembled session holds no row for at all. - * - * Why the second class: a contested id is withheld from the read-source override, so the write - * re-derives an owner — and for an id the catalog cannot name, that answer is 'local'. Adopting - * such a row would move it out of the partition that owns it and into the blob, which is the - * two-store split this whole change removes. Gap-filling stays available for a contested id the - * session already has a row for, because that row's own partition is what the write follows. - * Declining to adopt leaks a row into invisibility for one boot; it never deletes one. - */ -function unownedSessionKeys( - session: WorkspaceSessionState, - contestedSessionKeys: ReadonlySet, - foreignSessionKeys: ReadonlySet | undefined -): ReadonlySet { - if (contestedSessionKeys.size === 0) { - return foreignSessionKeys ?? new Set() - } - const unowned = new Set(foreignSessionKeys ?? []) - const known = workspaceIdsNamedByPartition(session) - for (const key of contestedSessionKeys) { - if (!known.has(normalizeWorkspaceSessionKeyToWorkspaceId(key))) { - unowned.add(key) - } - } - return unowned -} - -/** A worktree session key names its repo before `::`; a folder key names no repo at all. */ -function isWorktreeSessionKey(workspaceId: string): boolean { - return workspaceId.includes('::') -} diff --git a/src/renderer/src/lib/workspace-session-host-reconciliation.ts b/src/renderer/src/lib/workspace-session-host-reconciliation.ts new file mode 100644 index 000000000000..43bf4cfcb023 --- /dev/null +++ b/src/renderer/src/lib/workspace-session-host-reconciliation.ts @@ -0,0 +1,200 @@ +import type { Repo } from '../../../shared/repo-types' +import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' +import { normalizeExecutionHostId, type ExecutionHostId } from '../../../shared/execution-host' +import { normalizeWorkspaceSessionKeyToWorkspaceId } from '../../../shared/workspace-scope' +import { getRepoIdFromWorktreeId } from '../../../shared/worktree/id' +import { + createRepoRowExecutionHostLookup, + resolveWorktreeExecutionHost +} from '../../../shared/worktree-execution-host-resolution' +import { workspaceIdsNamedByPartition } from '../../../shared/workspace-session-stranded-partition-adoption' + +export type SshPartitionCatalogAttribution = { + contestedSessionKeys: Set + foreignSessionKeysByHostId: Map> + /** Ids the catalog resolves to that host (incl. bare connectionId fallback). Supersedes base + * residue so the host's live tabs win (#23390 / #25616). */ + ownedSessionKeysByHostId: Map> + /** Ids the catalog positively, currently resolves to exactly that host — the GAP-03 signal + * `reconciledWorktreeIdsForHost` is built from once migration residue elsewhere is ruled out. */ + confirmedSessionKeysByHostId: Map> +} + +/** A worktree session key names its repo before `::`; a folder key names no repo at all. */ +function isWorktreeSessionKey(workspaceId: string): boolean { + return workspaceId.includes('::') +} + +/** Ids that appear in more than one of these per-partition sets. */ +function sessionKeysHeldByMultiplePartitionSets(sets: readonly ReadonlySet[]): Set { + const held = new Set() + const contested = new Set() + for (const keys of sets) { + for (const key of keys) { + if (held.has(key)) { + contested.add(key) + } + held.add(key) + } + } + return contested +} + +/** + * What the repo catalog says about the workspaces each SSH partition names. + * + * `contested` — "one workspace written twice" is false, so adoption may gap-fill but never replace. + * Three sources, none of which is bare co-presence in 'local' and `ssh:`; that pair IS the + * shape the repair exists for, and reading it as a collision disables the repair: + * - the local/runtime rivalry the contention split already arbitrated; + * - two SSH partitions both naming the id, which that split never sees; + * - a repo id the catalog registers on more than one host. + * + * `foreign` — the catalog positively resolves the id to a different host. That is residue, not a + * rival claim: adopting it would show a stale row in front of the live one and then route it into + * the live partition. The same "positively says otherwise" rule `catalogReattributedAwayFrom` uses + * — an id the catalog cannot speak for is neither contested nor foreign, so a boot whose repos have + * not hydrated still reunites its rows. + * + * `owned` — the catalog resolves the id to this host (including bare `connectionId` fallback). Used + * to supersede base residue so the host's live tabs win. + * + * `confirmed` — the catalog positively resolves the id to EXACTLY this host, via the row's own + * `executionHostId` field (not a bare `connectionId` fallback). See `reconciledWorktreeIdsForHost`. + */ +export function sshPartitionCatalogAttribution( + repos: readonly Pick[], + sshPartitions: readonly (readonly [ExecutionHostId, WorkspaceSessionState | null])[], + mergedContested: ReadonlySet +): SshPartitionCatalogAttribution { + const contestedSessionKeys = new Set(mergedContested) + const foreignSessionKeysByHostId = new Map>() + const ownedSessionKeysByHostId = new Map>() + const confirmedSessionKeysByHostId = new Map>() + const repoLookup = createRepoRowExecutionHostLookup(repos) + const ownedByHostId = new Map>() + for (const [hostId, slice] of sshPartitions) { + if (!slice) { + continue + } + const foreign = new Set() + const owned = new Set() + const catalogOwned = new Set() + const confirmed = new Set() + for (const workspaceId of workspaceIdsNamedByPartition(slice)) { + // A folder key carries no repo id at all, and `getRepoIdFromWorktreeId` hands back the whole + // key rather than nothing, so the catalog would be asked about `folder:` and answer + // `unknown`. Right verdict, wasted resolution; skip it by shape instead. + const resolution = isWorktreeSessionKey(workspaceId) + ? resolveWorktreeExecutionHost(repoLookup, { + repoId: getRepoIdFromWorktreeId(workspaceId), + hostId: null + }) + : null + if (resolution?.kind === 'resolved' && resolution.hostId !== hostId) { + foreign.add(workspaceId) + continue + } + if (resolution?.kind === 'resolved') { + catalogOwned.add(workspaceId) + } else if (resolution?.kind === 'unresolved' && resolution.reason === 'ambiguous') { + contestedSessionKeys.add(workspaceId) + } + // GAP-03's `confirmed` reading is deliberately narrower than `resolution.hostId === hostId`: + // that verdict already treats a bare `connectionId` (no `executionHostId` on the row) as + // resolved, which is exactly the pre-#12723 catalog shape this repair's "one-shot" gap-fill + // exists for. Requiring the row's OWN `executionHostId` field keeps that legacy shape on the + // unconditional path and reserves the stricter "empty base row means zero" reading for a + // catalog that has actually recorded this repo's current host. + if ( + resolution?.kind === 'resolved' && + resolution.hostId === hostId && + normalizeExecutionHostId(resolution.owner?.executionHostId ?? null) === hostId + ) { + confirmed.add(workspaceId) + } + owned.add(workspaceId) + } + if (foreign.size > 0) { + foreignSessionKeysByHostId.set(hostId, foreign) + } + ownedSessionKeysByHostId.set(hostId, catalogOwned) + if (confirmed.size > 0) { + confirmedSessionKeysByHostId.set(hostId, confirmed) + } + ownedByHostId.set(hostId, owned) + } + // Why co-presence is asked only of the ids left after the catalog has spoken: one partition + // holding residue the catalog attributes elsewhere is a single owner plus a leftover, not a + // collision. Counting the leftover would withhold the real owner's rows from the write. + for (const workspaceId of sessionKeysHeldByMultiplePartitionSets([...ownedByHostId.values()])) { + contestedSessionKeys.add(workspaceId) + } + return { + contestedSessionKeys, + foreignSessionKeysByHostId, + ownedSessionKeysByHostId, + confirmedSessionKeysByHostId + } +} + +/** + * Keys this partition must not contribute: the ones the catalog gave to another host, plus a + * contested id the assembled session holds no row for at all. + * + * Why the second class: a contested id is withheld from the read-source override, so the write + * re-derives an owner — and for an id the catalog cannot name, that answer is 'local'. Adopting + * such a row would move it out of the partition that owns it and into the blob, which is the + * two-store split this whole change removes. Gap-filling stays available for a contested id the + * session already has a row for, because that row's own partition is what the write follows. + * Declining to adopt leaks a row into invisibility for one boot; it never deletes one. + */ +export function unownedSessionKeys( + session: WorkspaceSessionState, + contestedSessionKeys: ReadonlySet, + foreignSessionKeys: ReadonlySet | undefined +): ReadonlySet { + if (contestedSessionKeys.size === 0) { + return foreignSessionKeys ?? new Set() + } + const unowned = new Set(foreignSessionKeys ?? []) + const known = workspaceIdsNamedByPartition(session) + for (const key of contestedSessionKeys) { + if (!known.has(normalizeWorkspaceSessionKeyToWorkspaceId(key))) { + unowned.add(key) + } + } + return unowned +} + +/** Ids the catalog marks foreign for ANY host: demonstrably held residue on more than one + * partition, i.e. a migration rather than a plain reconnect. `reconciledWorktreeIdsForHost` + * withholds these so a freshly-moved connection's real, live tabs still adopt normally. */ +function migratedSessionKeys(attribution: SshPartitionCatalogAttribution): Set { + const migrated = new Set() + for (const foreign of attribution.foreignSessionKeysByHostId.values()) { + for (const workspaceId of foreign) { + migrated.add(workspaceId) + } + } + return migrated +} + +/** + * GAP-03 (stablyai/orca#22038 / revive_labs#962): worktree ids to pass as + * `reconciledWorktreeIds` into `adoptStrandedHostPartitionSession` for one SSH host. + * + * Confirmed-by-catalog ids for this host, minus any id another host also holds foreign residue for + * (a migration). Callers that learn the ordinary-restart connection gate later pass that through + * a later commit; this early shape is the offline decline set itself. + */ +export function reconciledWorktreeIdsForHost( + attribution: SshPartitionCatalogAttribution, + hostId: ExecutionHostId +): Set { + const reconciled = new Set(attribution.confirmedSessionKeysByHostId.get(hostId)) + for (const workspaceId of migratedSessionKeys(attribution)) { + reconciled.delete(workspaceId) + } + return reconciled +} diff --git a/src/renderer/src/lib/workspace-session-ssh-partition-ownership.test.ts b/src/renderer/src/lib/workspace-session-ssh-partition-ownership.test.ts index 26a3545f2295..e8cc6e3a1667 100644 --- a/src/renderer/src/lib/workspace-session-ssh-partition-ownership.test.ts +++ b/src/renderer/src/lib/workspace-session-ssh-partition-ownership.test.ts @@ -279,7 +279,11 @@ describe('a bare workspace id two partitions both hold', () => { it('keeps a declined row in its own partition when a sibling workspace is written', async () => { const SIBLING_ID = 'repo-sibling::/checkout' const partitions = { - local: session({}), + // Explicit empty row: this worktree IS known locally (the #12723 renderer/runtime split, + // same shape `strandedPartitions()` uses elsewhere) — a gap to fill, not GAP-03's "never + // heard of this worktree at all" shape that `workspace-session-host-offline-reconnect.test.ts` + // exercises. + local: session({ tabsByWorktree: { [SIBLING_ID]: [] } }), [SSH_HOST_ID]: session({ tabsByWorktree: { // Contested and held by no other partition, so the read declines it... diff --git a/src/shared/workspace-session-stranded-partition-adoption.ts b/src/shared/workspace-session-stranded-partition-adoption.ts index c36d35b4bef4..8beb77ba2cf2 100644 --- a/src/shared/workspace-session-stranded-partition-adoption.ts +++ b/src/shared/workspace-session-stranded-partition-adoption.ts @@ -320,6 +320,23 @@ export type StrandedPartitionAdoptionOptions = { foreignSessionKeys?: ReadonlySet /** Keys the repo catalog attributes to this host. */ ownedSessionKeys?: ReadonlySet + /** + * Worktree ids the repo catalog positively, currently attributes to EXACTLY this host, with no + * evidence any OTHER partition ever held residue for the same id (a migration). For one of + * these, an empty tab row is not the #12721 shape — the catalog's confirmation means this is a + * live, tracked connection, so a base with literally no `tabsByWorktree` entry for it is the + * server reporting zero tabs, not an unverifiable gap. A client that went offline while every + * tab for the worktree was closed comes back with exactly this shape: base empty/absent, host + * partition still naming the closed tabs from its last connected snapshot. Blindly gap-filling + * that repaints them into the merged session and the next `persistWorkspaceSessionByHost` call + * round-trips them back out to the host partition and every other client — the ghost-tab + * resurrection in stablyai/orca#22038 / revive_labs#962 (GAP-03). + * + * Declining here still never destroys anything: the id drops out of `adoptedWorkspaceIds`, so + * `partitionRowsTheWriteWontReturn` parks the row in the write-side shadow instead, the same + * "leak, never kill" safety net a contested id already gets. + */ + reconciledWorktreeIds?: ReadonlySet } export type StrandedPartitionAdoption = { @@ -355,6 +372,21 @@ export function adoptStrandedHostPartitionSession( for (const key of options.foreignSessionKeys ?? []) { adoptable.delete(normalizeWorkspaceSessionKeyToWorkspaceId(key)) } + // GAP-03: a worktree the catalog confirms belongs to exactly this host, with no evidence it + // ever lived on another one, does not get the #12721 unverifiable-gap reading when the base + // holds no `tabsByWorktree` row for it at all. The base's silence is the server's current + // truth (zero tabs), not a blind spot — so a host row that still names real tabs here is this + // client's stale pre-disconnect cache, not evidence of anything live. Declining leaves the row + // in place for `partitionRowsTheWriteWontReturn` to park rather than deleting it. + for (const workspaceId of options.reconciledWorktreeIds ?? []) { + if (!adoptable.has(workspaceId) || Object.hasOwn(base.tabsByWorktree ?? {}, workspaceId)) { + continue + } + const hostTabs = (host.tabsByWorktree ?? {})[workspaceId] + if (Array.isArray(hostTabs) && hostTabs.length > 0) { + adoptable.delete(workspaceId) + } + } if (adoptable.size === 0) { return { session: base, adoptedWorkspaceIds: NOTHING_ADOPTED } } From 59a631d8f4aba4c451ab10afd1d0ca77c3c0671c Mon Sep 17 00:00:00 2001 From: Lesley Murfin Date: Mon, 21 Sep 2026 22:02:12 -0600 Subject: [PATCH 03/23] test(GAP-03): Docker cross-site offline-reconnect tab resurrection harness Adds a two-launch (quit+relaunch) Playwright harness that reproduces the GAP-03 repro exactly: pair a client to a Dockerized SSH host, open N tabs, quit the client, close M tabs' local base row while leaving the SSH host-partition mirror stale (simulating 'closed server-side while offline'), relaunch, and measure resurrection plus the #12721 non-regression. Modeled on run-ssh-docker-e2e.mjs (wrapper shape), golden-quit-relaunch-session.spec.ts and persisted-session-production-upgrade.spec.ts (two-launch + on-disk fixture injection), and the docker-ssh-relay-* helpers already used by ssh-lost-kill-tab-resurrection.spec.ts. Also hardens docker-ssh-relay-target.ts: some hosts' tmp directories carry a default ACL that overrides ssh-keygen's normal 0600 permissions, which sshd then refuses under StrictModes. One chmodSync call fixes it unconditionally for every SSH-relay Docker e2e spec, not just this one. BEFORE-state (pre-fix, commit 36beb65f6b) RED evidence captured separately; see revive_labs specs/22038-gap-03-docker-cross-site-test-harness.md. Slice A of the GAP-03 before/after test-battery ticket: harness + baseline only. AFTER-state measurement, Calgary-site run, and aggregate report are follow-up slices. --- ...line-reconnect-tab-resurrection-docker.mjs | 54 +++++ ...offline-reconnect-tab-resurrection.spec.ts | 227 ++++++++++++++++++ tests/e2e/helpers/docker-ssh-relay-target.ts | 5 +- 3 files changed, 285 insertions(+), 1 deletion(-) create mode 100755 config/scripts/run-offline-reconnect-tab-resurrection-docker.mjs create mode 100644 tests/e2e/gap-03-offline-reconnect-tab-resurrection.spec.ts diff --git a/config/scripts/run-offline-reconnect-tab-resurrection-docker.mjs b/config/scripts/run-offline-reconnect-tab-resurrection-docker.mjs new file mode 100755 index 000000000000..c787601e2acb --- /dev/null +++ b/config/scripts/run-offline-reconnect-tab-resurrection-docker.mjs @@ -0,0 +1,54 @@ +#!/usr/bin/env node +import { spawnSync } from 'node:child_process' + +// Runs the GAP-03 (revive_labs#962 / stablyai/orca#22038) offline-reconnect tab resurrection +// harness: a two-launch Electron test that pairs a client to a Dockerized SSH host, opens N +// terminal tabs, quits the client, closes some of those tabs "server-side" while it is offline, +// relaunches the client, and measures whether the closed tabs resurrect. +// +// Modeled directly on run-ssh-docker-e2e.mjs: same pnpm/Playwright invocation shape, same +// ORCA_E2E_SSH_DOCKER env-gate the target spec reads to skip itself outside a Docker-capable host. +// +// ORCA_GAP03_TAB_COUNT (default 4) and ORCA_GAP03_CLOSE_COUNT (default 2) tune the repro's N/M +// without editing the spec, so the same harness can drive a smaller or larger KPI run. +const rawExtraArgs = process.argv.slice(2) +const extraArgs = rawExtraArgs[0] === '--' ? rawExtraArgs.slice(1) : rawExtraArgs +const pnpm = process.platform === 'win32' ? 'pnpm.cmd' : 'pnpm' +const env = { + ...process.env, + ORCA_E2E_SSH_DOCKER: '1', + ORCA_GAP03_TAB_COUNT: process.env.ORCA_GAP03_TAB_COUNT ?? '4', + ORCA_GAP03_CLOSE_COUNT: process.env.ORCA_GAP03_CLOSE_COUNT ?? '2' +} + +// Why: Node's CVE-2024-27980 hardening rejects .cmd spawns without shell on Windows. +const spawnOptions = { + stdio: 'inherit', + env, + shell: process.platform === 'win32' +} + +const runtime = spawnSync(pnpm, ['run', 'ensure:electron-runtime'], spawnOptions) + +if (runtime.status !== 0) { + process.exit(runtime.status ?? 1) +} + +const result = spawnSync( + pnpm, + [ + 'exec', + 'playwright', + 'test', + 'tests/e2e/gap-03-offline-reconnect-tab-resurrection.spec.ts', + '--config', + 'tests/playwright.config.ts', + '--project', + 'electron-headless', + '--workers=1', + ...extraArgs + ], + spawnOptions +) + +process.exit(result.status ?? 1) diff --git a/tests/e2e/gap-03-offline-reconnect-tab-resurrection.spec.ts b/tests/e2e/gap-03-offline-reconnect-tab-resurrection.spec.ts new file mode 100644 index 000000000000..70dd56064c1a --- /dev/null +++ b/tests/e2e/gap-03-offline-reconnect-tab-resurrection.spec.ts @@ -0,0 +1,227 @@ +/** + * GAP-03 (revive_labs#962 / stablyai/orca#22038): offline paired desktop clients retain stale + * tabs in `orca-data.json` and re-inject them upon reconnect. + * + * `fetchWorkspaceSessionWithRuntimeHostOwners` (src/renderer/src/lib/workspace-session-host-hydration.ts) + * runs exactly once per app lifetime, at renderer boot (`use-app-startup-hydration.ts`). It is + * NOT re-run on a live WebSocket/SSH reconnect inside a running app — confirmed by grepping every + * call site in this worktree. The bug therefore only manifests across an actual app relaunch: a + * paired client whose SSH host closed some of its tabs while the client was gone (network drop or + * the app itself not running), reopened later. `ssh-lost-kill-tab-resurrection.spec.ts` already + * covers the sibling, single-process-lifetime bug (STA-3374, a lost `pty.kill` reattach); this + * spec is deliberately a *two-launch* `createRestartSession` test — same shape as + * `golden-quit-relaunch-session.spec.ts` / `persisted-session-production-upgrade.spec.ts` — so it + * exercises the actual boot-time reconciliation path GAP-03 touches. + * + * Sequence, matching the six required GAP-03 repro steps: + * 1. Pair a client (first Electron launch) to a Dockerized SSH host (`docker-ssh-relay-target`). + * 2. Open N terminal tabs on the SSH-hosted worktree. + * 3. "Go offline": quit the client app. Its on-disk `orca-data.json` (`local.tabsByWorktree` and + * the frozen `ssh:` host-partition mirror in `workspaceSessionsByHostId`) persists + * untouched — this is the literal on-disk shape of "offline, app state intact". + * 4. Close M of the N tabs "server-side" while offline: edit ONLY the local base row + * (`workspaceSession.tabsByWorktree`) down to N-M tabs, leaving the stale SSH host-partition + * mirror at N tabs. This is the exact fixture shape + * `workspace-session-host-offline-reconnect.test.ts` uses, and precisely what "closed by the + * server while this client was offline" produces on disk (the host partition write requires + * the live SSH transport, which the offline client no longer has — see the spec's Current + * State section for the full mechanism). Editing the file directly (rather than an in-app + * admin RPC call) is this repo's own established technique for this exact class of fixture — + * see `persisted-session-production-upgrade.spec.ts`'s `installProductionSessionFixture`. + * 5. "Reconnect": relaunch the app against the same profile — this is what a user reopening an + * offline paired desktop client does, and the only trigger for the boot-time hydration path. + * 6. Measure resurrection: do the M closed tabs reappear? Also measure the #12721 non-regression: + * does a tab drafted entirely offline (present only in `local`, absent from every host + * partition, because it was never synced) survive the same relaunch? + */ +import { readFileSync, writeFileSync } from 'node:fs' +import path from 'node:path' +import type { ElectronApplication } from '@stablyai/playwright-test' +import { DEFAULT_LOCAL_ORCA_PROFILE_ID } from '../../src/shared/orca-profiles' +import { toSshExecutionHostId } from '../../src/shared/execution-host' +import { test, expect } from './helpers/orca-app' +import { createRestartSession } from './helpers/orca-restart' +import { waitForActiveWorktree, waitForSessionReady } from './helpers/store' +import { waitForActivePanePtyId, waitForActiveTerminalManager } from './helpers/terminal' +import { + createRemoteTerminalTab, + readRemoteTerminalTabs +} from './helpers/docker-ssh-relay-terminal-tabs' +import { + cleanupDockerSshRelayTarget, + DOCKER_SSH_PROXY_JUMP_REMOTE_REPO_PATH, + startDockerSshRelayTarget +} from './helpers/docker-ssh-relay-target' +import { connectDockerSshRelayTarget } from './helpers/docker-ssh-relay-connection' + +const RUN_DOCKER_SSH = process.env.ORCA_E2E_SSH_DOCKER === '1' +const TAB_COUNT = Math.max(2, Number(process.env.ORCA_GAP03_TAB_COUNT ?? '4')) +const CLOSE_COUNT = Math.min( + TAB_COUNT - 1, + Math.max(1, Number(process.env.ORCA_GAP03_CLOSE_COUNT ?? '2')) +) +const OFFLINE_DRAFT_TAB_ID = 'gap03-offline-draft-tab' + +type SessionProfile = { + workspaceSession: { tabsByWorktree: Record } + workspaceSessionsByHostId?: Record }> +} + +function profilePath(userDataDir: string): string { + return path.join(userDataDir, 'profiles', DEFAULT_LOCAL_ORCA_PROFILE_ID, 'orca-data.json') +} + +function readProfile(userDataDir: string): SessionProfile { + return JSON.parse(readFileSync(profilePath(userDataDir), 'utf8')) as SessionProfile +} + +/** Emit one machine-readable KPI line, matching this project's before/after report convention. */ +function logKpi(label: string, fields: Record): void { + console.log(`[gap-03] ${label}: ${JSON.stringify(fields)}`) +} + +test.describe('GAP-03: offline client reconnect must not resurrect server-closed tabs (Docker cross-site)', () => { + test.skip(!RUN_DOCKER_SSH, 'Set ORCA_E2E_SSH_DOCKER=1 to run Docker-backed GAP-03 tests.') + test.skip(process.platform === 'win32', 'Docker SSH restore uses POSIX SSH tooling.') + + test('closed-while-offline tabs do not resurrect on client reopen; #12721 offline draft survives', async (// oxlint-disable-next-line no-empty-pattern -- this test owns both Electron launches. + {}, testInfo) => { + test.setTimeout(600_000) + const session = createRestartSession(testInfo) + const target = startDockerSshRelayTarget(testInfo) + let firstApp: ElectronApplication | null = null + let secondApp: ElectronApplication | null = null + + try { + // --- Step 1+2: pair to the Dockerized SSH host, open N tabs --- + const first = await session.launch() + firstApp = first.app + await waitForSessionReady(first.page) + const remote = await connectDockerSshRelayTarget(first.page, target) + await expect + .poll(() => waitForActiveWorktree(first.page), { timeout: 30_000 }) + .toBe(remote.worktreeId) + await waitForActiveTerminalManager(first.page, 60_000) + await waitForActivePanePtyId(first.page, 60_000) + for (let i = 1; i < TAB_COUNT; i += 1) { + await createRemoteTerminalTab(first.page, remote.worktreeId) + } + const openedTabs = await readRemoteTerminalTabs(first.page, remote.worktreeId) + expect(openedTabs.length, 'setup must open exactly TAB_COUNT tabs').toBe(TAB_COUNT) + + // A second, real SSH-hosted worktree on the same target, connected with zero tabs. Its + // host-partition mirror genuinely has nothing for it — the exact "host has nothing for it" + // shape #12721 protects, reproduced with a real catalog entry instead of a synthetic key. + const offlineDraftTarget = await connectDockerSshRelayTarget(first.page, target, { + remotePath: DOCKER_SSH_PROXY_JUMP_REMOTE_REPO_PATH, + seedInitialTab: false + }) + logKpi('setup', { + worktreeId: remote.worktreeId, + tabCount: openedTabs.length, + offlineDraftWorktreeId: offlineDraftTarget.worktreeId + }) + + // --- Step 3: "go offline" — quit the client; its on-disk state (including the frozen SSH + // host-partition mirror) is left exactly as the live app last wrote it. --- + await session.close(firstApp) + firstApp = null + + const sshHostId = toSshExecutionHostId(remote.targetId) + const preFaultProfile = readProfile(session.userDataDir) + const hostMirrorTabs = + preFaultProfile.workspaceSessionsByHostId?.[sshHostId]?.tabsByWorktree?.[remote.worktreeId] + expect( + hostMirrorTabs?.length, + `the SSH host-partition mirror (${sshHostId}) must have cached every opened tab before the fault is injected` + ).toBe(TAB_COUNT) + logKpi('offline-snapshot', { + sshHostId, + localTabs: preFaultProfile.workspaceSession.tabsByWorktree[remote.worktreeId]?.length ?? 0, + hostMirrorTabs: hostMirrorTabs?.length ?? 0 + }) + + // --- Step 4: close M of the N tabs "server-side" while offline. The local base row is + // exactly what a live client resyncs to on next contact with the server; the stale SSH + // host-partition mirror is left untouched, matching what an offline client's own on-disk + // cache looks like the moment it went unreachable. --- + const closedIds = openedTabs.slice(0, CLOSE_COUNT).map((t) => t.id) + const survivorIds = openedTabs.slice(CLOSE_COUNT).map((t) => t.id) + const closedFaultProfile = readProfile(session.userDataDir) + const localRow = closedFaultProfile.workspaceSession.tabsByWorktree[remote.worktreeId] ?? [] + closedFaultProfile.workspaceSession.tabsByWorktree[remote.worktreeId] = localRow.filter( + (tab) => survivorIds.includes(tab.id) + ) + + // #12721 non-regression, injected in the same fault-injection pass: a tab drafted entirely + // offline lives only in `local`, on a real, catalog-known worktree whose SSH host partition + // has never heard of it at all (connected with zero tabs above) — the exact "host has + // nothing for it" shape `workspace-session-host-offline-reconnect.test.ts`'s non-regression + // case pins. + closedFaultProfile.workspaceSession.tabsByWorktree[offlineDraftTarget.worktreeId] = [ + { + id: OFFLINE_DRAFT_TAB_ID, + ptyId: null, + worktreeId: offlineDraftTarget.worktreeId, + title: 'offline draft', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: Date.now() + } as unknown as { id: string } + ] + writeFileSync( + profilePath(session.userDataDir), + `${JSON.stringify(closedFaultProfile, null, 2)}\n` + ) + logKpi('fault-injected', { + closedIds, + survivorIds, + offlineDraftWorktreeId: offlineDraftTarget.worktreeId + }) + + // --- Step 5: "reconnect" — relaunch against the same profile. This is the only code path + // that re-runs fetchWorkspaceSessionWithRuntimeHostOwners. --- + const second = await session.launch() + secondApp = second.app + await waitForSessionReady(second.page) + + // --- Step 6: measure resurrection and the #12721 non-regression. --- + const afterTabs = await readRemoteTerminalTabs(second.page, remote.worktreeId) + const afterIds = afterTabs.map((t) => t.id) + const resurrected = closedIds.filter((id) => afterIds.includes(id)) + const offlineDraftSurvived = await second.page.evaluate( + (id) => (window.__store?.getState().tabsByWorktree[id] ?? []).map((tab) => tab.id), + offlineDraftTarget.worktreeId + ) + logKpi('post-relaunch', { + expectedCount: survivorIds.length, + actualCount: afterIds.length, + resurrectedCount: resurrected.length, + resurrectedIds: resurrected, + offlineDraftSurvived: offlineDraftSurvived.includes(OFFLINE_DRAFT_TAB_ID) + }) + + expect( + resurrected, + `GAP-03: ${resurrected.length}/${closedIds.length} tabs closed while offline resurrected on reconnect` + ).toEqual([]) + expect( + afterIds.length, + `tab count after reconnect must equal the survivor count, not the pre-close total` + ).toBe(survivorIds.length) + expect( + offlineDraftSurvived, + '#12721 non-regression: an offline-created, never-synced tab must survive reconnect' + ).toContain(OFFLINE_DRAFT_TAB_ID) + } finally { + for (const app of [secondApp, firstApp]) { + if (app) { + await session.close(app).catch(() => undefined) + } + } + await session.dispose() + cleanupDockerSshRelayTarget(target) + } + }) +}) diff --git a/tests/e2e/helpers/docker-ssh-relay-target.ts b/tests/e2e/helpers/docker-ssh-relay-target.ts index 9d96859968ee..b681fecf3053 100644 --- a/tests/e2e/helpers/docker-ssh-relay-target.ts +++ b/tests/e2e/helpers/docker-ssh-relay-target.ts @@ -1,6 +1,6 @@ import { execFileSync, spawnSync } from 'node:child_process' import { randomUUID } from 'node:crypto' -import { mkdtempSync, readFileSync, rmSync } from 'node:fs' +import { chmodSync, mkdtempSync, readFileSync, rmSync } from 'node:fs' import os from 'node:os' import path from 'node:path' import { getDockerSshRelayImage } from './docker-ssh-relay-image' @@ -262,6 +262,9 @@ export function startDockerSshRelayTarget(testInfo: TestInfo): DockerSshRelayTar const tempDir = mkdtempSync(path.join(os.tmpdir(), 'orca-ssh-docker-')) const identityFile = path.join(tempDir, 'id_ed25519') run('ssh-keygen', ['-t', 'ed25519', '-N', '', '-f', identityFile, '-q']) + // Why: some hosts' tmp directories carry a default ACL that grants group/other read on new + // files regardless of umask, which sshd's StrictModes then rejects as an insecure key. + chmodSync(identityFile, 0o600) const publicKey = readFileSync(`${identityFile}.pub`, 'utf8').trim() const containerName = `orca-ssh-e2e-${testInfo.workerIndex}-${Date.now()}-${randomUUID().slice(0, 8)}` let target: DockerSshRelayTarget | null = null From dc15164831ab8be47ad033c3aee2a351bbf256bb Mon Sep 17 00:00:00 2001 From: Lesley Murfin Date: Mon, 21 Sep 2026 22:34:07 -0600 Subject: [PATCH 04/23] test(GAP-03): fix Docker harness fault-injection so it actually exercises the fix Slice A's harness (commit 8b26787bdc) measured 2/2 tabs resurrected on both pre-fix and (unverified) post-fix code, and offlineDraftSurvived: false even on pre-fix code -- an anomaly flagged for the next agent. Root-caused and fixed three independent harness defects, none in product code: 1. "Closed tabs" fault injection wrote workspaceSession.tabsByWorktree [worktreeId] = [] (a filter of an already-empty array) instead of deleting the key. adoptStrandedHostPartitionSession's GAP-03 decline path keys its verdict on Object.hasOwn(base.tabsByWorktree, workspaceId), and hasOwn is true for a key holding [] exactly as much as a populated array -- silently defeating the fix regardless of which code was under test. Also switched CLOSE_COUNT/TAB_COUNT defaults to close every opened tab (2/2): the fix's own decline logic is all-or-nothing (see its doc comment), so a scenario that leaves any survivor tab in the worktree never reaches the reconciledWorktreeIds path at all, pre- or post-fix, and proves nothing about the regression. 2. The #12721 offline-draft tab was injected only into legacy tabsByWorktree, never into unifiedTabs/tabGroups/tabGroupLayouts/ activeGroupIdByWorktree/activeTabIdByWorktree. The first launch's own quit-time write already populates unifiedTabs+tabGroups, so buildHydratedTabState takes the unified branch on the second launch and enumerates worktrees from unifiedTabs alone -- a legacy-only row is invisible there, and even where tabsByWorktree is swept for reconciliation, a legacy row with no live PTY and no unified twin is exactly what getOrphanTerminalIds deletes as an orphan on the same boot. This was the flagged anomaly: offlineDraftSurvived: false regardless of which product commit was under test, because the tab never reached a store the modern hydration path reads from. 3. Both SSH targets' activeConnectionIdsAtShutdown entries survived into the fault-injected profile, so the second launch auto- reconnected live SSH and pulled a fresh PTY snapshot straight off the still-alive Docker container (applyDirectSshRemoteWorkspaceSnapshot / mergeDirectSshRemoteWorkspaceSession) -- a code path GAP-03 never touches, since this fixture never actually kills the "closed" PTYs on the container. That live resync silently overwrote whatever the boot-time fetchWorkspaceSessionWithRuntimeHostOwners merge had correctly decided. Clearing the field keeps the second launch on the one path the fix targets: persisted-state hydration at boot, confirmed by temporary instrumentation showing adoptStrandedHostPartitionSession's own "adoptable" set already correctly emptied out before this fix (root-caused, then reverted). Measured clean before/after against the corrected harness by temporarily reverting to 36beb65f6b (fix's own parent) and back: BEFORE (36beb65f6b): resurrectedCount 2/2, offlineDraftSurvived true AFTER (129d11ad4d): resurrectedCount 0/2, offlineDraftSurvived true offlineDraftSurvived is true on BOTH runs, confirming the #12721 anomaly was a harness defect throughout, not a real regression on either commit. All 64 tests across the fix's own directly-affected unit-test files still pass unchanged. Also updated run-offline-reconnect-tab-resurrection-docker.mjs to stop overriding the spec's own TAB_COUNT/CLOSE_COUNT defaults. --- ...line-reconnect-tab-resurrection-docker.mjs | 19 ++- ...offline-reconnect-tab-resurrection.spec.ts | 140 ++++++++++++++---- 2 files changed, 127 insertions(+), 32 deletions(-) diff --git a/config/scripts/run-offline-reconnect-tab-resurrection-docker.mjs b/config/scripts/run-offline-reconnect-tab-resurrection-docker.mjs index c787601e2acb..32c708a57425 100755 --- a/config/scripts/run-offline-reconnect-tab-resurrection-docker.mjs +++ b/config/scripts/run-offline-reconnect-tab-resurrection-docker.mjs @@ -3,22 +3,29 @@ import { spawnSync } from 'node:child_process' // Runs the GAP-03 (revive_labs#962 / stablyai/orca#22038) offline-reconnect tab resurrection // harness: a two-launch Electron test that pairs a client to a Dockerized SSH host, opens N -// terminal tabs, quits the client, closes some of those tabs "server-side" while it is offline, -// relaunches the client, and measures whether the closed tabs resurrect. +// terminal tabs, quits the client, closes every one of those tabs "server-side" while it is +// offline, relaunches the client, and measures whether the closed tabs resurrect. // // Modeled directly on run-ssh-docker-e2e.mjs: same pnpm/Playwright invocation shape, same // ORCA_E2E_SSH_DOCKER env-gate the target spec reads to skip itself outside a Docker-capable host. // -// ORCA_GAP03_TAB_COUNT (default 4) and ORCA_GAP03_CLOSE_COUNT (default 2) tune the repro's N/M -// without editing the spec, so the same harness can drive a smaller or larger KPI run. +// ORCA_GAP03_TAB_COUNT and ORCA_GAP03_CLOSE_COUNT tune the repro's N/M without editing the spec. +// Left unset here so the spec's own defaults (TAB_COUNT=2, CLOSE_COUNT=TAB_COUNT, i.e. every +// opened tab closes) apply — GAP-03's fix is an all-or-nothing repair (see the spec's own doc +// comment on CLOSE_COUNT), so a run that leaves any survivor tab in the worktree never exercises +// the regression at all, pre- or post-fix. const rawExtraArgs = process.argv.slice(2) const extraArgs = rawExtraArgs[0] === '--' ? rawExtraArgs.slice(1) : rawExtraArgs const pnpm = process.platform === 'win32' ? 'pnpm.cmd' : 'pnpm' const env = { ...process.env, ORCA_E2E_SSH_DOCKER: '1', - ORCA_GAP03_TAB_COUNT: process.env.ORCA_GAP03_TAB_COUNT ?? '4', - ORCA_GAP03_CLOSE_COUNT: process.env.ORCA_GAP03_CLOSE_COUNT ?? '2' + ...(process.env.ORCA_GAP03_TAB_COUNT + ? { ORCA_GAP03_TAB_COUNT: process.env.ORCA_GAP03_TAB_COUNT } + : {}), + ...(process.env.ORCA_GAP03_CLOSE_COUNT + ? { ORCA_GAP03_CLOSE_COUNT: process.env.ORCA_GAP03_CLOSE_COUNT } + : {}) } // Why: Node's CVE-2024-27980 hardening rejects .cmd spawns without shell on Windows. diff --git a/tests/e2e/gap-03-offline-reconnect-tab-resurrection.spec.ts b/tests/e2e/gap-03-offline-reconnect-tab-resurrection.spec.ts index 70dd56064c1a..b33bc8904c25 100644 --- a/tests/e2e/gap-03-offline-reconnect-tab-resurrection.spec.ts +++ b/tests/e2e/gap-03-offline-reconnect-tab-resurrection.spec.ts @@ -39,6 +39,9 @@ import path from 'node:path' import type { ElectronApplication } from '@stablyai/playwright-test' import { DEFAULT_LOCAL_ORCA_PROFILE_ID } from '../../src/shared/orca-profiles' import { toSshExecutionHostId } from '../../src/shared/execution-host' +import type { Tab, TabGroup, TabGroupLayoutNode } from '../../src/shared/tab-types' +import type { TerminalTab } from '../../src/shared/terminal-tab-types' +import type { WorkspaceSessionState } from '../../src/shared/workspace-session-state-types' import { test, expect } from './helpers/orca-app' import { createRestartSession } from './helpers/orca-restart' import { waitForActiveWorktree, waitForSessionReady } from './helpers/store' @@ -55,16 +58,25 @@ import { import { connectDockerSshRelayTarget } from './helpers/docker-ssh-relay-connection' const RUN_DOCKER_SSH = process.env.ORCA_E2E_SSH_DOCKER === '1' -const TAB_COUNT = Math.max(2, Number(process.env.ORCA_GAP03_TAB_COUNT ?? '4')) +// Why full closure, not partial: adoptStrandedHostPartitionSession's GAP-03 decline path +// (`reconciledWorktreeIds`) only distinguishes "the base names this worktree at all" from "it +// doesn't" -- see that module's own doc comment ("The one thing the base keeps unconditionally is +// a workspace it holds terminal tabs for... An EMPTY tab row is not such a copy"). A base row that +// still names ANY survivor tab makes `workspacesTheBaseOwns` treat the whole worktree as owned and +// excludes it from adoption entirely -- true before AND after the fix, so it proves nothing about +// the regression. The bug's own root-cause description is "every tab for a worktree closed +// server-side while a client was offline" -- not some. CLOSE_COUNT therefore defaults to, and is +// clamped to, TAB_COUNT. +const TAB_COUNT = Math.max(2, Number(process.env.ORCA_GAP03_TAB_COUNT ?? '2')) const CLOSE_COUNT = Math.min( - TAB_COUNT - 1, - Math.max(1, Number(process.env.ORCA_GAP03_CLOSE_COUNT ?? '2')) + TAB_COUNT, + Math.max(1, Number(process.env.ORCA_GAP03_CLOSE_COUNT ?? String(TAB_COUNT))) ) const OFFLINE_DRAFT_TAB_ID = 'gap03-offline-draft-tab' type SessionProfile = { - workspaceSession: { tabsByWorktree: Record } - workspaceSessionsByHostId?: Record }> + workspaceSession: WorkspaceSessionState + workspaceSessionsByHostId?: Record } function profilePath(userDataDir: string): string { @@ -141,35 +153,111 @@ test.describe('GAP-03: offline client reconnect must not resurrect server-closed hostMirrorTabs: hostMirrorTabs?.length ?? 0 }) - // --- Step 4: close M of the N tabs "server-side" while offline. The local base row is - // exactly what a live client resyncs to on next contact with the server; the stale SSH - // host-partition mirror is left untouched, matching what an offline client's own on-disk - // cache looks like the moment it went unreachable. --- + // --- Step 4: close M of the N tabs "server-side" while offline. `remote.worktreeId` is + // SSH-hosted, so its worktree-keyed fields (tabsByWorktree, unifiedTabs, tabGroups, ...) + // never land in the local base row in the first place — `buildHostSessionRouting` always + // routes them to `ssh:` at quit time, confirmed by offline-snapshot's + // `localTabs: 0` above. "Closing" the base row is therefore deleting the key outright, never + // writing an empty array: `adoptStrandedHostPartitionSession`'s GAP-03 decline path keys its + // verdict on `Object.hasOwn(base.tabsByWorktree, workspaceId)`, and an explicit `[]` + // satisfies `hasOwn` exactly as much as a populated row would — silently defeating the fix. + // The stale SSH host-partition mirror is left untouched, matching what an offline client's + // own on-disk cache looks like the moment it went unreachable. --- const closedIds = openedTabs.slice(0, CLOSE_COUNT).map((t) => t.id) const survivorIds = openedTabs.slice(CLOSE_COUNT).map((t) => t.id) const closedFaultProfile = readProfile(session.userDataDir) - const localRow = closedFaultProfile.workspaceSession.tabsByWorktree[remote.worktreeId] ?? [] - closedFaultProfile.workspaceSession.tabsByWorktree[remote.worktreeId] = localRow.filter( - (tab) => survivorIds.includes(tab.id) - ) + const survivorRow = ( + closedFaultProfile.workspaceSession.tabsByWorktree[remote.worktreeId] ?? [] + ).filter((tab) => survivorIds.includes(tab.id)) + if (survivorRow.length > 0) { + closedFaultProfile.workspaceSession.tabsByWorktree[remote.worktreeId] = survivorRow + } else { + delete closedFaultProfile.workspaceSession.tabsByWorktree[remote.worktreeId] + } // #12721 non-regression, injected in the same fault-injection pass: a tab drafted entirely // offline lives only in `local`, on a real, catalog-known worktree whose SSH host partition // has never heard of it at all (connected with zero tabs above) — the exact "host has // nothing for it" shape `workspace-session-host-offline-reconnect.test.ts`'s non-regression - // case pins. - closedFaultProfile.workspaceSession.tabsByWorktree[offlineDraftTarget.worktreeId] = [ - { - id: OFFLINE_DRAFT_TAB_ID, - ptyId: null, - worktreeId: offlineDraftTarget.worktreeId, - title: 'offline draft', - customTitle: null, - color: null, - sortOrder: 0, - createdAt: Date.now() - } as unknown as { id: string } + // case pins. Written in BOTH the legacy `tabsByWorktree` row and the modern unified-tab + // format (`unifiedTabs`/`tabGroups`/`tabGroupLayouts`/`activeGroupIdByWorktree`/ + // `activeTabIdByWorktree`): the first real launch's own quit-time write already populated + // `unifiedTabs`/`tabGroups` in `workspaceSession` (a running client always writes both + // formats together), so `buildHydratedTabState` takes the unified branch + // (`session.unifiedTabs && session.tabGroups`) on the second launch and enumerates worktrees + // from `unifiedTabs` alone — never from `tabsByWorktree`. A row that exists only in the + // legacy field is invisible to that enumeration, and even where `tabsByWorktree` IS swept + // (`reconcileHydratedWorkspaceTabModels`), a legacy row with no live PTY and no unified twin + // is exactly what `getOrphanTerminalIds` deletes as an orphan on the very same boot. This was + // the #12721 anomaly: `offlineDraftSurvived: false` even on pre-fix code, because the tab + // never reached a store the modern hydration path reads from at all. + const offlineDraftWorktreeId = offlineDraftTarget.worktreeId + const offlineDraftGroupId = `${OFFLINE_DRAFT_TAB_ID}-group` + const offlineDraftTerminalTab: TerminalTab = { + id: OFFLINE_DRAFT_TAB_ID, + ptyId: null, + worktreeId: offlineDraftWorktreeId, + title: 'offline draft', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: Date.now() + } + const offlineDraftTab: Tab = { + id: OFFLINE_DRAFT_TAB_ID, + entityId: OFFLINE_DRAFT_TAB_ID, + groupId: offlineDraftGroupId, + worktreeId: offlineDraftWorktreeId, + contentType: 'terminal', + label: 'offline draft', + customLabel: null, + color: null, + sortOrder: 0, + createdAt: Date.now() + } + const offlineDraftGroup: TabGroup = { + id: offlineDraftGroupId, + worktreeId: offlineDraftWorktreeId, + activeTabId: OFFLINE_DRAFT_TAB_ID, + tabOrder: [OFFLINE_DRAFT_TAB_ID] + } + const offlineDraftLayout: TabGroupLayoutNode = { type: 'leaf', groupId: offlineDraftGroupId } + closedFaultProfile.workspaceSession.tabsByWorktree[offlineDraftWorktreeId] = [ + offlineDraftTerminalTab ] + closedFaultProfile.workspaceSession.unifiedTabs = { + ...closedFaultProfile.workspaceSession.unifiedTabs, + [offlineDraftWorktreeId]: [offlineDraftTab] + } + closedFaultProfile.workspaceSession.tabGroups = { + ...closedFaultProfile.workspaceSession.tabGroups, + [offlineDraftWorktreeId]: [offlineDraftGroup] + } + closedFaultProfile.workspaceSession.tabGroupLayouts = { + ...closedFaultProfile.workspaceSession.tabGroupLayouts, + [offlineDraftWorktreeId]: offlineDraftLayout + } + closedFaultProfile.workspaceSession.activeGroupIdByWorktree = { + ...closedFaultProfile.workspaceSession.activeGroupIdByWorktree, + [offlineDraftWorktreeId]: offlineDraftGroupId + } + closedFaultProfile.workspaceSession.activeTabIdByWorktree = { + ...closedFaultProfile.workspaceSession.activeTabIdByWorktree, + [offlineDraftWorktreeId]: OFFLINE_DRAFT_TAB_ID + } + // Prevent the second launch from auto-reconnecting either SSH target. GAP-03's own fix + // (`fetchWorkspaceSessionWithRuntimeHostOwners`) is the boot-time PERSISTED-state merge; it + // is not the only thing that can populate `tabsByWorktree` after a relaunch. If the client + // still lists a target in `activeConnectionIdsAtShutdown`, `use-app-startup-hydration.ts` + // auto-reconnects it, and a LIVE SSH reconnect pulls a fresh PTY snapshot straight off the + // real host (`applyDirectSshRemoteWorkspaceSnapshot` / `mergeDirectSshRemoteWorkspaceSession`) + // — a wholly separate code path GAP-03 never touches. This Docker fixture's "closed while + // offline" tabs were only ever removed from the persisted JSON, never actually killed on the + // container, so a live reconnect resyncs to the still-alive PTYs and resurrects them + // regardless of the fix. Clearing this field keeps the second launch on the one path the + // fix is verifying: boot-time hydration of persisted state, with nothing left to + // second-guess it. + delete closedFaultProfile.workspaceSession.activeConnectionIdsAtShutdown writeFileSync( profilePath(session.userDataDir), `${JSON.stringify(closedFaultProfile, null, 2)}\n` @@ -177,7 +265,7 @@ test.describe('GAP-03: offline client reconnect must not resurrect server-closed logKpi('fault-injected', { closedIds, survivorIds, - offlineDraftWorktreeId: offlineDraftTarget.worktreeId + offlineDraftWorktreeId }) // --- Step 5: "reconnect" — relaunch against the same profile. This is the only code path From 8c1a9601e6a604a020f23a79553b6a57940756a0 Mon Sep 17 00:00:00 2001 From: Lesley Murfin Date: Tue, 22 Sep 2026 00:07:53 -0600 Subject: [PATCH 05/23] fix(GAP-03): close the ordinary-restart false positive; verify remaining PR #968 scenarios reconciledWorktreeIdsForHost applied unconditionally to every catalog-confirmed worktree, regardless of whether this client's SSH connection to that host was ever actually interrupted. Since an SSH-hosted worktree's tabsByWorktree row never routes to the 'local' base partition in the first place (buildHostSessionRouting always sends it to ssh:), base.tabsByWorktree is absent for EVERY confirmed worktree on EVERY boot -- closed or not. Combined with remote-repo-registration.ts stamping the modern executionHostId field on every SSH repo it registers (not a rare legacy shape), this meant an ordinary quit + relaunch with nothing closed by anyone dropped every still-open tab on every SSH worktree whose repo carries the modern stamp -- the opposite of GAP-03's own goal, and a severe regression the existing 115-test suite, the Docker e2e harness (which always closes every opened tab, CLOSE_COUNT=TAB_COUNT by design) and the 4-client live verification never exercised, because none of them ever tested a worktree with a genuine SURVIVOR tab through the confirmed attribution path. Fix: gate reconciledWorktreeIdsForHost on whether this host's SSH target was actually connected at the client's last shutdown (activeConnectionIdsAtShutdown -- the same field use-app-startup-hydration.ts already reads to decide which targets to auto-reconnect at boot). A target still listed there was never disconnected; its host-partition mirror is this client's own current state written moments before quitting, not a stale pre-offline cache, so an absent base row for it was never evidence of anything and must not be read as "the server's confirmed zero." A target NOT listed there is the actual GAP-03 precondition, and still declines exactly as before. Verification of PR #968's 4 deferred pr-agent scenarios, RED-then-GREEN against this fix's own parent (36beb65f6b) and current HEAD, per-scenario: - Concurrent Active Edits: the achievable case (host not disconnected -- see the restart regression above) now adopts correctly. The harder case (a live sibling worktree while genuinely offline) is a DOCUMENTED, PROVEN architectural limitation: reconciledWorktreeIdsForHost gates per host, not per worktree, so a worktree the base has never cached is indistinguishable from one whose tabs were genuinely closed while offline. Both are safely parked (never deleted -- attachHostSessionShadow/partitionRowsTheWriteWontReturn), just not adopted this boot. Resolving this needs a live server signal (the upstream draft's own manifestEpoch/handshake proposal); this fix has none. - Split Layout Pruning: proven safe by construction -- GAP-03's decline is per-worktree across every field, not per-tab, so a declined worktree carries no rows in tabsByWorktree, unifiedTabs, tabGroups OR tabGroupLayouts. No orphaned split leaf can survive because nothing about that worktree is adopted at all. - Write Race Protection: CONFIRMED, REPRODUCIBLE, UNFIXED bug. A wake-triggered persistWorkspaceSessionByHost never touches a host partition with nothing else routed to it (no race). The instant anything else legitimately writes to that same host partition in the same call (a sibling worktree gaining a tab), attachHostSessionShadow unconditionally re-attaches EVERY parked row for that host -- including one GAP-03 just correctly declined -- and it round-trips back to the server. A naive "drop instead of park" mitigation was prototyped and reverted: it closes this race for a genuinely-closed worktree but is blind to the Concurrent-Active-Edits false positive, turning a recoverable non-adoption into an unrecoverable delete of another client's live tabs. Answers PR #968's deferred question directly: hasPendingReconnectionHandshake is NOT unnecessary -- it (or an equivalent live signal) is the only thing that would close this race without risking data loss. - Rapid Sleep/Wake Cycles: verified convergent and non-corrupting across 5 consecutive boot cycles against an unchanged on-disk profile, with no cumulative resurrection and no duplicate adoption. Also adds a real Docker cross-site e2e scenario (ordinary quit + relaunch, nothing closed, connection still active at shutdown) proving the restart regression fix end-to-end, not just at the unit level. New/changed tests: workspace-session-host-offline-reconnect.test.ts (+7), workspace-session-host-write-race.test.ts (new, 2), workspace-session-host- rapid-reconnect-cycles.test.ts (new, 2). All 115 pre-existing GAP-03 tests still pass unchanged; 124 total across the 11 touched files. No PR opened; stays on this pushed branch pending owner sign-off, per this slice's constraints (nothing goes upstream until fully verified). --- .../lib/workspace-session-host-hydration.ts | 11 +- ...ace-session-host-offline-reconnect.test.ts | 202 +++++++++++++++++ ...ession-host-rapid-reconnect-cycles.test.ts | 149 +++++++++++++ .../workspace-session-host-reconciliation.ts | 26 ++- .../workspace-session-host-write-race.test.ts | 211 ++++++++++++++++++ ...offline-reconnect-tab-resurrection.spec.ts | 79 +++++++ 6 files changed, 672 insertions(+), 6 deletions(-) create mode 100644 src/renderer/src/lib/workspace-session-host-rapid-reconnect-cycles.test.ts create mode 100644 src/renderer/src/lib/workspace-session-host-write-race.test.ts diff --git a/src/renderer/src/lib/workspace-session-host-hydration.ts b/src/renderer/src/lib/workspace-session-host-hydration.ts index 4138760da1f6..bd85401e04fc 100644 --- a/src/renderer/src/lib/workspace-session-host-hydration.ts +++ b/src/renderer/src/lib/workspace-session-host-hydration.ts @@ -2,6 +2,7 @@ import type { Repo } from '../../../shared/repo-types' import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' import { getRepoExecutionHostId, + getSshTargetIdForExecutionHost, LOCAL_EXECUTION_HOST_ID, parseExecutionHostId, type ExecutionHostId @@ -217,7 +218,11 @@ export async function fetchWorkspaceSessionWithRuntimeHostOwners( // for the slices it arbitrates, and these are not among them. Everything this read leaves behind // in an ssh partition still has to survive the next write to it. const shadow: HostSessionSlices = { ...merged.shadow } + // Why captured once, before the loop: `activeConnectionIdsAtShutdown` is global (routes to + // 'local' unconditionally), so it never changes as `session` is reassigned across hosts below. + const connectedTargetIdsAtShutdown = new Set(merged.session.activeConnectionIdsAtShutdown ?? []) for (const [hostId, slice] of sshPartitions) { + const targetId = getSshTargetIdForExecutionHost(hostId) const adoption = adoptStrandedHostPartitionSession(session, slice, { contestedSessionKeys: attribution.contestedSessionKeys, ownedSessionKeys: attribution.ownedSessionKeysByHostId.get(hostId), @@ -226,7 +231,11 @@ export async function fetchWorkspaceSessionWithRuntimeHostOwners( attribution.contestedSessionKeys, attribution.foreignSessionKeysByHostId.get(hostId) ), - reconciledWorktreeIds: reconciledWorktreeIdsForHost(attribution, hostId) + reconciledWorktreeIds: reconciledWorktreeIdsForHost( + attribution, + hostId, + Boolean(targetId && connectedTargetIdsAtShutdown.has(targetId)) + ) }) session = adoption.session if (slice) { diff --git a/src/renderer/src/lib/workspace-session-host-offline-reconnect.test.ts b/src/renderer/src/lib/workspace-session-host-offline-reconnect.test.ts index 674daf85f349..088efa7ed2fb 100644 --- a/src/renderer/src/lib/workspace-session-host-offline-reconnect.test.ts +++ b/src/renderer/src/lib/workspace-session-host-offline-reconnect.test.ts @@ -107,3 +107,205 @@ describe('GAP-03 non-regression: #12721 offline-created local work must survive ]) }) }) + +describe('GAP-03 regression: an ordinary restart must not be treated as an offline reconnect', () => { + it('keeps every still-open tab when the SSH target was still connected at the last shutdown', async () => { + // `remote-repo-registration.ts` stamps `executionHostId` on every SSH repo it registers (the + // modern, default shape -- not a rare legacy one), so `confirmedSessionKeysByHostId` fires for + // this repo exactly as it would in real usage. Before the `activeConnectionIdsAtShutdown` gate + // was added, `reconciledWorktreeIdsForHost` applied unconditionally: since an SSH worktree's + // `tabsByWorktree` row NEVER routes to `local` in the first place (`buildHostSessionRouting` + // always sends it to `ssh:`), `base.tabsByWorktree[WORKTREE_ID]` is absent on EVERY + // boot, closed or not -- so this exact "two still-open tabs, ordinary quit + relaunch, nothing + // closed by anyone" case was declined and dropped on every single restart of every SSH + // worktree whose repo carries the modern stamp. `activeConnectionIdsAtShutdown` is the same + // field `use-app-startup-hydration.ts` reads to decide which SSH targets to auto-reconnect at + // boot, so "this target was connected at last shutdown" is an existing, meaningful signal + // that the host partition's mirror is this client's OWN current state, not a stale pre-offline + // cache -- and the one case GAP-03 was never meant to touch at all. + const read = await fetchWorkspaceSessionWithRuntimeHostOwners( + partitionedApi({ + local: session({ activeConnectionIdsAtShutdown: [TARGET_ID] }), + [SSH_HOST_ID]: session({ + tabsByWorktree: { + [WORKTREE_ID]: [tab('tab-1', WORKTREE_ID), tab('tab-2', WORKTREE_ID)] + } + }) + }), + [{ id: REPO_ID, connectionId: TARGET_ID, executionHostId: SSH_HOST_ID }] + ) + + expect(read.session.tabsByWorktree[WORKTREE_ID]?.map((entry) => entry.id)).toEqual([ + 'tab-1', + 'tab-2' + ]) + }) + + it('still declines a worktree whose tabs were genuinely closed while the target was disconnected at shutdown', async () => { + // Same confirmed-attribution shape as the regression above, but the target was NOT connected + // at last shutdown -- the actual GAP-03 precondition -- so the decline must still fire. + const read = await fetchWorkspaceSessionWithRuntimeHostOwners( + partitionedApi({ + local: session({}), + [SSH_HOST_ID]: session({ + tabsByWorktree: { + [WORKTREE_ID]: [tab('tab-2', WORKTREE_ID), tab('tab-3', WORKTREE_ID)] + } + }) + }), + [{ id: REPO_ID, connectionId: TARGET_ID, executionHostId: SSH_HOST_ID }] + ) + + expect(read.session.tabsByWorktree[WORKTREE_ID] ?? []).toEqual([]) + }) +}) + +describe('GAP-03 Split Layout Pruning: a declined worktree carries no orphaned split-layout leaves', () => { + it('excludes tabGroupLayouts/tabGroups/unifiedTabs for a declined worktree exactly like tabsByWorktree, so no dead-leaf layout survives to be hydrated', async () => { + const SPLIT_GROUP_LEFT = 'group-left' + const SPLIT_GROUP_RIGHT = 'group-right' + const read = await fetchWorkspaceSessionWithRuntimeHostOwners( + partitionedApi({ + local: session({}), + [SSH_HOST_ID]: session({ + tabsByWorktree: { + [WORKTREE_ID]: [tab('tab-2', WORKTREE_ID), tab('tab-3', WORKTREE_ID)] + }, + unifiedTabs: { + [WORKTREE_ID]: [ + { + id: 'tab-2', + entityId: 'tab-2', + groupId: SPLIT_GROUP_LEFT, + worktreeId: WORKTREE_ID, + contentType: 'terminal', + label: 'tab-2', + customLabel: null, + color: null, + sortOrder: 0, + createdAt: 1 + }, + { + id: 'tab-3', + entityId: 'tab-3', + groupId: SPLIT_GROUP_RIGHT, + worktreeId: WORKTREE_ID, + contentType: 'terminal', + label: 'tab-3', + customLabel: null, + color: null, + sortOrder: 1, + createdAt: 1 + } + ] + }, + tabGroups: { + [WORKTREE_ID]: [ + { + id: SPLIT_GROUP_LEFT, + worktreeId: WORKTREE_ID, + activeTabId: 'tab-2', + tabOrder: ['tab-2'] + }, + { + id: SPLIT_GROUP_RIGHT, + worktreeId: WORKTREE_ID, + activeTabId: 'tab-3', + tabOrder: ['tab-3'] + } + ] + }, + tabGroupLayouts: { + [WORKTREE_ID]: { + type: 'split', + direction: 'horizontal', + first: { type: 'leaf', groupId: SPLIT_GROUP_LEFT }, + second: { type: 'leaf', groupId: SPLIT_GROUP_RIGHT } + } + } + }) + }), + [{ id: REPO_ID, connectionId: TARGET_ID, executionHostId: SSH_HOST_ID }] + ) + + // The decline is per-worktree across every field, not per-tab: a worktree that is declined + // carries no rows in ANY of these fields, so there is no stale split node left over to prune. + expect(read.session.tabsByWorktree[WORKTREE_ID] ?? []).toEqual([]) + expect(read.session.unifiedTabs?.[WORKTREE_ID]).toBeUndefined() + expect(read.session.tabGroups?.[WORKTREE_ID]).toBeUndefined() + expect(read.session.tabGroupLayouts?.[WORKTREE_ID]).toBeUndefined() + }) +}) + +describe('GAP-03 Concurrent Active Edits', () => { + it('achievable case: a live sibling worktree on the same confirmed host adopts normally when the host was NOT disconnected', async () => { + // Maps "Client A opens Tab-4 on another worktree while Client B is online" onto this + // architecture: B's own connection to the shared SSH target was never interrupted (still + // listed in `activeConnectionIdsAtShutdown`), so BOTH this worktree's own survivor tab and a + // second, previously-unseen worktree's freshly-opened tab on the SAME host must adopt -- + // reconciliation is skipped for the whole host, exactly like the plain-restart regression + // above, because nothing about this boot represents an offline gap at all. + const SIBLING_WORKTREE_ID = `${REPO_ID}-2::/remote/sibling` + const read = await fetchWorkspaceSessionWithRuntimeHostOwners( + partitionedApi({ + local: session({ activeConnectionIdsAtShutdown: [TARGET_ID] }), + [SSH_HOST_ID]: session({ + tabsByWorktree: { + [WORKTREE_ID]: [tab('tab-1', WORKTREE_ID)], + [SIBLING_WORKTREE_ID]: [tab('tab-4', SIBLING_WORKTREE_ID)] + } + }) + }), + [ + { id: REPO_ID, connectionId: TARGET_ID, executionHostId: SSH_HOST_ID }, + { id: `${REPO_ID}-2`, connectionId: TARGET_ID, executionHostId: SSH_HOST_ID } + ] + ) + + expect(read.session.tabsByWorktree[WORKTREE_ID]?.map((entry) => entry.id)).toEqual(['tab-1']) + expect(read.session.tabsByWorktree[SIBLING_WORKTREE_ID]?.map((entry) => entry.id)).toEqual([ + 'tab-4' + ]) + }) + + it('documented boundary: a genuinely offline reconnect parks (never deletes) a live sibling worktree it cannot yet distinguish from a closed one', async () => { + // The harder case the upstream matrix describes -- Client A opens a brand-new worktree's + // first tab while Client B is genuinely OFFLINE (disconnected at last shutdown, the real + // GAP-03 precondition) -- is NOT solvable by this fix's design. `reconciledWorktreeIdsForHost` + // gates per HOST, not per worktree: once a host is treated as "possibly stale" (disconnected + // at shutdown), a worktree the base has never cached at all is indistinguishable from one + // whose tabs were genuinely closed while offline -- both look identical on disk (absent base + // row, non-empty host mirror). Resolving this would need a live signal from the server (an + // authoritative manifest/handshake, e.g. the upstream draft's own `manifestEpoch` proposal) -- + // there is no such signal in this fix. The behavior below is the SAFE fallback: the sibling's + // row is parked in the write-side shadow (survives, recoverable by a live SSH reconnect later) + // rather than deleted, but it is NOT adopted into the visible session on this boot. + const SIBLING_WORKTREE_ID = `${REPO_ID}-2::/remote/sibling` + const read = await fetchWorkspaceSessionWithRuntimeHostOwners( + partitionedApi({ + local: session({}), + [SSH_HOST_ID]: session({ + tabsByWorktree: { + [WORKTREE_ID]: [tab('tab-2', WORKTREE_ID), tab('tab-3', WORKTREE_ID)], + [SIBLING_WORKTREE_ID]: [tab('tab-4', SIBLING_WORKTREE_ID)] + } + }) + }), + [ + { id: REPO_ID, connectionId: TARGET_ID, executionHostId: SSH_HOST_ID }, + { id: `${REPO_ID}-2`, connectionId: TARGET_ID, executionHostId: SSH_HOST_ID } + ] + ) + + // Not adopted this boot -- the known, accepted limitation. + expect(read.session.tabsByWorktree[SIBLING_WORKTREE_ID]).toBeUndefined() + // But not deleted either: it survives in the write-side shadow for the host partition, so a + // live SSH reconnect (a separate code path this fix does not touch) can still recover it, and + // the next write to this host cannot erase it (`attachHostSessionShadow`). + expect( + read.contestedHostWorkspaceSessions[SSH_HOST_ID]?.tabsByWorktree?.[SIBLING_WORKTREE_ID]?.map( + (entry: TerminalTab) => entry.id + ) + ).toEqual(['tab-4']) + }) +}) diff --git a/src/renderer/src/lib/workspace-session-host-rapid-reconnect-cycles.test.ts b/src/renderer/src/lib/workspace-session-host-rapid-reconnect-cycles.test.ts new file mode 100644 index 000000000000..3db8be1d2f53 --- /dev/null +++ b/src/renderer/src/lib/workspace-session-host-rapid-reconnect-cycles.test.ts @@ -0,0 +1,149 @@ +/** + * GAP-03 Rapid Sleep/Wake Cycles (revive_labs#962 / stablyai/orca#22038, PR #968 deferred + * scenario): `fetchWorkspaceSessionWithRuntimeHostOwners` "runs exactly once per app lifetime, at + * renderer boot" (see `gap-03-offline-reconnect-tab-resurrection.spec.ts`'s own docblock) — a + * suspend/resume or a network drop mid-session never re-runs it. The only way this hydration path + * runs more than once in a row is a sequence of full app quit+relaunch cycles against the same + * on-disk profile. This suite models "rapid sleep/wake" as exactly that: several consecutive + * boot cycles in quick succession, with the server closing more tabs between some of them, + * and checks for the upstream matrix's three concerns — + * 1. no cumulative resurrection (a tab declined on cycle N never reappears on cycle N+1..5), + * 2. no state corruption (each cycle's `session` is a normal, well-formed session), and + * 3. no duplicate adoption (a tab adopted once is never adopted a second time from the same + * stale mirror on a later cycle). + */ +import { describe, expect, it } from 'vitest' +import { getDefaultWorkspaceSession } from '../../../shared/constants' +import type { ExecutionHostId } from '../../../shared/execution-host' +import { normalizeExecutionHostId } from '../../../shared/execution-host' +import type { TerminalTab } from '../../../shared/terminal-tab-types' +import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' +import { fetchWorkspaceSessionWithRuntimeHostOwners } from './workspace-session-host-hydration' + +const TARGET_ID = 'target-1' +const SSH_HOST_ID: ExecutionHostId = `ssh:${TARGET_ID}` +const REPO_ID = 'repo-remote' +const WORKTREE_ID = `${REPO_ID}::/remote/checkout` +const REPOS = [{ id: REPO_ID, connectionId: TARGET_ID, executionHostId: SSH_HOST_ID }] + +function session(overrides: Partial): WorkspaceSessionState { + return { ...getDefaultWorkspaceSession(), ...overrides } +} + +function tab(id: string, worktreeId: string): TerminalTab { + return { + id, + ptyId: `pty-${id}`, + worktreeId, + title: id, + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } +} + +function partitionedApi(partitions: Partial>) { + return { + get: async (hostId?: ExecutionHostId) => + partitions[hostId ?? 'local'] ?? getDefaultWorkspaceSession(), + listHostIds: async () => + Object.keys(partitions).flatMap((hostId) => normalizeExecutionHostId(hostId) ?? []) + } +} + +describe('GAP-03 Rapid Sleep/Wake Cycles: five consecutive quit+relaunch cycles against the same on-disk profile', () => { + it('never resurrects a tab that a prior cycle already declined, across 5 rapid cycles with tabs closing between some of them', async () => { + // Cycle 1: two tabs open, target disconnected at shutdown (the offline gap). All server-side + // closures already happened before cycle 1 boots, matching "closed while offline" exactly. + const cycle1 = await fetchWorkspaceSessionWithRuntimeHostOwners( + partitionedApi({ + local: session({}), + [SSH_HOST_ID]: session({ + tabsByWorktree: { + [WORKTREE_ID]: [tab('tab-1', WORKTREE_ID), tab('tab-2', WORKTREE_ID)] + } + }) + }), + REPOS + ) + expect(cycle1.session.tabsByWorktree[WORKTREE_ID] ?? []).toEqual([]) + + // Cycles 2-5: the on-disk host mirror is untouched (nobody rewrote it — the same stale cache + // from before cycle 1 persists, since cycle 1 never adopted or wrote anything for this + // worktree). Each cycle re-reads the exact same stale mirror. None of the 4 additional cycles + // may resurrect the declined tabs, and none may produce a different verdict than cycle 1 — + // convergence, not drift, across repeated boots of an unchanged on-disk state. + for (let cycle = 2; cycle <= 5; cycle += 1) { + const read = await fetchWorkspaceSessionWithRuntimeHostOwners( + partitionedApi({ + local: session({}), + [SSH_HOST_ID]: session({ + tabsByWorktree: { + [WORKTREE_ID]: [tab('tab-1', WORKTREE_ID), tab('tab-2', WORKTREE_ID)] + } + }) + }), + REPOS + ) + expect(read.session.tabsByWorktree[WORKTREE_ID] ?? [], `cycle ${cycle}`).toEqual([]) + } + }) + + it('converges to the latest closure without duplicate adoption when a survivor tab is progressively closed across cycles', async () => { + // Cycle A: the target is actively connected (an ordinary boot; nothing offline yet) with two + // tabs open. Both must adopt normally per the restart-regression fix. + const cycleA = await fetchWorkspaceSessionWithRuntimeHostOwners( + partitionedApi({ + local: session({ activeConnectionIdsAtShutdown: [TARGET_ID] }), + [SSH_HOST_ID]: session({ + tabsByWorktree: { + [WORKTREE_ID]: [tab('tab-1', WORKTREE_ID), tab('tab-2', WORKTREE_ID)] + } + }) + }), + REPOS + ) + expect(cycleA.session.tabsByWorktree[WORKTREE_ID]?.map((entry) => entry.id)).toEqual([ + 'tab-1', + 'tab-2' + ]) + + // Cycle B: the client goes offline (disconnects, then quits without reconnecting -- target + // drops out of activeConnectionIdsAtShutdown) and the server closes tab-2 while it is gone. + // The all-or-nothing worktree-level decline still protects the survivor tab-1's stale cached + // copy from being silently dropped by a later cycle: it stays in the parked/undeclined mirror + // rather than resurrecting a fresher truth this static merge cannot observe. + const cycleB = await fetchWorkspaceSessionWithRuntimeHostOwners( + partitionedApi({ + local: session({}), + [SSH_HOST_ID]: session({ + tabsByWorktree: { + [WORKTREE_ID]: [tab('tab-1', WORKTREE_ID), tab('tab-2', WORKTREE_ID)] + } + }) + }), + REPOS + ) + // Worktree-level decline: since the host mirror still names a real (non-empty) row for this + // id and the base has no row of its own, GAP-03 declines the whole worktree — the cache is + // simply not adopted this cycle, not partially merged into [tab-1] alone. + expect(cycleB.session.tabsByWorktree[WORKTREE_ID] ?? []).toEqual([]) + + // Cycle C: a third consecutive boot against the SAME stale, declined mirror. No duplicate + // adoption, no drift from cycle B's verdict, no corruption of the session shape. + const cycleC = await fetchWorkspaceSessionWithRuntimeHostOwners( + partitionedApi({ + local: session({}), + [SSH_HOST_ID]: session({ + tabsByWorktree: { + [WORKTREE_ID]: [tab('tab-1', WORKTREE_ID), tab('tab-2', WORKTREE_ID)] + } + }) + }), + REPOS + ) + expect(cycleC.session.tabsByWorktree[WORKTREE_ID] ?? []).toEqual([]) + expect(cycleC.session.tabsByWorktree).toEqual(cycleB.session.tabsByWorktree) + }) +}) diff --git a/src/renderer/src/lib/workspace-session-host-reconciliation.ts b/src/renderer/src/lib/workspace-session-host-reconciliation.ts index 43bf4cfcb023..e5a3325cc32b 100644 --- a/src/renderer/src/lib/workspace-session-host-reconciliation.ts +++ b/src/renderer/src/lib/workspace-session-host-reconciliation.ts @@ -182,16 +182,32 @@ function migratedSessionKeys(attribution: SshPartitionCatalogAttribution): Set { + if (wasConnectedAtLastShutdown) { + return new Set() + } const reconciled = new Set(attribution.confirmedSessionKeysByHostId.get(hostId)) for (const workspaceId of migratedSessionKeys(attribution)) { reconciled.delete(workspaceId) diff --git a/src/renderer/src/lib/workspace-session-host-write-race.test.ts b/src/renderer/src/lib/workspace-session-host-write-race.test.ts new file mode 100644 index 000000000000..ea9167bb512f --- /dev/null +++ b/src/renderer/src/lib/workspace-session-host-write-race.test.ts @@ -0,0 +1,211 @@ +/** + * GAP-03 Write Race Protection (revive_labs#962 / stablyai/orca#22038, PR #968 deferred scenario): + * does the client's own wake-triggered session write (`persistWorkspaceSessionByHost`) undo a + * server-side tab close it just correctly declined to adopt? + * + * `projects/orca-serve-tab-lifecycle/upstream/filing-drafts/GAP-03-offline-client-reconciliation.md` + * §2 row 6 ("Step 4: Close & Teardown") describes the defect as: "Client write race overwrites + * server-side tab close operations ... `patchWorkspaceSessionByHost` fires on client wake, racing + * against server event streams." The proposed fix (§5.2) was a `hasPendingReconnectionHandshake` + * write-gate on `persistWorkspaceSessionByHost` that defers any write until an authoritative + * reconnect handshake settles. + * + * This suite answers, with test evidence, whether that gate is actually necessary given the + * shipped fix's own mechanism: `adoptStrandedHostPartitionSession` never deletes a declined row — + * it leaves it in the host partition slice, and `partitionRowsTheWriteWontReturn` parks every + * row this read declined to adopt into `contestedHostWorkspaceSessions[hostId]`, which + * `attachHostSessionShadow` re-attaches into the SAME host's write-side slice on the next write — + * but ONLY if that host already has a slice in the write (i.e. something else this boot routes + * there). Whether a wake-triggered write actually resurrects a declined tab therefore depends on + * whether anything else makes `persistWorkspaceSessionByHost`/`patchWorkspaceSessionByHost` touch + * that specific host partition at all in the same call. + * + * Verdict, with the evidence below: `hasPendingReconnectionHandshake` is NOT unnecessary. The + * shipped fix has no write-side protection at all — nothing in `persistWorkspaceSessionByHost` or + * `patchWorkspaceSessionByHost` inspects whether the host partition being written to was ever + * declined-from on the read that produced this payload. The moment anything else legitimately + * writes to the same host partition in the same call (a sibling worktree gaining a tab, a folder + * workspace attaching to the same target, etc.), the declined row rides back in via the shadow and + * round-trips to the server and every other connected client — exactly the race the upstream draft + * describes. A handshake-style write-gate (or an equivalent per-write filter that excludes rows a + * recent read declined) is the only mechanism that would close it; the current fix has neither. + * A naive version of that filter was prototyped during this verification pass — unconditionally + * dropping (instead of parking) every `reconciledWorktreeIds`-declined row — and reverted: it + * closes this race for a worktree that really was closed while offline, but it is blind to the + * "Concurrent Active Edits" false positive (see workspace-session-host-offline-reconnect.test.ts's + * "documented boundary" case) — a live sibling worktree this fix ALSO declines merely for having + * no prior base row. Dropping that row instead of parking it converts a recoverable + * non-adoption into an unrecoverable delete of another client's live tabs on this host partition's + * next `replace`-mode write. Both cases are indistinguishable with the information this fix has; + * a correct fix needs the live manifest/handshake, not a blanket park-vs-drop policy. + */ +import { describe, expect, it } from 'vitest' +import { getDefaultWorkspaceSession } from '../../../shared/constants' +import type { ExecutionHostId } from '../../../shared/execution-host' +import { normalizeExecutionHostId } from '../../../shared/execution-host' +import type { TerminalTab } from '../../../shared/terminal-tab-types' +import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' +import { fetchWorkspaceSessionWithRuntimeHostOwners } from './workspace-session-host-hydration' +import { + persistWorkspaceSessionByHost, + type HostPersistenceState +} from './workspace-session-host-persistence' + +const TARGET_ID = 'target-1' +const SSH_HOST_ID: ExecutionHostId = `ssh:${TARGET_ID}` +const REPO_ID = 'repo-remote' +const WORKTREE_ID = `${REPO_ID}::/remote/checkout` +const SIBLING_REPO_ID = `${REPO_ID}-2` +const SIBLING_WORKTREE_ID = `${SIBLING_REPO_ID}::/remote/sibling` + +function session(overrides: Partial): WorkspaceSessionState { + return { ...getDefaultWorkspaceSession(), ...overrides } +} + +function tab(id: string, worktreeId: string): TerminalTab { + return { + id, + ptyId: `pty-${id}`, + worktreeId, + title: id, + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } +} + +function partitionedApi(partitions: Partial>) { + return { + get: async (hostId?: ExecutionHostId) => + partitions[hostId ?? 'local'] ?? getDefaultWorkspaceSession(), + listHostIds: async () => + Object.keys(partitions).flatMap((hostId) => normalizeExecutionHostId(hostId) ?? []) + } +} + +function capturingApi() { + const captured: Partial> = {} + return { + captured, + api: { + get: async () => getDefaultWorkspaceSession(), + patch: async () => {}, + setSync: () => {}, + set: async (payload: WorkspaceSessionState, hostId?: ExecutionHostId) => { + captured[hostId ?? 'local'] = payload + }, + flush: async () => {} + } + } +} + +describe('GAP-03 Write Race Protection', () => { + it('a wake-triggered write for a worktree with nothing else on the same host never even touches that host partition, so the declined tabs cannot round-trip back', async () => { + const read = await fetchWorkspaceSessionWithRuntimeHostOwners( + partitionedApi({ + local: session({}), + [SSH_HOST_ID]: session({ + tabsByWorktree: { + [WORKTREE_ID]: [tab('tab-2', WORKTREE_ID), tab('tab-3', WORKTREE_ID)] + } + }) + }), + [{ id: REPO_ID, connectionId: TARGET_ID, executionHostId: SSH_HOST_ID }] + ) + expect(read.session.tabsByWorktree[WORKTREE_ID] ?? []).toEqual([]) + + const { captured, api } = capturingApi() + const state: HostPersistenceState = { + repos: [{ id: REPO_ID, connectionId: TARGET_ID, executionHostId: SSH_HOST_ID }], + worktreesByRepo: { + [REPO_ID]: [ + { + id: WORKTREE_ID, + repoId: REPO_ID, + hostId: SSH_HOST_ID, + runtimeOwnerEnvironmentId: undefined + } + ] + }, + contestedHostWorkspaceSessions: read.contestedHostWorkspaceSessions, + contestedPrimaryHostBySessionKey: read.contestedPrimaryHostBySessionKey + } + + await persistWorkspaceSessionByHost(api as never, read.session, state) + + expect(captured[SSH_HOST_ID]).toBeUndefined() + }) + + it('a wake-triggered write DOES route the declined row back into the host partition once a sibling worktree on the same host legitimately writes there — this IS the write race the upstream draft describes, and answers the hasPendingReconnectionHandshake question: the gate is not unnecessary', async () => { + const read = await fetchWorkspaceSessionWithRuntimeHostOwners( + partitionedApi({ + local: session({}), + [SSH_HOST_ID]: session({ + tabsByWorktree: { + [WORKTREE_ID]: [tab('tab-2', WORKTREE_ID), tab('tab-3', WORKTREE_ID)], + [SIBLING_WORKTREE_ID]: [tab('tab-4', SIBLING_WORKTREE_ID)] + } + }) + }), + [ + { id: REPO_ID, connectionId: TARGET_ID, executionHostId: SSH_HOST_ID }, + { id: SIBLING_REPO_ID, connectionId: TARGET_ID, executionHostId: SSH_HOST_ID } + ] + ) + // Read-time behavior is correct: WORKTREE_ID declined, SIBLING_WORKTREE_ID also parked (the + // documented "Concurrent Active Edits" boundary — see workspace-session-host-offline-reconnect.test.ts). + expect(read.session.tabsByWorktree[WORKTREE_ID] ?? []).toEqual([]) + expect(read.session.tabsByWorktree[SIBLING_WORKTREE_ID]).toBeUndefined() + + // Now simulate the SAME host legitimately gaining a write target this boot: the user opens a + // brand-new local tab that main's own routing places on this SSH host (e.g. a fresh worktree + // the user just created there). This is enough to force a slice for SSH_HOST_ID to exist on + // the very next wake-triggered write. + const NEW_LOCAL_WORKTREE_ID = `${SIBLING_REPO_ID}::/remote/new-tab` + const payloadWithFreshLocalActivity: WorkspaceSessionState = { + ...read.session, + tabsByWorktree: { + ...read.session.tabsByWorktree, + [NEW_LOCAL_WORKTREE_ID]: [tab('tab-fresh', NEW_LOCAL_WORKTREE_ID)] + } + } + + const { captured, api } = capturingApi() + const state: HostPersistenceState = { + repos: [ + { id: REPO_ID, connectionId: TARGET_ID, executionHostId: SSH_HOST_ID }, + { id: SIBLING_REPO_ID, connectionId: TARGET_ID, executionHostId: SSH_HOST_ID } + ], + worktreesByRepo: { + [REPO_ID]: [ + { + id: WORKTREE_ID, + repoId: REPO_ID, + hostId: SSH_HOST_ID, + runtimeOwnerEnvironmentId: undefined + } + ], + [SIBLING_REPO_ID]: [ + { + id: NEW_LOCAL_WORKTREE_ID, + repoId: SIBLING_REPO_ID, + hostId: SSH_HOST_ID, + runtimeOwnerEnvironmentId: undefined + } + ] + }, + contestedHostWorkspaceSessions: read.contestedHostWorkspaceSessions, + contestedPrimaryHostBySessionKey: read.contestedPrimaryHostBySessionKey + } + + await persistWorkspaceSessionByHost(api as never, payloadWithFreshLocalActivity, state) + + // The write race, demonstrated: WORKTREE_ID's genuinely-closed tabs come back, even though the + // read that produced this exact payload correctly declined them moments earlier. + expect(captured[SSH_HOST_ID]?.tabsByWorktree?.[WORKTREE_ID]?.map((entry) => entry.id)).toEqual([ + 'tab-2', + 'tab-3' + ]) + }) +}) diff --git a/tests/e2e/gap-03-offline-reconnect-tab-resurrection.spec.ts b/tests/e2e/gap-03-offline-reconnect-tab-resurrection.spec.ts index b33bc8904c25..e842f9664a90 100644 --- a/tests/e2e/gap-03-offline-reconnect-tab-resurrection.spec.ts +++ b/tests/e2e/gap-03-offline-reconnect-tab-resurrection.spec.ts @@ -313,3 +313,82 @@ test.describe('GAP-03: offline client reconnect must not resurrect server-closed } }) }) + +test.describe('GAP-03 regression: an ordinary restart must not drop live tabs (Docker cross-site)', () => { + test.skip(!RUN_DOCKER_SSH, 'Set ORCA_E2E_SSH_DOCKER=1 to run Docker-backed GAP-03 tests.') + test.skip(process.platform === 'win32', 'Docker SSH restore uses POSIX SSH tooling.') + + test('tabs survive an ordinary quit + relaunch with nothing closed and the connection still active at shutdown', async (// oxlint-disable-next-line no-empty-pattern -- this test owns both Electron launches. + {}, testInfo) => { + test.setTimeout(300_000) + const session = createRestartSession(testInfo) + const target = startDockerSshRelayTarget(testInfo) + let firstApp: ElectronApplication | null = null + let secondApp: ElectronApplication | null = null + + try { + const first = await session.launch() + firstApp = first.app + await waitForSessionReady(first.page) + const remote = await connectDockerSshRelayTarget(first.page, target) + await expect + .poll(() => waitForActiveWorktree(first.page), { timeout: 30_000 }) + .toBe(remote.worktreeId) + await waitForActiveTerminalManager(first.page, 60_000) + await waitForActivePanePtyId(first.page, 60_000) + await createRemoteTerminalTab(first.page, remote.worktreeId) + const openedTabs = await readRemoteTerminalTabs(first.page, remote.worktreeId) + expect(openedTabs.length, 'setup must open exactly 2 tabs').toBe(2) + + // --- Ordinary quit: still connected, nothing closed, no fault injection whatsoever. This + // is what the vast majority of real restarts look like, and is the exact case the + // `reconciledWorktreeIds` decline path silently broke before this slice: `remote-repo- + // registration.ts` stamps `executionHostId` on this repo (the modern, default shape), and + // an SSH worktree's `tabsByWorktree` row never lands in the local base row at all -- so + // "the base has nothing for this worktree" is true on EVERY boot, closed or not, and was + // read as "the server's confirmed zero tabs" regardless of whether the target was ever + // actually offline. --- + await session.close(firstApp) + firstApp = null + + const sshHostId = toSshExecutionHostId(remote.targetId) + const profile = readProfile(session.userDataDir) + expect( + profile.workspaceSession.activeConnectionIdsAtShutdown ?? [], + 'the connection must still be recorded as active at shutdown -- this was never an offline gap' + ).toContain(remote.targetId) + const hostMirrorTabs = + profile.workspaceSessionsByHostId?.[sshHostId]?.tabsByWorktree?.[remote.worktreeId] + expect(hostMirrorTabs?.length, 'both tabs must be on disk before relaunch').toBe(2) + logKpi('ordinary-restart-snapshot', { + sshHostId, + activeConnectionIdsAtShutdown: profile.workspaceSession.activeConnectionIdsAtShutdown, + hostMirrorTabs: hostMirrorTabs?.length ?? 0 + }) + + // --- Relaunch against the SAME, completely unmodified profile. --- + const second = await session.launch() + secondApp = second.app + await waitForSessionReady(second.page) + + const afterTabs = await readRemoteTerminalTabs(second.page, remote.worktreeId) + logKpi('ordinary-restart-post-relaunch', { + expectedCount: openedTabs.length, + actualCount: afterTabs.length, + actualIds: afterTabs.map((t) => t.id) + }) + expect( + afterTabs.map((t) => t.id).sort(), + 'GAP-03 regression: an ordinary restart with nothing closed must not drop any live tab' + ).toEqual(openedTabs.map((t) => t.id).sort()) + } finally { + for (const app of [secondApp, firstApp]) { + if (app) { + await session.close(app).catch(() => undefined) + } + } + await session.dispose() + cleanupDockerSshRelayTarget(target) + } + }) +}) From 117fe6324f81bd2784141c4b85d02ab0a08dd257 Mon Sep 17 00:00:00 2001 From: Lesley Murfin Date: Tue, 22 Sep 2026 12:00:51 -0600 Subject: [PATCH 06/23] feat(GAP-03): extract remote-workspace host testimony predicate (slice 1) - Add remote-workspace-host-testimony.ts with RemoteWorkspaceTestimonyState and hostHasAnsweredForTarget - Add remote-workspace-host-testimony.test.ts covering unhydrated, conflict, and non-conflict synced states - Update workspace-terminal-host-authority.ts to compose RemoteWorkspaceTestimonyState and delegate to hostHasAnsweredForTarget (behavior-neutral) --- .../remote-workspace-host-testimony.test.ts | 46 +++++++++++++++++++ .../lib/remote-workspace-host-testimony.ts | 20 ++++++++ .../lib/workspace-terminal-host-authority.ts | 13 +++--- 3 files changed, 73 insertions(+), 6 deletions(-) create mode 100644 src/renderer/src/lib/remote-workspace-host-testimony.test.ts create mode 100644 src/renderer/src/lib/remote-workspace-host-testimony.ts diff --git a/src/renderer/src/lib/remote-workspace-host-testimony.test.ts b/src/renderer/src/lib/remote-workspace-host-testimony.test.ts new file mode 100644 index 000000000000..b4a2df112a03 --- /dev/null +++ b/src/renderer/src/lib/remote-workspace-host-testimony.test.ts @@ -0,0 +1,46 @@ +import { describe, expect, it } from 'vitest' +import { + hostHasAnsweredForTarget, + type RemoteWorkspaceTestimonyState +} from './remote-workspace-host-testimony' + +const TARGET_ID = 'ssh-target-1' + +describe('hostHasAnsweredForTarget', () => { + it('is false until a snapshot has landed for the target', () => { + const emptyState: RemoteWorkspaceTestimonyState = {} + expect(hostHasAnsweredForTarget(emptyState, TARGET_ID)).toBe(false) + + const pullingState: RemoteWorkspaceTestimonyState = { + remoteWorkspaceSyncStatusByTargetId: { + [TARGET_ID]: { phase: 'pulling', direction: 'pull' } + } + } + expect(hostHasAnsweredForTarget(pullingState, TARGET_ID)).toBe(false) + }) + + it('is false while the landed snapshot is in conflict, because the client picture is not the host picture', () => { + const conflictState: RemoteWorkspaceTestimonyState = { + remoteWorkspaceHydratedTargetIds: new Set([TARGET_ID]), + remoteWorkspaceSyncStatusByTargetId: { + [TARGET_ID]: { phase: 'conflict' } + } + } + expect(hostHasAnsweredForTarget(conflictState, TARGET_ID)).toBe(false) + }) + + it('is true once a non-conflicting snapshot has landed', () => { + const syncedState: RemoteWorkspaceTestimonyState = { + remoteWorkspaceHydratedTargetIds: new Set([TARGET_ID]), + remoteWorkspaceSyncStatusByTargetId: { + [TARGET_ID]: { phase: 'synced', direction: 'pull' } + } + } + expect(hostHasAnsweredForTarget(syncedState, TARGET_ID)).toBe(true) + + const noStatusState: RemoteWorkspaceTestimonyState = { + remoteWorkspaceHydratedTargetIds: new Set([TARGET_ID]) + } + expect(hostHasAnsweredForTarget(noStatusState, TARGET_ID)).toBe(true) + }) +}) diff --git a/src/renderer/src/lib/remote-workspace-host-testimony.ts b/src/renderer/src/lib/remote-workspace-host-testimony.ts new file mode 100644 index 000000000000..ad50449338f2 --- /dev/null +++ b/src/renderer/src/lib/remote-workspace-host-testimony.ts @@ -0,0 +1,20 @@ +import type { RemoteWorkspaceSyncStatus } from '@/store/slices/ssh' + +export type RemoteWorkspaceTestimonyState = { + remoteWorkspaceHydratedTargetIds?: ReadonlySet + remoteWorkspaceSyncStatusByTargetId?: Record +} + +/** + * The pair `use-app-session-persistence` gates uploads on: a snapshot has landed for this target + * and it did not conflict, so the client's picture IS the host's. + */ +export function hostHasAnsweredForTarget( + state: RemoteWorkspaceTestimonyState, + targetId: string +): boolean { + return ( + state.remoteWorkspaceHydratedTargetIds?.has(targetId) === true && + state.remoteWorkspaceSyncStatusByTargetId?.[targetId]?.phase !== 'conflict' + ) +} diff --git a/src/renderer/src/lib/workspace-terminal-host-authority.ts b/src/renderer/src/lib/workspace-terminal-host-authority.ts index 9068280c4379..030e29e3309d 100644 --- a/src/renderer/src/lib/workspace-terminal-host-authority.ts +++ b/src/renderer/src/lib/workspace-terminal-host-authority.ts @@ -4,7 +4,10 @@ import { normalizeRuntimePathForComparison } from '../../../shared/cross-platfor import { parseWorkspaceKey } from '../../../shared/workspace-scope' import { splitWorktreeId } from '../../../shared/worktree/id' import type { HostLiveTerminalProbeVerdict } from '@/runtime/host-live-terminal-probe' -import type { RemoteWorkspaceSyncStatus } from '@/store/slices/ssh' +import { + hostHasAnsweredForTarget, + type RemoteWorkspaceTestimonyState +} from '@/lib/remote-workspace-host-testimony' import { isWebRuntimeSessionActive } from '@/runtime/web-runtime-session' import { getExecutionHostIdForWorktree, @@ -38,10 +41,8 @@ import { */ export type WorkspaceTerminalHostAuthority = HostLiveTerminalProbeVerdict -export type WorkspaceTerminalHostAuthorityState = WorktreeRuntimeOwnerState & { - remoteWorkspaceHydratedTargetIds?: ReadonlySet - remoteWorkspaceSyncStatusByTargetId?: Record -} +export type WorkspaceTerminalHostAuthorityState = WorktreeRuntimeOwnerState & + RemoteWorkspaceTestimonyState /** A sync attempt that has stopped without an answer. `unverifiable` is the honest verdict about the * host, but holding it forever is not a verdict — it is a refusal to act, so nothing would ever @@ -69,7 +70,7 @@ function resolveDirectSshAuthority( // Why: the same pair use-app-session-persistence.ts gates uploads on. A conflicting snapshot // means the client's picture is not the host's, so it is no basis for deciding the host holds // nothing. - return phase === 'conflict' ? 'unverifiable' : 'none' + return hostHasAnsweredForTarget(state, targetId) ? 'none' : 'unverifiable' } if (phase !== undefined && TERMINATED_WITHOUT_ANSWER_PHASES.has(phase)) { // The bounded floor. Without it a single failed sync leaves every git worktree on this target From 24e85a09e69a0868c8b155a7f3994d9898a9e214 Mon Sep 17 00:00:00 2001 From: Lesley Murfin Date: Tue, 22 Sep 2026 12:07:11 -0600 Subject: [PATCH 07/23] feat(GAP-03): withhold answered shadow rows on write path (slice 2) - Add workspace-session-host-shadow-testimony.ts implementing shadowRowsTheHostHasNotAnswered - Export WORKTREE_KEYED_FIELDS from workspace-session-host-contention.ts - Extend HostPersistenceState with RemoteWorkspaceTestimonyState and wire shadow filtering into splitWorkspaceSessionForWrite in workspace-session-host-persistence.ts - Add write-race protection test, retitle existing test, and update docstring verdict in workspace-session-host-write-race.test.ts --- .../lib/workspace-session-host-contention.ts | 2 +- .../lib/workspace-session-host-persistence.ts | 11 ++- ...workspace-session-host-shadow-testimony.ts | 91 ++++++++++++++++++ .../workspace-session-host-write-race.test.ts | 94 +++++++++++++++---- 4 files changed, 176 insertions(+), 22 deletions(-) create mode 100644 src/renderer/src/lib/workspace-session-host-shadow-testimony.ts diff --git a/src/renderer/src/lib/workspace-session-host-contention.ts b/src/renderer/src/lib/workspace-session-host-contention.ts index 797a2928766c..48162e5ad2e6 100644 --- a/src/renderer/src/lib/workspace-session-host-contention.ts +++ b/src/renderer/src/lib/workspace-session-host-contention.ts @@ -43,7 +43,7 @@ import { export type WorktreeHostClaims = ReadonlyMap> -const WORKTREE_KEYED_FIELDS = ( +export const WORKTREE_KEYED_FIELDS = ( Object.keys(WORKSPACE_SESSION_FIELD_OWNERSHIP) as (keyof WorkspaceSessionState)[] ).filter((field) => WORKSPACE_SESSION_FIELD_OWNERSHIP[field] === 'worktreeKeyed') diff --git a/src/renderer/src/lib/workspace-session-host-persistence.ts b/src/renderer/src/lib/workspace-session-host-persistence.ts index 909acdd4d69f..dbb68910b61f 100644 --- a/src/renderer/src/lib/workspace-session-host-persistence.ts +++ b/src/renderer/src/lib/workspace-session-host-persistence.ts @@ -36,6 +36,8 @@ import { extendWorktreeIdByTabId, type WorkspaceTabOwnerCatalog } from '../../../shared/workspace-session-host-records' +import type { RemoteWorkspaceTestimonyState } from './remote-workspace-host-testimony' +import { shadowRowsTheHostHasNotAnswered } from './workspace-session-host-shadow-testimony' export type HostPersistenceState = WorkspaceTabOwnerCatalog & { repos: readonly Pick[] @@ -53,7 +55,7 @@ export type HostPersistenceState = WorkspaceTabOwnerCatalog & { /** Partition each restored session key was read from. Routing honours it so a write returns rows * to their own partition instead of re-deriving an owner the read never agreed to. */ contestedPrimaryHostBySessionKey?: Record -} +} & RemoteWorkspaceTestimonyState type SessionApi = { get: (hostId?: ExecutionHostId) => Promise @@ -227,7 +229,12 @@ function splitWorkspaceSessionForWrite( const slices = splitWorkspaceSessionByHost(payload, routing.hostIdByWorktreeId, { worktreeIdByTabId }) - attachHostSessionShadow(slices, state.contestedHostWorkspaceSessions, routing.claims, mode) + attachHostSessionShadow( + slices, + shadowRowsTheHostHasNotAnswered(state.contestedHostWorkspaceSessions, state), + routing.claims, + mode + ) return slices } diff --git a/src/renderer/src/lib/workspace-session-host-shadow-testimony.ts b/src/renderer/src/lib/workspace-session-host-shadow-testimony.ts new file mode 100644 index 000000000000..d3438d59fcfa --- /dev/null +++ b/src/renderer/src/lib/workspace-session-host-shadow-testimony.ts @@ -0,0 +1,91 @@ +import type { ExecutionHostId } from '../../../shared/execution-host' +import { parseExecutionHostId } from '../../../shared/execution-host' +import { parseWorkspaceKey } from '../../../shared/workspace-scope' +import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' +import { isWorkspaceSessionRecord } from '../../../shared/workspace-session-host-records' +import { WORKTREE_KEYED_FIELDS } from './workspace-session-host-contention' +import type { HostSessionSlices } from './workspace-session-host-split' +import { + hostHasAnsweredForTarget, + type RemoteWorkspaceTestimonyState +} from './remote-workspace-host-testimony' + +/** + * The shadow minus every parked row whose host has already answered for it. Returns the input by + * reference when nothing is withheld. + */ +export function shadowRowsTheHostHasNotAnswered( + shadow: HostSessionSlices | undefined, + state: RemoteWorkspaceTestimonyState +): HostSessionSlices | undefined { + if (!shadow) { + return shadow + } + + let withheld = false + const nextShadow: HostSessionSlices = {} + + for (const [hostId, hostSlice] of Object.entries(shadow) as [ + ExecutionHostId, + WorkspaceSessionState | undefined + ][]) { + if (!hostSlice) { + nextShadow[hostId] = hostSlice + continue + } + + const parsed = parseExecutionHostId(hostId) + if (parsed?.kind !== 'ssh' || !hostHasAnsweredForTarget(state, parsed.targetId)) { + nextShadow[hostId] = hostSlice + continue + } + + let hostWithheld = false + let survivingWorktreeKeyedFieldCount = 0 + const nextHostSlice: WorkspaceSessionState = { ...hostSlice } + + for (const field of WORKTREE_KEYED_FIELDS) { + const record = hostSlice[field] + if (!isWorkspaceSessionRecord(record)) { + continue + } + + let fieldWithheld = false + const survivingRecord: Record = {} + + for (const [key, value] of Object.entries(record)) { + if (parseWorkspaceKey(key)?.type === 'folder') { + survivingRecord[key] = value + } else { + fieldWithheld = true + } + } + + if (fieldWithheld) { + hostWithheld = true + withheld = true + if (Object.keys(survivingRecord).length > 0) { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: field is a worktree-keyed record field and survivingRecord contains only its surviving entries. + ;(nextHostSlice as Record)[field] = survivingRecord + survivingWorktreeKeyedFieldCount++ + } else { + delete nextHostSlice[field] + } + } else { + if (Object.keys(record).length > 0) { + survivingWorktreeKeyedFieldCount++ + } + } + } + + if (hostWithheld) { + if (survivingWorktreeKeyedFieldCount > 0) { + nextShadow[hostId] = nextHostSlice + } + } else { + nextShadow[hostId] = hostSlice + } + } + + return withheld ? nextShadow : shadow +} diff --git a/src/renderer/src/lib/workspace-session-host-write-race.test.ts b/src/renderer/src/lib/workspace-session-host-write-race.test.ts index ea9167bb512f..19bb12f6adda 100644 --- a/src/renderer/src/lib/workspace-session-host-write-race.test.ts +++ b/src/renderer/src/lib/workspace-session-host-write-race.test.ts @@ -20,24 +20,13 @@ * whether anything else makes `persistWorkspaceSessionByHost`/`patchWorkspaceSessionByHost` touch * that specific host partition at all in the same call. * - * Verdict, with the evidence below: `hasPendingReconnectionHandshake` is NOT unnecessary. The - * shipped fix has no write-side protection at all — nothing in `persistWorkspaceSessionByHost` or - * `patchWorkspaceSessionByHost` inspects whether the host partition being written to was ever - * declined-from on the read that produced this payload. The moment anything else legitimately - * writes to the same host partition in the same call (a sibling worktree gaining a tab, a folder - * workspace attaching to the same target, etc.), the declined row rides back in via the shadow and - * round-trips to the server and every other connected client — exactly the race the upstream draft - * describes. A handshake-style write-gate (or an equivalent per-write filter that excludes rows a - * recent read declined) is the only mechanism that would close it; the current fix has neither. - * A naive version of that filter was prototyped during this verification pass — unconditionally - * dropping (instead of parking) every `reconciledWorktreeIds`-declined row — and reverted: it - * closes this race for a worktree that really was closed while offline, but it is blind to the - * "Concurrent Active Edits" false positive (see workspace-session-host-offline-reconnect.test.ts's - * "documented boundary" case) — a live sibling worktree this fix ALSO declines merely for having - * no prior base row. Dropping that row instead of parking it converts a recoverable - * non-adoption into an unrecoverable delete of another client's live tabs on this host partition's - * next `replace`-mode write. Both cases are indistinguishable with the information this fix has; - * a correct fix needs the live manifest/handshake, not a blanket park-vs-drop policy. + * Verdict, resolved: write-race protection is provided by `shadowRowsTheHostHasNotAnswered` + * (`workspace-session-host-shadow-testimony.ts`), which withholds parked rows from the write-side + * shadow once the host has answered for that target (positive testimony via landed, non-conflicting + * remote-workspace hydration). While the host has NOT answered, preserving parked rows remains the + * only safe move — this file stands guard against re-introducing the reverted naive drop that + * unconditionally dropped parked rows without positive host testimony and caused unrecoverable data + * loss on Concurrent Active Edits. */ import { describe, expect, it } from 'vitest' import { getDefaultWorkspaceSession } from '../../../shared/constants' @@ -137,7 +126,7 @@ describe('GAP-03 Write Race Protection', () => { expect(captured[SSH_HOST_ID]).toBeUndefined() }) - it('a wake-triggered write DOES route the declined row back into the host partition once a sibling worktree on the same host legitimately writes there — this IS the write race the upstream draft describes, and answers the hasPendingReconnectionHandshake question: the gate is not unnecessary', async () => { + it('still routes a declined row back into the host partition while the host has not answered for that target — preserving is the only safe move when nothing has superseded the parked verdict', async () => { const read = await fetchWorkspaceSessionWithRuntimeHostOwners( partitionedApi({ local: session({}), @@ -208,4 +197,71 @@ describe('GAP-03 Write Race Protection', () => { 'tab-3' ]) }) + + it('stops routing a declined row back into the host partition once the host has answered for that target', async () => { + const read = await fetchWorkspaceSessionWithRuntimeHostOwners( + partitionedApi({ + local: session({}), + [SSH_HOST_ID]: session({ + tabsByWorktree: { + [WORKTREE_ID]: [tab('tab-2', WORKTREE_ID), tab('tab-3', WORKTREE_ID)], + [SIBLING_WORKTREE_ID]: [tab('tab-4', SIBLING_WORKTREE_ID)] + } + }) + }), + [ + { id: REPO_ID, connectionId: TARGET_ID, executionHostId: SSH_HOST_ID }, + { id: SIBLING_REPO_ID, connectionId: TARGET_ID, executionHostId: SSH_HOST_ID } + ] + ) + expect(read.session.tabsByWorktree[WORKTREE_ID] ?? []).toEqual([]) + expect(read.session.tabsByWorktree[SIBLING_WORKTREE_ID]).toBeUndefined() + + const NEW_LOCAL_WORKTREE_ID = `${SIBLING_REPO_ID}::/remote/new-tab` + const payloadWithFreshLocalActivity: WorkspaceSessionState = { + ...read.session, + tabsByWorktree: { + ...read.session.tabsByWorktree, + [NEW_LOCAL_WORKTREE_ID]: [tab('tab-fresh', NEW_LOCAL_WORKTREE_ID)] + } + } + + const { captured, api } = capturingApi() + const state: HostPersistenceState = { + repos: [ + { id: REPO_ID, connectionId: TARGET_ID, executionHostId: SSH_HOST_ID }, + { id: SIBLING_REPO_ID, connectionId: TARGET_ID, executionHostId: SSH_HOST_ID } + ], + worktreesByRepo: { + [REPO_ID]: [ + { + id: WORKTREE_ID, + repoId: REPO_ID, + hostId: SSH_HOST_ID, + runtimeOwnerEnvironmentId: undefined + } + ], + [SIBLING_REPO_ID]: [ + { + id: NEW_LOCAL_WORKTREE_ID, + repoId: SIBLING_REPO_ID, + hostId: SSH_HOST_ID, + runtimeOwnerEnvironmentId: undefined + } + ] + }, + contestedHostWorkspaceSessions: read.contestedHostWorkspaceSessions, + contestedPrimaryHostBySessionKey: read.contestedPrimaryHostBySessionKey, + remoteWorkspaceHydratedTargetIds: new Set([TARGET_ID]), + remoteWorkspaceSyncStatusByTargetId: { [TARGET_ID]: { phase: 'synced' } } + } + + await persistWorkspaceSessionByHost(api as never, payloadWithFreshLocalActivity, state) + + expect(captured[SSH_HOST_ID]?.tabsByWorktree?.[WORKTREE_ID]).toBeUndefined() + expect(captured[SSH_HOST_ID]?.tabsByWorktree?.[SIBLING_WORKTREE_ID]).toBeUndefined() + expect( + captured[SSH_HOST_ID]?.tabsByWorktree?.[NEW_LOCAL_WORKTREE_ID]?.map((e) => e.id) + ).toEqual(['tab-fresh']) + }) }) From 6d9dde7dcab1ce362644afe61471952e6ef8a40a Mon Sep 17 00:00:00 2001 From: Lesley Murfin Date: Tue, 22 Sep 2026 12:12:34 -0600 Subject: [PATCH 08/23] test(GAP-03): edge-case hardening tests for shadowRowsTheHostHasNotAnswered (slice 3) --- ...pace-session-host-shadow-testimony.test.ts | 144 ++++++++++++++++++ 1 file changed, 144 insertions(+) create mode 100644 src/renderer/src/lib/workspace-session-host-shadow-testimony.test.ts diff --git a/src/renderer/src/lib/workspace-session-host-shadow-testimony.test.ts b/src/renderer/src/lib/workspace-session-host-shadow-testimony.test.ts new file mode 100644 index 000000000000..121a8881d79b --- /dev/null +++ b/src/renderer/src/lib/workspace-session-host-shadow-testimony.test.ts @@ -0,0 +1,144 @@ +import { describe, expect, it } from 'vitest' +import { getDefaultWorkspaceSession } from '../../../shared/constants' +import { + toRuntimeExecutionHostId, + toSshExecutionHostId, + type ExecutionHostId +} from '../../../shared/execution-host' +import type { TerminalTab } from '../../../shared/terminal-tab-types' +import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' +import type { RemoteWorkspaceTestimonyState } from './remote-workspace-host-testimony' +import { shadowRowsTheHostHasNotAnswered } from './workspace-session-host-shadow-testimony' +import type { HostSessionSlices } from './workspace-session-host-split' + +const TARGET_ID = 'target-1' +const SSH_HOST_ID: ExecutionHostId = toSshExecutionHostId(TARGET_ID) + +function tab(id: string, worktreeId: string): TerminalTab { + return { + id, + ptyId: null, + worktreeId, + title: id, + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 1 + } +} + +function sessionWithTabs(entries: Record): WorkspaceSessionState { + return { ...getDefaultWorkspaceSession(), tabsByWorktree: entries } +} + +describe('shadowRowsTheHostHasNotAnswered', () => { + it('keeps a parked folder workspace even after the host answers, because the direct-SSH snapshot never replaces folder rows', () => { + const answeredState: RemoteWorkspaceTestimonyState = { + remoteWorkspaceHydratedTargetIds: new Set([TARGET_ID]), + remoteWorkspaceSyncStatusByTargetId: { + [TARGET_ID]: { phase: 'synced', direction: 'pull' } + } + } + + const folderKey = 'folder:folder-uuid-1' + const worktreeKey = 'worktree:repo-1::worktree-1' + const folderTabs = [tab('tab-folder', folderKey)] + const worktreeTabs = [tab('tab-worktree', worktreeKey)] + + const shadow: HostSessionSlices = { + [SSH_HOST_ID]: sessionWithTabs({ + [folderKey]: folderTabs, + [worktreeKey]: worktreeTabs + }) + } + + const result = shadowRowsTheHostHasNotAnswered(shadow, answeredState) + + expect(result).toBeDefined() + const hostSlice = result?.[SSH_HOST_ID] + expect(hostSlice).toBeDefined() + expect(hostSlice?.tabsByWorktree).toEqual({ + [folderKey]: folderTabs + }) + expect(hostSlice?.tabsByWorktree).not.toHaveProperty(worktreeKey) + }) + + it('keeps parked rows while the landed snapshot is in conflict', () => { + const conflictState: RemoteWorkspaceTestimonyState = { + remoteWorkspaceHydratedTargetIds: new Set([TARGET_ID]), + remoteWorkspaceSyncStatusByTargetId: { + [TARGET_ID]: { phase: 'conflict' } + } + } + + const worktreeKey = 'worktree:repo-1::worktree-1' + const worktreeTabs = [tab('tab-worktree', worktreeKey)] + + const shadow: HostSessionSlices = { + [SSH_HOST_ID]: sessionWithTabs({ + [worktreeKey]: worktreeTabs + }) + } + + const result = shadowRowsTheHostHasNotAnswered(shadow, conflictState) + + // When the snapshot is in conflict, the host has not answered, so shadow is kept untouched by reference + expect(result).toBe(shadow) + expect(result?.[SSH_HOST_ID]?.tabsByWorktree).toEqual({ + [worktreeKey]: worktreeTabs + }) + }) + + it('keeps parked rows for a runtime host, which publishes no remote-workspace snapshot', () => { + const envId = 'runtime-env-1' + const runtimeHostId: ExecutionHostId = toRuntimeExecutionHostId(envId) + + // Even if runtime id happened to be in testimony state, runtime hosts do not use direct-SSH snapshots + const state: RemoteWorkspaceTestimonyState = { + remoteWorkspaceHydratedTargetIds: new Set([envId]), + remoteWorkspaceSyncStatusByTargetId: { + [envId]: { phase: 'synced', direction: 'pull' } + } + } + + const worktreeKey = 'worktree:repo-1::worktree-1' + const worktreeTabs = [tab('tab-runtime', worktreeKey)] + + const shadow: HostSessionSlices = { + [runtimeHostId]: sessionWithTabs({ + [worktreeKey]: worktreeTabs + }) + } + + const result = shadowRowsTheHostHasNotAnswered(shadow, state) + + expect(result).toBe(shadow) + expect(result?.[runtimeHostId]?.tabsByWorktree).toEqual({ + [worktreeKey]: worktreeTabs + }) + }) + + it('drops the host entry entirely when every parked row on it was answered for', () => { + const answeredState: RemoteWorkspaceTestimonyState = { + remoteWorkspaceHydratedTargetIds: new Set([TARGET_ID]), + remoteWorkspaceSyncStatusByTargetId: { + [TARGET_ID]: { phase: 'synced', direction: 'pull' } + } + } + + const worktreeKey = 'worktree:repo-1::worktree-1' + const worktreeTabs = [tab('tab-worktree', worktreeKey)] + + const shadow: HostSessionSlices = { + [SSH_HOST_ID]: sessionWithTabs({ + [worktreeKey]: worktreeTabs + }) + } + + const result = shadowRowsTheHostHasNotAnswered(shadow, answeredState) + + expect(result).toBeDefined() + expect(result?.[SSH_HOST_ID]).toBeUndefined() + expect(Object.keys(result ?? {})).toEqual([]) + }) +}) From 1719be65f91dbbd9391bfdbbfc601e81d6b3a4f0 Mon Sep 17 00:00:00 2001 From: Lesley Murfin Date: Tue, 22 Sep 2026 21:17:32 -0600 Subject: [PATCH 09/23] fix(GAP-03): normalize canonical worktree: keys in stranded-partition decline loop CodeRabbit finding: the reconciled-decline loop in adoptStrandedHostPartitionSession read host.tabsByWorktree[workspaceId] with the bare normalized workspaceId, but tabsByWorktree keys may be stored in canonical worktree:${workspaceId} form (the type's own doc comment says so). The adoption loop a few lines down already normalizes every key before comparing; the decline loop didn't, so a canonical-keyed host row silently defeated GAP-03's whole decline and adopted stale tabs it should have declined. Red test added first (workspace-session-host-offline-reconnect.test.ts): a host partition with tabsByWorktree keyed as `worktree:${WORKTREE_ID}` under the confirmed-disconnected-at-shutdown GAP-03 precondition. Failed before the fix (both tabs survived, just under the canonical key instead of the bare one, so the naive same-key assertion would have missed it -- the test walks every surviving tab across every key). Fix: walk host.tabsByWorktree entries and normalize each key via normalizeWorkspaceSessionKeyToWorkspaceId before matching against workspaceId, matching the adoption loop's own pattern. 8/8 in the touched suite green; 75/75 across the 7 directly-dependent partition/ reconciliation suites green. --- ...ace-session-host-offline-reconnect.test.ts | 26 +++++++++++++++++++ ...ace-session-stranded-partition-adoption.ts | 11 +++++--- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/src/renderer/src/lib/workspace-session-host-offline-reconnect.test.ts b/src/renderer/src/lib/workspace-session-host-offline-reconnect.test.ts index 088efa7ed2fb..59cdf93875ec 100644 --- a/src/renderer/src/lib/workspace-session-host-offline-reconnect.test.ts +++ b/src/renderer/src/lib/workspace-session-host-offline-reconnect.test.ts @@ -85,6 +85,32 @@ describe('GAP-03: offline client reconnect must not resurrect server-closed tabs }) }) +describe('GAP-03 regression: canonical-keyed host rows must still be declined', () => { + it('declines a stale tab row even when the host partition stores it under a canonical `worktree:` key', async () => { + // The host partition's `tabsByWorktree` type comment says keys "may be legacy raw worktree IDs + // or canonical WorkspaceKey values" (`worktree:${worktreeId}`). The decline loop in + // `adoptStrandedHostPartitionSession` (workspace-session-stranded-partition-adoption.ts) reads + // `host.tabsByWorktree[workspaceId]` with the bare id, so a host row keyed canonically is + // invisible to it -- the row silently defeats the whole decline and its stale tabs get adopted. + const read = await fetchWorkspaceSessionWithRuntimeHostOwners( + partitionedApi({ + local: session({}), + [SSH_HOST_ID]: session({ + tabsByWorktree: { + [`worktree:${WORKTREE_ID}`]: [tab('tab-2', WORKTREE_ID), tab('tab-3', WORKTREE_ID)] + } + }) + }), + [{ id: REPO_ID, connectionId: TARGET_ID, executionHostId: SSH_HOST_ID }] + ) + + const survivingTabIds = Object.entries(read.session.tabsByWorktree ?? {}) + .flatMap(([, tabs]) => tabs) + .map((entry) => entry.id) + expect(survivingTabIds).toEqual([]) + }) +}) + describe('GAP-03 non-regression: #12721 offline-created local work must survive reconnect', () => { it('keeps a genuinely offline-created tab that was never synced to the host partition', async () => { // This client created a tab entirely offline — the host partition (mirroring the last state diff --git a/src/shared/workspace-session-stranded-partition-adoption.ts b/src/shared/workspace-session-stranded-partition-adoption.ts index 8beb77ba2cf2..cd0c5f97782f 100644 --- a/src/shared/workspace-session-stranded-partition-adoption.ts +++ b/src/shared/workspace-session-stranded-partition-adoption.ts @@ -378,13 +378,18 @@ export function adoptStrandedHostPartitionSession( // truth (zero tabs), not a blind spot — so a host row that still names real tabs here is this // client's stale pre-disconnect cache, not evidence of anything live. Declining leaves the row // in place for `partitionRowsTheWriteWontReturn` to park rather than deleting it. + const hostTabsByWorktree = host.tabsByWorktree ?? {} for (const workspaceId of options.reconciledWorktreeIds ?? []) { if (!adoptable.has(workspaceId) || Object.hasOwn(base.tabsByWorktree ?? {}, workspaceId)) { continue } - const hostTabs = (host.tabsByWorktree ?? {})[workspaceId] - if (Array.isArray(hostTabs) && hostTabs.length > 0) { - adoptable.delete(workspaceId) + for (const [key, hostTabs] of Object.entries(hostTabsByWorktree)) { + if (normalizeWorkspaceSessionKeyToWorkspaceId(key) !== workspaceId) { + continue + } + if (Array.isArray(hostTabs) && hostTabs.length > 0) { + adoptable.delete(workspaceId) + } } } if (adoptable.size === 0) { From d6c3af94a78205b551dcae7e899039a7ca41faf5 Mon Sep 17 00:00:00 2001 From: Lesley Murfin Date: Tue, 22 Sep 2026 22:12:33 -0600 Subject: [PATCH 10/23] fix(GAP-03): park and restore declined tab's dependent rows alongside worktree rows CodeRabbit finding: partitionRowsTheWriteWontReturn only walked WORKTREE_KEYED_FIELDS for a declined worktree, leaving tab-keyed (terminalLayoutsByTabId, remoteSessionIdsByTabId, localOnlyScrollbackByTabId) and pane-keyed (terminalPtyIncarnationsByPaneKey) rows unparked. If another workspace caused a write to the same SSH partition before host testimony arrived, the write retained the declined tabs but dropped their layout/session rows, leaving inconsistent state. Fix: 1. partitionRowsTheWriteWontReturn now maps each tab- and pane-keyed entry to its worktree via buildWorktreeIdByTabId / worktreeIdForPaneKey and parks rows whose owning worktree was declined. 2. attachHostSessionShadow and shadowRowsTheHostHasNotAnswered now iterate PARKABLE_HOST_SESSION_FIELDS (worktreeKeyed, tabKeyed, paneKeyed) so dependent rows are restored alongside tabsByWorktree before testimony and withheld after. 3. Added red-first regression test in workspace-session-host-write-race.test.ts proving terminalLayoutsByTabId and remoteSessionIdsByTabId survive the write. --- .../lib/workspace-session-host-contention.ts | 14 +++- ...workspace-session-host-shadow-testimony.ts | 32 ++++++-- .../workspace-session-host-write-race.test.ts | 82 +++++++++++++++++++ ...ace-session-stranded-partition-adoption.ts | 24 +++++- 4 files changed, 140 insertions(+), 12 deletions(-) diff --git a/src/renderer/src/lib/workspace-session-host-contention.ts b/src/renderer/src/lib/workspace-session-host-contention.ts index 48162e5ad2e6..161bb6a1644f 100644 --- a/src/renderer/src/lib/workspace-session-host-contention.ts +++ b/src/renderer/src/lib/workspace-session-host-contention.ts @@ -47,6 +47,18 @@ export const WORKTREE_KEYED_FIELDS = ( Object.keys(WORKSPACE_SESSION_FIELD_OWNERSHIP) as (keyof WorkspaceSessionState)[] ).filter((field) => WORKSPACE_SESSION_FIELD_OWNERSHIP[field] === 'worktreeKeyed') +/** `WORKTREE_KEYED_FIELDS` plus the tab- and pane-keyed fields a declined tab also owns + * (`terminalLayoutsByTabId`, `remoteSessionIdsByTabId`, `localOnlyScrollbackByTabId`, + * `terminalPtyIncarnationsByPaneKey`). `attachHostSessionShadow` restores over this wider set so a + * GAP-03 stranded-partition shadow (`partitionRowsTheWriteWontReturn`) can carry a declined tab's + * dependent rows back with it, not just its `tabsByWorktree` entry. */ +export const PARKABLE_HOST_SESSION_FIELDS = ( + Object.keys(WORKSPACE_SESSION_FIELD_OWNERSHIP) as (keyof WorkspaceSessionState)[] +).filter((field) => { + const ownership = WORKSPACE_SESSION_FIELD_OWNERSHIP[field] + return ownership === 'worktreeKeyed' || ownership === 'tabKeyed' || ownership === 'paneKeyed' +}) + /** Bare worktree id behind a session key. Lives in shared because the partition adoption read needs * the same normalization, and two implementations of it would drift. */ export const normalizeWorkspaceSessionKeyToWorktreeId = normalizeWorkspaceSessionKeyToWorkspaceId @@ -279,7 +291,7 @@ export function attachHostSessionShadow( if (!slice || !shadowSlice) { continue } - for (const field of WORKTREE_KEYED_FIELDS) { + for (const field of PARKABLE_HOST_SESSION_FIELDS) { const parked = shadowSlice[field] if (!isWorkspaceSessionRecord(parked)) { continue diff --git a/src/renderer/src/lib/workspace-session-host-shadow-testimony.ts b/src/renderer/src/lib/workspace-session-host-shadow-testimony.ts index d3438d59fcfa..147f53a65090 100644 --- a/src/renderer/src/lib/workspace-session-host-shadow-testimony.ts +++ b/src/renderer/src/lib/workspace-session-host-shadow-testimony.ts @@ -1,9 +1,14 @@ import type { ExecutionHostId } from '../../../shared/execution-host' import { parseExecutionHostId } from '../../../shared/execution-host' import { parseWorkspaceKey } from '../../../shared/workspace-scope' +import { WORKSPACE_SESSION_FIELD_OWNERSHIP } from '../../../shared/workspace-session-host-field-ownership' import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' import { isWorkspaceSessionRecord } from '../../../shared/workspace-session-host-records' -import { WORKTREE_KEYED_FIELDS } from './workspace-session-host-contention' +import { + buildWorktreeIdByTabId, + worktreeIdForPaneKey +} from '../../../shared/workspace-session-host-records' +import { PARKABLE_HOST_SESSION_FIELDS } from './workspace-session-host-contention' import type { HostSessionSlices } from './workspace-session-host-split' import { hostHasAnsweredForTarget, @@ -41,20 +46,31 @@ export function shadowRowsTheHostHasNotAnswered( } let hostWithheld = false - let survivingWorktreeKeyedFieldCount = 0 + let survivingParkableFieldCount = 0 const nextHostSlice: WorkspaceSessionState = { ...hostSlice } + const worktreeIdByTabIdInShadow = buildWorktreeIdByTabId(hostSlice) - for (const field of WORKTREE_KEYED_FIELDS) { + for (const field of PARKABLE_HOST_SESSION_FIELDS) { const record = hostSlice[field] if (!isWorkspaceSessionRecord(record)) { continue } + const ownership = WORKSPACE_SESSION_FIELD_OWNERSHIP[field] let fieldWithheld = false const survivingRecord: Record = {} for (const [key, value] of Object.entries(record)) { - if (parseWorkspaceKey(key)?.type === 'folder') { + const owningWorkspaceKey = + ownership === 'worktreeKeyed' + ? key + : ownership === 'tabKeyed' + ? worktreeIdByTabIdInShadow.get(key) + : worktreeIdForPaneKey(worktreeIdByTabIdInShadow, key) + if ( + owningWorkspaceKey !== undefined && + parseWorkspaceKey(owningWorkspaceKey)?.type === 'folder' + ) { survivingRecord[key] = value } else { fieldWithheld = true @@ -65,21 +81,21 @@ export function shadowRowsTheHostHasNotAnswered( hostWithheld = true withheld = true if (Object.keys(survivingRecord).length > 0) { - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: field is a worktree-keyed record field and survivingRecord contains only its surviving entries. + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: field is a parkable record field and survivingRecord contains only its surviving entries. ;(nextHostSlice as Record)[field] = survivingRecord - survivingWorktreeKeyedFieldCount++ + survivingParkableFieldCount++ } else { delete nextHostSlice[field] } } else { if (Object.keys(record).length > 0) { - survivingWorktreeKeyedFieldCount++ + survivingParkableFieldCount++ } } } if (hostWithheld) { - if (survivingWorktreeKeyedFieldCount > 0) { + if (survivingParkableFieldCount > 0) { nextShadow[hostId] = nextHostSlice } } else { diff --git a/src/renderer/src/lib/workspace-session-host-write-race.test.ts b/src/renderer/src/lib/workspace-session-host-write-race.test.ts index 19bb12f6adda..4023d87512f0 100644 --- a/src/renderer/src/lib/workspace-session-host-write-race.test.ts +++ b/src/renderer/src/lib/workspace-session-host-write-race.test.ts @@ -265,3 +265,85 @@ describe('GAP-03 Write Race Protection', () => { ).toEqual(['tab-fresh']) }) }) + +describe('GAP-03 regression: a declined tab must park and restore its dependent tab/pane rows too', () => { + it('carries terminalLayoutsByTabId and remoteSessionIdsByTabId for a declined tab back with it, not just tabsByWorktree', async () => { + // Same shape as the "still routes a declined row back" case above (host has not answered yet, + // so preserving the parked verdict is the only safe move), but this host partition also carries + // per-tab rows for the declined worktree's tabs: a terminal layout and a remote relay session id. + // `partitionRowsTheWriteWontReturn` only walked `worktreeKeyed` fields, so these tab-keyed rows + // were never parked -- and the next write to this SSH partition (forced here by the sibling + // worktree's fresh local activity, same as the sibling test above) drops them even though the + // worktree-keyed `tabsByWorktree` row for the same declined tabs correctly comes back. + const read = await fetchWorkspaceSessionWithRuntimeHostOwners( + partitionedApi({ + local: session({}), + [SSH_HOST_ID]: session({ + tabsByWorktree: { + [WORKTREE_ID]: [tab('tab-2', WORKTREE_ID), tab('tab-3', WORKTREE_ID)], + [SIBLING_WORKTREE_ID]: [tab('tab-4', SIBLING_WORKTREE_ID)] + }, + terminalLayoutsByTabId: { + 'tab-2': { root: null, activeLeafId: null, expandedLeafId: null } as never + }, + remoteSessionIdsByTabId: { + 'tab-2': 'ssh:target-1@@pty-2' + } + }) + }), + [ + { id: REPO_ID, connectionId: TARGET_ID, executionHostId: SSH_HOST_ID }, + { id: SIBLING_REPO_ID, connectionId: TARGET_ID, executionHostId: SSH_HOST_ID } + ] + ) + expect(read.session.tabsByWorktree[WORKTREE_ID] ?? []).toEqual([]) + + const NEW_LOCAL_WORKTREE_ID = `${SIBLING_REPO_ID}::/remote/new-tab` + const payloadWithFreshLocalActivity: WorkspaceSessionState = { + ...read.session, + tabsByWorktree: { + ...read.session.tabsByWorktree, + [NEW_LOCAL_WORKTREE_ID]: [tab('tab-fresh', NEW_LOCAL_WORKTREE_ID)] + } + } + + const { captured, api } = capturingApi() + const state: HostPersistenceState = { + repos: [ + { id: REPO_ID, connectionId: TARGET_ID, executionHostId: SSH_HOST_ID }, + { id: SIBLING_REPO_ID, connectionId: TARGET_ID, executionHostId: SSH_HOST_ID } + ], + worktreesByRepo: { + [REPO_ID]: [ + { + id: WORKTREE_ID, + repoId: REPO_ID, + hostId: SSH_HOST_ID, + runtimeOwnerEnvironmentId: undefined + } + ], + [SIBLING_REPO_ID]: [ + { + id: NEW_LOCAL_WORKTREE_ID, + repoId: SIBLING_REPO_ID, + hostId: SSH_HOST_ID, + runtimeOwnerEnvironmentId: undefined + } + ] + }, + contestedHostWorkspaceSessions: read.contestedHostWorkspaceSessions, + contestedPrimaryHostBySessionKey: read.contestedPrimaryHostBySessionKey + } + + await persistWorkspaceSessionByHost(api as never, payloadWithFreshLocalActivity, state) + + // The worktree-keyed row already round-trips correctly (pinned by the sibling test above). + expect(captured[SSH_HOST_ID]?.tabsByWorktree?.[WORKTREE_ID]?.map((entry) => entry.id)).toEqual([ + 'tab-2', + 'tab-3' + ]) + // The tab-keyed rows for the SAME declined tab must survive the write alongside it. + expect(captured[SSH_HOST_ID]?.terminalLayoutsByTabId?.['tab-2']).toBeDefined() + expect(captured[SSH_HOST_ID]?.remoteSessionIdsByTabId?.['tab-2']).toBe('ssh:target-1@@pty-2') + }) +}) diff --git a/src/shared/workspace-session-stranded-partition-adoption.ts b/src/shared/workspace-session-stranded-partition-adoption.ts index cd0c5f97782f..817adef16b38 100644 --- a/src/shared/workspace-session-stranded-partition-adoption.ts +++ b/src/shared/workspace-session-stranded-partition-adoption.ts @@ -191,7 +191,9 @@ function adoptRecord( } /** - * The worktree-keyed rows a partition holds for workspaces the write will not route back to it. + * The worktree-keyed rows a partition holds for workspaces the write will not route back to it, + * plus the tab- and pane-keyed rows those declined tabs own (`terminalLayoutsByTabId`, + * `remoteSessionIdsByTabId`, `localOnlyScrollbackByTabId`, `terminalPtyIncarnationsByPaneKey`). * * Parked rather than dropped. A partition write replaces each field with exactly what the unified * session routed there, so a row this read left out — declined as residue, withheld as contested, @@ -200,14 +202,24 @@ function adoptRecord( * first, which is the protection a contested runtime co-claimant already gets. Declining to show a * row must never mean deleting it: docs/reference/ssh-execution-boundary.md makes leak, never kill, * the safe direction, and a row no partition holds at all is unrecoverable. + * + * Why the tab/pane sweep and not just `tabsByWorktree`: a declined worktree's `terminalLayoutsByTabId` + * and `remoteSessionIdsByTabId` rows are keyed by tab id, not worktree id, so the worktree-keyed + * walk above never sees them. Without this, a write landing on the same SSH partition before a live + * answer keeps the declined tab (via the worktree-keyed park) but drops its layout and relay-session + * rows, leaving the restored tab with no pane to reattach to. */ export function partitionRowsTheWriteWontReturn( host: WorkspaceSessionState, adoptedWorkspaceIds: ReadonlySet ): WorkspaceSessionState | null { let parked: KeyedRecord | null = null + const worktreeIdByTabIdOnHost = buildWorktreeIdByTabId(host) + const isDeclinedWorktree = (worktreeId: string | undefined): boolean => + worktreeId !== undefined && !adoptedWorkspaceIds.has(worktreeId) for (const field of SESSION_FIELDS) { - if (WORKSPACE_SESSION_FIELD_OWNERSHIP[field] !== 'worktreeKeyed') { + const ownership: WorkspaceSessionFieldOwnership = WORKSPACE_SESSION_FIELD_OWNERSHIP[field] + if (ownership !== 'worktreeKeyed' && ownership !== 'tabKeyed' && ownership !== 'paneKeyed') { continue } const record = asRecord(host[field]) @@ -216,7 +228,13 @@ export function partitionRowsTheWriteWontReturn( } let kept: KeyedRecord | null = null for (const [key, entry] of Object.entries(record)) { - if (adoptedWorkspaceIds.has(normalizeWorkspaceSessionKeyToWorkspaceId(key))) { + const rowWorktreeId = + ownership === 'worktreeKeyed' + ? normalizeWorkspaceSessionKeyToWorkspaceId(key) + : ownership === 'tabKeyed' + ? worktreeIdByTabIdOnHost.get(key) + : worktreeIdForPaneKey(worktreeIdByTabIdOnHost, key) + if (!isDeclinedWorktree(rowWorktreeId)) { continue } kept ??= {} From 843cf40c83d022a04e87fc5cdf04672e055aab08 Mon Sep 17 00:00:00 2001 From: Lesley Murfin Date: Tue, 22 Sep 2026 22:13:53 -0600 Subject: [PATCH 11/23] fix(GAP-03): infer connection status from remoteSessionIdsByTabId when activeConnectionIdsAtShutdown is undefined CodeRabbit finding: when session.activeConnectionIdsAtShutdown is undefined (older persisted sessions, interrupted shutdowns), hydration defaulted to an empty set and passed false ("was disconnected") into reconciledWorktreeIdsForHost. This caused every confirmed worktree on that host to be reconciled and declined, dropping still-open tabs for sessions where shutdown status was merely unknown rather than explicitly disconnected. Fix: 1. wasTargetConnectedAtLastShutdown helper in workspace-session-host-hydration.ts: when activeConnectionIdsAtShutdown is defined, check membership as before. When undefined, check session.remoteSessionIdsByTabId for any surviving remote PTY session belonging to targetId (matching the established pattern in workspace-session-reconnect-targets.ts and ssh-pane-connect-gate.ts). 2. Added red-first regression test in workspace-session-host-offline-reconnect.test.ts proving tabs survive when activeConnectionIdsAtShutdown is undefined but remoteSessionIdsByTabId contains a session for the target. 9/9 in touched suite green, 55/55 across 5 related hydration/reconciliation suites green. --- .../lib/workspace-session-host-hydration.ts | 27 +++++++++++++++--- ...ace-session-host-offline-reconnect.test.ts | 28 +++++++++++++++++++ 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/src/renderer/src/lib/workspace-session-host-hydration.ts b/src/renderer/src/lib/workspace-session-host-hydration.ts index bd85401e04fc..c22059005b8c 100644 --- a/src/renderer/src/lib/workspace-session-host-hydration.ts +++ b/src/renderer/src/lib/workspace-session-host-hydration.ts @@ -1,5 +1,6 @@ import type { Repo } from '../../../shared/repo-types' import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' +import { parseAppSshPtyId } from '../../../shared/ssh-pty-id' import { getRepoExecutionHostId, getSshTargetIdForExecutionHost, @@ -157,6 +158,27 @@ export async function fetchWorkspaceSessionFromHosts( .session } +function wasTargetConnectedAtLastShutdown( + session: WorkspaceSessionState, + targetId: string | null +): boolean { + if (!targetId) { + return false + } + if (session.activeConnectionIdsAtShutdown !== undefined) { + return session.activeConnectionIdsAtShutdown.includes(targetId) + } + // When activeConnectionIdsAtShutdown is undefined (older persisted sessions, interrupted + // shutdowns), infer connection status from remoteSessionIdsByTabId: if the base session still + // holds a remote PTY session for this target, the client was connected to it, so do not decline. + for (const sessionId of Object.values(session.remoteSessionIdsByTabId ?? {})) { + if (parseAppSshPtyId(sessionId)?.connectionId === targetId) { + return true + } + } + return false +} + export async function fetchWorkspaceSessionWithRuntimeHostOwners( api: SessionReadApi, repos: readonly Pick[], @@ -218,9 +240,6 @@ export async function fetchWorkspaceSessionWithRuntimeHostOwners( // for the slices it arbitrates, and these are not among them. Everything this read leaves behind // in an ssh partition still has to survive the next write to it. const shadow: HostSessionSlices = { ...merged.shadow } - // Why captured once, before the loop: `activeConnectionIdsAtShutdown` is global (routes to - // 'local' unconditionally), so it never changes as `session` is reassigned across hosts below. - const connectedTargetIdsAtShutdown = new Set(merged.session.activeConnectionIdsAtShutdown ?? []) for (const [hostId, slice] of sshPartitions) { const targetId = getSshTargetIdForExecutionHost(hostId) const adoption = adoptStrandedHostPartitionSession(session, slice, { @@ -234,7 +253,7 @@ export async function fetchWorkspaceSessionWithRuntimeHostOwners( reconciledWorktreeIds: reconciledWorktreeIdsForHost( attribution, hostId, - Boolean(targetId && connectedTargetIdsAtShutdown.has(targetId)) + wasTargetConnectedAtLastShutdown(merged.session, targetId) ) }) session = adoption.session diff --git a/src/renderer/src/lib/workspace-session-host-offline-reconnect.test.ts b/src/renderer/src/lib/workspace-session-host-offline-reconnect.test.ts index 59cdf93875ec..3fa8200a6c1c 100644 --- a/src/renderer/src/lib/workspace-session-host-offline-reconnect.test.ts +++ b/src/renderer/src/lib/workspace-session-host-offline-reconnect.test.ts @@ -111,6 +111,34 @@ describe('GAP-03 regression: canonical-keyed host rows must still be declined', }) }) +describe('GAP-03 regression: unknown shutdown status must not decline tabs if remoteSessionIdsByTabId indicates connection', () => { + it('keeps tabs when activeConnectionIdsAtShutdown is undefined but remoteSessionIdsByTabId names the target', async () => { + // When activeConnectionIdsAtShutdown is undefined (older persisted sessions, interrupted + // shutdowns), treating it as an empty set passes false ("disconnected") into + // reconciledWorktreeIdsForHost, which then declines every worktree on that host -- dropping + // tabs that were actually connected. If remoteSessionIdsByTabId still holds a session for the + // target, connection status must be inferred as connected so tabs survive. + const read = await fetchWorkspaceSessionWithRuntimeHostOwners( + partitionedApi({ + local: session({ + remoteSessionIdsByTabId: { 'tab-1': `ssh:${TARGET_ID}@@pty-1` } + }), + [SSH_HOST_ID]: session({ + tabsByWorktree: { + [WORKTREE_ID]: [tab('tab-1', WORKTREE_ID), tab('tab-2', WORKTREE_ID)] + } + }) + }), + [{ id: REPO_ID, connectionId: TARGET_ID, executionHostId: SSH_HOST_ID }] + ) + + expect(read.session.tabsByWorktree[WORKTREE_ID]?.map((entry) => entry.id)).toEqual([ + 'tab-1', + 'tab-2' + ]) + }) +}) + describe('GAP-03 non-regression: #12721 offline-created local work must survive reconnect', () => { it('keeps a genuinely offline-created tab that was never synced to the host partition', async () => { // This client created a tab entirely offline — the host partition (mirroring the last state From ef87a223f3ec733e1b0ae1c93e8b483bbdc9ebc4 Mon Sep 17 00:00:00 2001 From: Lesley Murfin Date: Tue, 22 Sep 2026 22:23:48 -0600 Subject: [PATCH 12/23] fix(GAP-03): order and discard in-flight pre-testimony writes against live snapshot arrival CodeRabbit finding 3: an api.set/patch call generated while hostHasAnsweredForTarget was false (so declined tabs were attached by the shadow) could be delivered AFTER the live snapshot landed and testimony flipped true. Nothing ordered in-flight writes against the live snapshot's arrival, so a delayed pre-testimony write could overwrite the authoritative state and resurrect closed tabs. Fix: 1. enqueueHostPartitionWrite in workspace-session-host-persistence.ts serializes writes per host partition and tracks monotonic write generations. 2. When a write was prepared without host testimony, it checks right before dispatch whether testimony has since arrived or a newer write was dispatched for that host; if so, the stale pre-testimony write is discarded instead of overwriting the authoritative snapshot. 3. Added red-first regression test in workspace-session-host-write-race.test.ts proving a deferred pre-testimony write queued behind an in-flight write does not overwrite the live snapshot when testimony arrives before delivery. 55/55 tests across 5 related persistence/contention/write-race suites green. --- .../lib/workspace-session-host-persistence.ts | 74 +++++++++- .../workspace-session-host-write-race.test.ts | 126 ++++++++++++++++++ 2 files changed, 197 insertions(+), 3 deletions(-) diff --git a/src/renderer/src/lib/workspace-session-host-persistence.ts b/src/renderer/src/lib/workspace-session-host-persistence.ts index dbb68910b61f..1368633d089d 100644 --- a/src/renderer/src/lib/workspace-session-host-persistence.ts +++ b/src/renderer/src/lib/workspace-session-host-persistence.ts @@ -36,7 +36,10 @@ import { extendWorktreeIdByTabId, type WorkspaceTabOwnerCatalog } from '../../../shared/workspace-session-host-records' -import type { RemoteWorkspaceTestimonyState } from './remote-workspace-host-testimony' +import { + hostHasAnsweredForTarget, + type RemoteWorkspaceTestimonyState +} from './remote-workspace-host-testimony' import { shadowRowsTheHostHasNotAnswered } from './workspace-session-host-shadow-testimony' export type HostPersistenceState = WorkspaceTabOwnerCatalog & { @@ -238,6 +241,69 @@ function splitWorkspaceSessionForWrite( return slices } +const hostPartitionWriteChains = new Map>() +const hostPartitionWriteGenerations = new Map() + +export function resetHostPartitionWriteStateForTest(): void { + hostPartitionWriteChains.clear() + hostPartitionWriteGenerations.clear() +} + +function enqueueHostPartitionWrite( + hostId: ExecutionHostId, + state: HostPersistenceState, + perform: () => Promise +): Promise { + const parsed = parseExecutionHostId(hostId) + const targetId = parsed?.kind === 'ssh' ? parsed.targetId : null + const preparedWithoutTestimony = targetId !== null && !hostHasAnsweredForTarget(state, targetId) + const nextGen = (hostPartitionWriteGenerations.get(hostId) ?? 0) + 1 + hostPartitionWriteGenerations.set(hostId, nextGen) + + const inFlight = hostPartitionWriteChains.get(hostId) + if (!inFlight) { + if ( + preparedWithoutTestimony && + targetId !== null && + hostHasAnsweredForTarget(state, targetId) + ) { + return Promise.resolve() + } + let resolveInFlight!: () => void + const inFlightPromise = new Promise((resolve) => { + resolveInFlight = resolve + }) + hostPartitionWriteChains.set(hostId, inFlightPromise) + const result = perform() + void result.finally(() => { + resolveInFlight() + if (hostPartitionWriteChains.get(hostId) === inFlightPromise) { + hostPartitionWriteChains.delete(hostId) + } + }) + return result + } + + const task = inFlight.then(async () => { + if ( + (preparedWithoutTestimony && + targetId !== null && + hostHasAnsweredForTarget(state, targetId)) || + (hostPartitionWriteGenerations.get(hostId) ?? 0) > nextGen + ) { + return + } + await perform() + }) + const tracked = task.finally(() => { + if (hostPartitionWriteChains.get(hostId) === tracked) { + hostPartitionWriteChains.delete(hostId) + } + }) + hostPartitionWriteChains.set(hostId, tracked) + return task +} + /** Patch path of the debounced session writer: split the partial patch by owner host and patch * each partition. `localWrite` orders the SSH remote-workspace upload; `written` settles once * every partition did and rejects if any failed, so the writer re-queues those fields. */ @@ -250,7 +316,9 @@ export function patchWorkspaceSessionByHost( const local = (slices[LOCAL_EXECUTION_HOST_ID] ?? patch) as WorkspaceSessionPatch const localWrite = api.patch(local) const hostWrites = nonLocalHostSessionEntries(slices).map(([hostId, slice]) => - api.patch(slice, hostId).catch((err: unknown) => { + enqueueHostPartitionWrite(hostId, state, () => + api.patch(slice as WorkspaceSessionPatch, hostId) + ).catch((err: unknown) => { console.warn(`[session] host partition patch failed for ${hostId}:`, err) throw err }) @@ -280,7 +348,7 @@ export async function persistWorkspaceSessionByHost( const slices = splitWorkspaceSessionForWrite(payload, state, 'replace') const writes: Promise[] = [api.set(slices[LOCAL_EXECUTION_HOST_ID] ?? payload)] for (const [hostId, slice] of nonLocalHostSessionEntries(slices)) { - writes.push(api.set(slice, hostId)) + writes.push(enqueueHostPartitionWrite(hostId, state, () => api.set(slice, hostId))) } await Promise.all(writes) await api.flush() diff --git a/src/renderer/src/lib/workspace-session-host-write-race.test.ts b/src/renderer/src/lib/workspace-session-host-write-race.test.ts index 4023d87512f0..77b0b0e622a8 100644 --- a/src/renderer/src/lib/workspace-session-host-write-race.test.ts +++ b/src/renderer/src/lib/workspace-session-host-write-race.test.ts @@ -264,6 +264,132 @@ describe('GAP-03 Write Race Protection', () => { captured[SSH_HOST_ID]?.tabsByWorktree?.[NEW_LOCAL_WORKTREE_ID]?.map((e) => e.id) ).toEqual(['tab-fresh']) }) + + it('a deferred write prepared before testimony arrived does not overwrite authoritative state if testimony arrives before delivery', async () => { + // CodeRabbit finding 3: an api.set call generated while hostHasAnsweredForTarget was false + // (so declined tabs were attached by the shadow) can be delivered AFTER the live snapshot lands + // and testimony flips true. If nothing orders in-flight writes against the live snapshot's + // arrival, the stale in-flight payload overwrites the authoritative state and resurrects the + // closed tabs. + const read = await fetchWorkspaceSessionWithRuntimeHostOwners( + partitionedApi({ + local: session({}), + [SSH_HOST_ID]: session({ + tabsByWorktree: { + [WORKTREE_ID]: [tab('tab-2', WORKTREE_ID), tab('tab-3', WORKTREE_ID)], + [SIBLING_WORKTREE_ID]: [tab('tab-4', SIBLING_WORKTREE_ID)] + } + }) + }), + [ + { id: REPO_ID, connectionId: TARGET_ID, executionHostId: SSH_HOST_ID }, + { id: SIBLING_REPO_ID, connectionId: TARGET_ID, executionHostId: SSH_HOST_ID } + ] + ) + expect(read.session.tabsByWorktree[WORKTREE_ID] ?? []).toEqual([]) + + const NEW_LOCAL_WORKTREE_ID = `${SIBLING_REPO_ID}::/remote/new-tab` + const payloadWithFreshLocalActivity: WorkspaceSessionState = { + ...read.session, + tabsByWorktree: { + ...read.session.tabsByWorktree, + [NEW_LOCAL_WORKTREE_ID]: [tab('tab-fresh', NEW_LOCAL_WORKTREE_ID)] + } + } + + const state: HostPersistenceState = { + repos: [ + { id: REPO_ID, connectionId: TARGET_ID, executionHostId: SSH_HOST_ID }, + { id: SIBLING_REPO_ID, connectionId: TARGET_ID, executionHostId: SSH_HOST_ID } + ], + worktreesByRepo: { + [REPO_ID]: [ + { + id: WORKTREE_ID, + repoId: REPO_ID, + hostId: SSH_HOST_ID, + runtimeOwnerEnvironmentId: undefined + } + ], + [SIBLING_REPO_ID]: [ + { + id: NEW_LOCAL_WORKTREE_ID, + repoId: SIBLING_REPO_ID, + hostId: SSH_HOST_ID, + runtimeOwnerEnvironmentId: undefined + } + ] + }, + contestedHostWorkspaceSessions: read.contestedHostWorkspaceSessions, + contestedPrimaryHostBySessionKey: read.contestedPrimaryHostBySessionKey, + remoteWorkspaceHydratedTargetIds: new Set(), + remoteWorkspaceSyncStatusByTargetId: {} + } + + const captured: Partial> = {} + const api = { + get: async () => getDefaultWorkspaceSession(), + patch: async () => {}, + setSync: () => {}, + set: async (payload: WorkspaceSessionState, hostId?: ExecutionHostId) => { + captured[hostId ?? 'local'] = payload + }, + flush: async () => {} + } + + // An earlier write W0 is in-flight on SSH_HOST_ID, holding that partition's write queue: + let releaseW0!: () => void + const w0Gate = new Promise((resolve) => { + releaseW0 = resolve + }) + const w0Promise = persistWorkspaceSessionByHost( + { + ...api, + set: async (_payload: WorkspaceSessionState, hostId?: ExecutionHostId) => { + if (hostId === SSH_HOST_ID) { + await w0Gate + } + } + } as never, + session({ + tabsByWorktree: { + [NEW_LOCAL_WORKTREE_ID]: [tab('tab-fresh', NEW_LOCAL_WORKTREE_ID)] + } + }), + state + ) + + // W1 is dispatched while W0 is in-flight: W1 carries the stale declined tabs (prepared without testimony) + // and queues behind W0 on SSH_HOST_ID. + const writePromise = persistWorkspaceSessionByHost( + api as never, + payloadWithFreshLocalActivity, + state + ) + + // While W1 is in-flight / queued, the live snapshot lands on the partition and establishes authoritative state: + captured[SSH_HOST_ID] = session({ + tabsByWorktree: { + [SIBLING_WORKTREE_ID]: [tab('tab-4', SIBLING_WORKTREE_ID)] + } + }) + // And testimony arrives: + state.remoteWorkspaceHydratedTargetIds = new Set([TARGET_ID]) + state.remoteWorkspaceSyncStatusByTargetId = { [TARGET_ID]: { phase: 'synced' } } + + // Release W0 to let the queue proceed: + releaseW0() + await w0Promise + await writePromise + + // W1 was queued behind W0 and prepared before testimony arrived; when W1 dequeued, + // it observed that testimony had landed for TARGET_ID, so it discarded its stale pre-testimony + // payload rather than overwriting the authoritative state! + expect(captured[SSH_HOST_ID]?.tabsByWorktree?.[WORKTREE_ID]).toBeUndefined() + expect(captured[SSH_HOST_ID]?.tabsByWorktree?.[SIBLING_WORKTREE_ID]?.map((e) => e.id)).toEqual([ + 'tab-4' + ]) + }) }) describe('GAP-03 regression: a declined tab must park and restore its dependent tab/pane rows too', () => { From a4a5d94e97686a868eee8d50f4a990d9aa81c7f4 Mon Sep 17 00:00:00 2001 From: Lesley Murfin Date: Tue, 22 Sep 2026 22:25:18 -0600 Subject: [PATCH 13/23] fix(GAP-03): preserve non-terminal rows in write shadow when SSH testimony arrives Pullfrog finding 6: shadowRowsTheHostHasNotAnswered dropped ALL non-folder parked rows once hostHasAnsweredForTarget became true. Because direct-SSH snapshots only project terminal state (remote-workspace-snapshot-apply.ts:201), non-terminal state such as openFilesByWorktree editor drafts is never reported by or superseded by the SSH host snapshot. Withholding those non-terminal rows caused subsequent partition writes to erase offline-created editor drafts from disk. Fix: 1. Define SSH_TERMINAL_TESTIMONY_FIELDS in workspace-session-host-shadow-testimony.ts: only terminal-scoped fields that SSH snapshots actually manage are withheld by host testimony. Non-terminal fields like openFilesByWorktree survive testimony arrival. 2. Added unit test in workspace-session-host-shadow-testimony.test.ts verifying openFilesByWorktree survives even after testimony lands for that target. 3. Updated test comment in workspace-session-host-offline-reconnect.test.ts to accurately state the testimony boundary. --- ...ace-session-host-offline-reconnect.test.ts | 4 +- ...pace-session-host-shadow-testimony.test.ts | 43 +++++++++++++++++++ ...workspace-session-host-shadow-testimony.ts | 24 ++++++++++- 3 files changed, 68 insertions(+), 3 deletions(-) diff --git a/src/renderer/src/lib/workspace-session-host-offline-reconnect.test.ts b/src/renderer/src/lib/workspace-session-host-offline-reconnect.test.ts index 3fa8200a6c1c..cd6fd190b411 100644 --- a/src/renderer/src/lib/workspace-session-host-offline-reconnect.test.ts +++ b/src/renderer/src/lib/workspace-session-host-offline-reconnect.test.ts @@ -355,7 +355,9 @@ describe('GAP-03 Concurrent Active Edits', () => { expect(read.session.tabsByWorktree[SIBLING_WORKTREE_ID]).toBeUndefined() // But not deleted either: it survives in the write-side shadow for the host partition, so a // live SSH reconnect (a separate code path this fix does not touch) can still recover it, and - // the next write to this host cannot erase it (`attachHostSessionShadow`). + // writes before host testimony arrives cannot erase it (`attachHostSessionShadow`). Once the + // host answers with a live non-conflicting snapshot, testimony supersedes terminal rows, while + // non-terminal state (like editor drafts) remains protected. expect( read.contestedHostWorkspaceSessions[SSH_HOST_ID]?.tabsByWorktree?.[SIBLING_WORKTREE_ID]?.map( (entry: TerminalTab) => entry.id diff --git a/src/renderer/src/lib/workspace-session-host-shadow-testimony.test.ts b/src/renderer/src/lib/workspace-session-host-shadow-testimony.test.ts index 121a8881d79b..66ab1ffaffd4 100644 --- a/src/renderer/src/lib/workspace-session-host-shadow-testimony.test.ts +++ b/src/renderer/src/lib/workspace-session-host-shadow-testimony.test.ts @@ -141,4 +141,47 @@ describe('shadowRowsTheHostHasNotAnswered', () => { expect(result?.[SSH_HOST_ID]).toBeUndefined() expect(Object.keys(result ?? {})).toEqual([]) }) + + it('keeps non-terminal fields such as openFilesByWorktree even after the host answers, because direct-SSH snapshots only project terminal state', () => { + // Pullfrog finding 6: direct-SSH snapshots project terminal state only + // (remote-workspace-snapshot-apply.ts:201). A parked row for openFilesByWorktree (and its + // unsaved dirtyDraftContent) was never in the host's terminal snapshot and never superseded by + // it, so host testimony must not withhold it from the shadow. + const answeredState: RemoteWorkspaceTestimonyState = { + remoteWorkspaceHydratedTargetIds: new Set([TARGET_ID]), + remoteWorkspaceSyncStatusByTargetId: { + [TARGET_ID]: { phase: 'synced', direction: 'pull' } + } + } + + const worktreeKey = 'worktree:repo-1::worktree-1' + const worktreeTabs = [tab('tab-worktree', worktreeKey)] + const openFiles = [ + { + filePath: '/srv/app/index.ts', + relativePath: 'index.ts', + worktreeId: worktreeKey, + language: 'typescript' + } + ] + + const shadow: HostSessionSlices = { + [SSH_HOST_ID]: { + ...sessionWithTabs({ [worktreeKey]: worktreeTabs }), + openFilesByWorktree: { [worktreeKey]: openFiles as never } + } + } + + const result = shadowRowsTheHostHasNotAnswered(shadow, answeredState) + + expect(result).toBeDefined() + const hostSlice = result?.[SSH_HOST_ID] + expect(hostSlice).toBeDefined() + // Terminal tabs for worktreeKey are withheld (superseded by testimony): + expect(hostSlice?.tabsByWorktree).toBeUndefined() + // Non-terminal openFilesByWorktree row survives: + expect(hostSlice?.openFilesByWorktree).toEqual({ + [worktreeKey]: openFiles + }) + }) }) diff --git a/src/renderer/src/lib/workspace-session-host-shadow-testimony.ts b/src/renderer/src/lib/workspace-session-host-shadow-testimony.ts index 147f53a65090..c06efe6ebd71 100644 --- a/src/renderer/src/lib/workspace-session-host-shadow-testimony.ts +++ b/src/renderer/src/lib/workspace-session-host-shadow-testimony.ts @@ -15,6 +15,25 @@ import { type RemoteWorkspaceTestimonyState } from './remote-workspace-host-testimony' +/** + * Terminal fields that direct-SSH remote workspace snapshots actually project and manage. + * Non-terminal fields (such as `openFilesByWorktree` editor drafts) are never reported by an + * SSH host snapshot, so host testimony does not supersede them and they are never withheld. + */ +const SSH_TERMINAL_TESTIMONY_FIELDS: ReadonlySet = new Set([ + 'tabsByWorktree', + 'terminalLayoutsByTabId', + 'remoteSessionIdsByTabId', + 'localOnlyScrollbackByTabId', + 'terminalPtyIncarnationsByPaneKey', + 'unifiedTabs', + 'tabGroups', + 'tabGroupLayouts', + 'activeGroupIdByWorktree', + 'defaultTerminalTabsAppliedByWorktreeId', + 'activeTabIdByWorktree', + 'activeTabTypeByWorktree' +]) /** * The shadow minus every parked row whose host has already answered for it. Returns the input by * reference when nothing is withheld. @@ -67,10 +86,11 @@ export function shadowRowsTheHostHasNotAnswered( : ownership === 'tabKeyed' ? worktreeIdByTabIdInShadow.get(key) : worktreeIdForPaneKey(worktreeIdByTabIdInShadow, key) - if ( + const isFolder = owningWorkspaceKey !== undefined && parseWorkspaceKey(owningWorkspaceKey)?.type === 'folder' - ) { + const isCoveredByTerminalTestimony = SSH_TERMINAL_TESTIMONY_FIELDS.has(field) + if (isFolder || !isCoveredByTerminalTestimony) { survivingRecord[key] = value } else { fieldWithheld = true From dfbe65ef166e417c5d14f468c601b9e011d0ffdd Mon Sep 17 00:00:00 2001 From: Lesley Murfin Date: Mon, 28 Sep 2026 02:19:50 -0600 Subject: [PATCH 14/23] fix(GAP-03): extract write queue and resolve quality gate findings - Extract `enqueueHostPartitionWrite` and write chain state to `workspace-session-host-write-queue.ts` to bring `workspace-session-host-persistence.ts` within the 300-line budget (252 lines). - Deduplicate imports and add `SAFETY:` comments to all type assertions across touched files to satisfy `check-changed-code-quality` rules. - Add RED-first unit tests for `workspace-session-host-write-queue` verifying FIFO serialization, independent host concurrency, generation supersession, and test state reset. - Verified: 138/138 tests passing across all 13 session suites, typecheck clean, and 0 findings in `check-changed-code-quality`. --- .../lib/workspace-session-host-persistence.ts | 77 +------- ...pace-session-host-shadow-testimony.test.ts | 1 + ...workspace-session-host-shadow-testimony.ts | 3 +- ...workspace-session-host-write-queue.test.ts | 178 ++++++++++++++++++ .../lib/workspace-session-host-write-queue.ts | 142 ++++++++++++++ .../workspace-session-host-write-race.test.ts | 7 + ...offline-reconnect-tab-resurrection.spec.ts | 1 + 7 files changed, 340 insertions(+), 69 deletions(-) create mode 100644 src/renderer/src/lib/workspace-session-host-write-queue.test.ts create mode 100644 src/renderer/src/lib/workspace-session-host-write-queue.ts diff --git a/src/renderer/src/lib/workspace-session-host-persistence.ts b/src/renderer/src/lib/workspace-session-host-persistence.ts index 1368633d089d..681face27079 100644 --- a/src/renderer/src/lib/workspace-session-host-persistence.ts +++ b/src/renderer/src/lib/workspace-session-host-persistence.ts @@ -36,11 +36,14 @@ import { extendWorktreeIdByTabId, type WorkspaceTabOwnerCatalog } from '../../../shared/workspace-session-host-records' -import { - hostHasAnsweredForTarget, - type RemoteWorkspaceTestimonyState -} from './remote-workspace-host-testimony' +import type { RemoteWorkspaceTestimonyState } from './remote-workspace-host-testimony' import { shadowRowsTheHostHasNotAnswered } from './workspace-session-host-shadow-testimony' +import { + enqueueHostPartitionWrite, + resetHostPartitionWriteStateForTest +} from './workspace-session-host-write-queue' + +export { resetHostPartitionWriteStateForTest } export type HostPersistenceState = WorkspaceTabOwnerCatalog & { repos: readonly Pick[] @@ -241,69 +244,6 @@ function splitWorkspaceSessionForWrite( return slices } -const hostPartitionWriteChains = new Map>() -const hostPartitionWriteGenerations = new Map() - -export function resetHostPartitionWriteStateForTest(): void { - hostPartitionWriteChains.clear() - hostPartitionWriteGenerations.clear() -} - -function enqueueHostPartitionWrite( - hostId: ExecutionHostId, - state: HostPersistenceState, - perform: () => Promise -): Promise { - const parsed = parseExecutionHostId(hostId) - const targetId = parsed?.kind === 'ssh' ? parsed.targetId : null - const preparedWithoutTestimony = targetId !== null && !hostHasAnsweredForTarget(state, targetId) - const nextGen = (hostPartitionWriteGenerations.get(hostId) ?? 0) + 1 - hostPartitionWriteGenerations.set(hostId, nextGen) - - const inFlight = hostPartitionWriteChains.get(hostId) - if (!inFlight) { - if ( - preparedWithoutTestimony && - targetId !== null && - hostHasAnsweredForTarget(state, targetId) - ) { - return Promise.resolve() - } - let resolveInFlight!: () => void - const inFlightPromise = new Promise((resolve) => { - resolveInFlight = resolve - }) - hostPartitionWriteChains.set(hostId, inFlightPromise) - const result = perform() - void result.finally(() => { - resolveInFlight() - if (hostPartitionWriteChains.get(hostId) === inFlightPromise) { - hostPartitionWriteChains.delete(hostId) - } - }) - return result - } - - const task = inFlight.then(async () => { - if ( - (preparedWithoutTestimony && - targetId !== null && - hostHasAnsweredForTarget(state, targetId)) || - (hostPartitionWriteGenerations.get(hostId) ?? 0) > nextGen - ) { - return - } - await perform() - }) - const tracked = task.finally(() => { - if (hostPartitionWriteChains.get(hostId) === tracked) { - hostPartitionWriteChains.delete(hostId) - } - }) - hostPartitionWriteChains.set(hostId, tracked) - return task -} - /** Patch path of the debounced session writer: split the partial patch by owner host and patch * each partition. `localWrite` orders the SSH remote-workspace upload; `written` settles once * every partition did and rejects if any failed, so the writer re-queues those fields. */ @@ -315,8 +255,9 @@ export function patchWorkspaceSessionByHost( const slices = splitWorkspaceSessionForWrite(patch as WorkspaceSessionState, state, 'patch') const local = (slices[LOCAL_EXECUTION_HOST_ID] ?? patch) as WorkspaceSessionPatch const localWrite = api.patch(local) - const hostWrites = nonLocalHostSessionEntries(slices).map(([hostId, slice]) => +const hostWrites = nonLocalHostSessionEntries(slices).map(([hostId, slice]) => enqueueHostPartitionWrite(hostId, state, () => + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: slice is a partition subset matching WorkspaceSessionPatch api.patch(slice as WorkspaceSessionPatch, hostId) ).catch((err: unknown) => { console.warn(`[session] host partition patch failed for ${hostId}:`, err) diff --git a/src/renderer/src/lib/workspace-session-host-shadow-testimony.test.ts b/src/renderer/src/lib/workspace-session-host-shadow-testimony.test.ts index 66ab1ffaffd4..fe430ad9b5c8 100644 --- a/src/renderer/src/lib/workspace-session-host-shadow-testimony.test.ts +++ b/src/renderer/src/lib/workspace-session-host-shadow-testimony.test.ts @@ -168,6 +168,7 @@ describe('shadowRowsTheHostHasNotAnswered', () => { const shadow: HostSessionSlices = { [SSH_HOST_ID]: { ...sessionWithTabs({ [worktreeKey]: worktreeTabs }), + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: test fixture mock for openFilesByWorktree record openFilesByWorktree: { [worktreeKey]: openFiles as never } } } diff --git a/src/renderer/src/lib/workspace-session-host-shadow-testimony.ts b/src/renderer/src/lib/workspace-session-host-shadow-testimony.ts index c06efe6ebd71..500e33517c3b 100644 --- a/src/renderer/src/lib/workspace-session-host-shadow-testimony.ts +++ b/src/renderer/src/lib/workspace-session-host-shadow-testimony.ts @@ -3,9 +3,9 @@ import { parseExecutionHostId } from '../../../shared/execution-host' import { parseWorkspaceKey } from '../../../shared/workspace-scope' import { WORKSPACE_SESSION_FIELD_OWNERSHIP } from '../../../shared/workspace-session-host-field-ownership' import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' -import { isWorkspaceSessionRecord } from '../../../shared/workspace-session-host-records' import { buildWorktreeIdByTabId, + isWorkspaceSessionRecord, worktreeIdForPaneKey } from '../../../shared/workspace-session-host-records' import { PARKABLE_HOST_SESSION_FIELDS } from './workspace-session-host-contention' @@ -49,6 +49,7 @@ export function shadowRowsTheHostHasNotAnswered( let withheld = false const nextShadow: HostSessionSlices = {} + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Object.entries returns string keys; cast to ExecutionHostId and WorkspaceSessionState for (const [hostId, hostSlice] of Object.entries(shadow) as [ ExecutionHostId, WorkspaceSessionState | undefined diff --git a/src/renderer/src/lib/workspace-session-host-write-queue.test.ts b/src/renderer/src/lib/workspace-session-host-write-queue.test.ts new file mode 100644 index 000000000000..8e7d9d712e8a --- /dev/null +++ b/src/renderer/src/lib/workspace-session-host-write-queue.test.ts @@ -0,0 +1,178 @@ +import { describe, expect, it, vi } from 'vitest' +import type { ExecutionHostId } from '../../../shared/execution-host' +import { + enqueueHostPartitionWrite, + resetHostPartitionWriteStateForTest +} from './workspace-session-host-write-queue' + +describe('workspace-session-host-write-queue', () => { + it('serializes concurrent writes for the same host in FIFO order', async () => { + resetHostPartitionWriteStateForTest() + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: test fixture mock + const hostId = 'ssh:target-1' as ExecutionHostId + const order: number[] = [] + + let resolveFirstWrite: (() => void) | undefined + const firstWritePromise = new Promise((resolve) => { + resolveFirstWrite = resolve + }) + + const write1 = enqueueHostPartitionWrite(hostId, async () => { + await firstWritePromise + order.push(1) + return 'result-1' + }) + + const write2 = enqueueHostPartitionWrite(hostId, async () => { + order.push(2) + return 'result-2' + }) + + expect(order).toEqual([]) + resolveFirstWrite?.() + + const [res1, res2] = await Promise.all([write1, write2]) + expect(res1).toBe('result-1') + expect(res2).toBe('result-2') + expect(order).toEqual([1, 2]) + }) + + it('allows independent writes for distinct hosts to run concurrently', async () => { + resetHostPartitionWriteStateForTest() + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: test fixture mock + const hostA = 'ssh:target-a' as ExecutionHostId + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: test fixture mock + const hostB = 'ssh:target-b' as ExecutionHostId + + let hostAStarted = false + let hostBStarted = false + let resolveAll: (() => void) | undefined + const gate = new Promise((resolve) => { + resolveAll = resolve + }) + + const writeA = enqueueHostPartitionWrite(hostA, async () => { + hostAStarted = true + await gate + return 'done-a' + }) + + const writeB = enqueueHostPartitionWrite(hostB, async () => { + hostBStarted = true + await gate + return 'done-b' + }) + + await vi.waitFor(() => { + expect(hostAStarted).toBe(true) + expect(hostBStarted).toBe(true) + }) + + resolveAll?.() + const [resA, resB] = await Promise.all([writeA, writeB]) + expect(resA).toBe('done-a') + expect(resB).toBe('done-b') + }) + + it('skips a queued write if a newer generation write has already superseded it', async () => { + resetHostPartitionWriteStateForTest() + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: test fixture mock + const hostId = 'ssh:target-1' as ExecutionHostId + + const executionLog: string[] = [] + + let resolveActive: (() => void) | undefined + const activeBlocker = new Promise((resolve) => { + resolveActive = resolve + }) + + const activeWrite = enqueueHostPartitionWrite( + hostId, + async () => { + executionLog.push('active') + await activeBlocker + return 'active-res' + }, + { generation: 1 } + ) + + const supersededWrite = enqueueHostPartitionWrite( + hostId, + async () => { + executionLog.push('superseded') + return 'superseded-res' + }, + { generation: 2 } + ) + + const latestWrite = enqueueHostPartitionWrite( + hostId, + async () => { + executionLog.push('latest') + return 'latest-res' + }, + { generation: 3 } + ) + + resolveActive?.() + + const [resActive, resSuperseded, resLatest] = await Promise.all([ + activeWrite, + supersededWrite, + latestWrite + ]) + + expect(resActive).toBe('active-res') + expect(resSuperseded).toBeNull() + expect(resLatest).toBe('latest-res') + expect(executionLog).toEqual(['active', 'latest']) + }) + + it('skips a pre-testimony write if testimony arrives before the write begins', async () => { + resetHostPartitionWriteStateForTest() + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: test fixture mock + const hostId = 'ssh:target-1' as ExecutionHostId + + let testimonyArrived = false + let resolveActive: (() => void) | undefined + const activeBlocker = new Promise((resolve) => { + resolveActive = resolve + }) + + const activeWrite = enqueueHostPartitionWrite(hostId, async () => { + await activeBlocker + return 'active-done' + }) + + const writeFn = vi.fn().mockResolvedValue('testimony-guarded-res') + + const guardedWrite = enqueueHostPartitionWrite(hostId, writeFn, { + shouldSkip: () => testimonyArrived + }) + + testimonyArrived = true + resolveActive?.() + + const [resActive, resGuarded] = await Promise.all([activeWrite, guardedWrite]) + expect(resActive).toBe('active-done') + expect(resGuarded).toBeNull() + expect(writeFn).not.toHaveBeenCalled() + }) + + it('resetHostPartitionWriteStateForTest clears write chains and generation counters', async () => { + resetHostPartitionWriteStateForTest() + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: test fixture mock + const hostId = 'ssh:target-1' as ExecutionHostId + + const write1 = await enqueueHostPartitionWrite(hostId, async () => 'first', { generation: 10 }) + expect(write1).toBe('first') + + resetHostPartitionWriteStateForTest() + + // With a lower generation number after reset, it should not be treated as superseded + const writeAfterReset = await enqueueHostPartitionWrite(hostId, async () => 'after-reset', { + generation: 1 + }) + expect(writeAfterReset).toBe('after-reset') + }) +}) diff --git a/src/renderer/src/lib/workspace-session-host-write-queue.ts b/src/renderer/src/lib/workspace-session-host-write-queue.ts new file mode 100644 index 000000000000..a3feae5aad06 --- /dev/null +++ b/src/renderer/src/lib/workspace-session-host-write-queue.ts @@ -0,0 +1,142 @@ +import { parseExecutionHostId, type ExecutionHostId } from '../../../shared/execution-host' +import { + hostHasAnsweredForTarget, + type RemoteWorkspaceTestimonyState +} from './remote-workspace-host-testimony' + +export type HostPartitionWriteOptions = { + generation?: number + shouldSkip?: () => boolean + state?: RemoteWorkspaceTestimonyState +} + +const hostPartitionWriteChains = new Map>() +const hostPartitionWriteGenerations = new Map() + +export function resetHostPartitionWriteStateForTest(): void { + hostPartitionWriteChains.clear() + hostPartitionWriteGenerations.clear() +} + +export function enqueueHostPartitionWrite( + hostId: ExecutionHostId, + state: RemoteWorkspaceTestimonyState, + perform: () => Promise, + options?: HostPartitionWriteOptions +): Promise +export function enqueueHostPartitionWrite( + hostId: ExecutionHostId, + perform: () => Promise, + options?: HostPartitionWriteOptions +): Promise +export function enqueueHostPartitionWrite( + hostId: ExecutionHostId, + state: RemoteWorkspaceTestimonyState, + perform: () => Promise, + options?: HostPartitionWriteOptions +): Promise +export function enqueueHostPartitionWrite( + hostId: ExecutionHostId, + performOrState: (() => Promise) | RemoteWorkspaceTestimonyState, + optionsOrPerform?: HostPartitionWriteOptions | (() => Promise), + maybeOptions?: HostPartitionWriteOptions +): Promise { + let perform: () => Promise + let options: HostPartitionWriteOptions | undefined + + if (typeof performOrState === 'function') { + perform = performOrState + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: overloaded parameter discrimination + options = optionsOrPerform as HostPartitionWriteOptions | undefined + } else { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: overloaded parameter discrimination + perform = optionsOrPerform as () => Promise + options = { + ...maybeOptions, + state: performOrState + } + } + + const state = options?.state + const parsed = parseExecutionHostId(hostId) + const targetId = parsed?.kind === 'ssh' ? parsed.targetId : null + const preparedWithoutTestimony = + targetId !== null && state ? !hostHasAnsweredForTarget(state, targetId) : false + + const currentGen = hostPartitionWriteGenerations.get(hostId) ?? 0 + if (options?.generation !== undefined && options.generation < currentGen) { + return Promise.resolve(null) + } + + const assignedGen = options?.generation ?? currentGen + 1 + hostPartitionWriteGenerations.set(hostId, Math.max(currentGen, assignedGen)) + + const shouldDiscard = (): boolean => { + if (options?.shouldSkip?.()) { + return true + } + if ( + preparedWithoutTestimony && + targetId !== null && + state && + hostHasAnsweredForTarget(state, targetId) + ) { + return true + } + if ((hostPartitionWriteGenerations.get(hostId) ?? 0) > assignedGen) { + return true + } + return false + } + + const inFlight = hostPartitionWriteChains.get(hostId) + if (!inFlight) { + if (shouldDiscard()) { + return Promise.resolve(null) + } + + let resolveInFlight!: () => void + const inFlightPromise = new Promise((resolve) => { + resolveInFlight = resolve + }) + hostPartitionWriteChains.set(hostId, inFlightPromise) + + let result: Promise + try { + result = perform() + } catch (err) { + resolveInFlight() + hostPartitionWriteChains.delete(hostId) + return Promise.reject(err) + } + + void result.finally(() => { + resolveInFlight() + if (hostPartitionWriteChains.get(hostId) === inFlightPromise) { + hostPartitionWriteChains.delete(hostId) + } + }) + return result + } + + const task = inFlight.then(async (): Promise => { + if (shouldDiscard()) { + return null + } + return await perform() + }) + + let resolveTracked!: () => void + const trackedPromise = new Promise((resolve) => { + resolveTracked = resolve + }) + void task.finally(() => { + resolveTracked() + if (hostPartitionWriteChains.get(hostId) === trackedPromise) { + hostPartitionWriteChains.delete(hostId) + } + }) + hostPartitionWriteChains.set(hostId, trackedPromise) + + return task +} diff --git a/src/renderer/src/lib/workspace-session-host-write-race.test.ts b/src/renderer/src/lib/workspace-session-host-write-race.test.ts index 77b0b0e622a8..ac6b59e9a9e8 100644 --- a/src/renderer/src/lib/workspace-session-host-write-race.test.ts +++ b/src/renderer/src/lib/workspace-session-host-write-race.test.ts @@ -121,6 +121,7 @@ describe('GAP-03 Write Race Protection', () => { contestedPrimaryHostBySessionKey: read.contestedPrimaryHostBySessionKey } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: test fixture mock for DurableSessionApi await persistWorkspaceSessionByHost(api as never, read.session, state) expect(captured[SSH_HOST_ID]).toBeUndefined() @@ -188,6 +189,7 @@ describe('GAP-03 Write Race Protection', () => { contestedPrimaryHostBySessionKey: read.contestedPrimaryHostBySessionKey } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: test fixture mock for DurableSessionApi await persistWorkspaceSessionByHost(api as never, payloadWithFreshLocalActivity, state) // The write race, demonstrated: WORKTREE_ID's genuinely-closed tabs come back, even though the @@ -256,6 +258,7 @@ describe('GAP-03 Write Race Protection', () => { remoteWorkspaceSyncStatusByTargetId: { [TARGET_ID]: { phase: 'synced' } } } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: test fixture mock for DurableSessionApi await persistWorkspaceSessionByHost(api as never, payloadWithFreshLocalActivity, state) expect(captured[SSH_HOST_ID]?.tabsByWorktree?.[WORKTREE_ID]).toBeUndefined() @@ -343,6 +346,7 @@ describe('GAP-03 Write Race Protection', () => { releaseW0 = resolve }) const w0Promise = persistWorkspaceSessionByHost( + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: test fixture mock for DurableSessionApi { ...api, set: async (_payload: WorkspaceSessionState, hostId?: ExecutionHostId) => { @@ -362,6 +366,7 @@ describe('GAP-03 Write Race Protection', () => { // W1 is dispatched while W0 is in-flight: W1 carries the stale declined tabs (prepared without testimony) // and queues behind W0 on SSH_HOST_ID. const writePromise = persistWorkspaceSessionByHost( + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: test fixture mock for DurableSessionApi api as never, payloadWithFreshLocalActivity, state @@ -410,6 +415,7 @@ describe('GAP-03 regression: a declined tab must park and restore its dependent [SIBLING_WORKTREE_ID]: [tab('tab-4', SIBLING_WORKTREE_ID)] }, terminalLayoutsByTabId: { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: test fixture mock for TerminalLayout 'tab-2': { root: null, activeLeafId: null, expandedLeafId: null } as never }, remoteSessionIdsByTabId: { @@ -461,6 +467,7 @@ describe('GAP-03 regression: a declined tab must park and restore its dependent contestedPrimaryHostBySessionKey: read.contestedPrimaryHostBySessionKey } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: test fixture mock for DurableSessionApi await persistWorkspaceSessionByHost(api as never, payloadWithFreshLocalActivity, state) // The worktree-keyed row already round-trips correctly (pinned by the sibling test above). diff --git a/tests/e2e/gap-03-offline-reconnect-tab-resurrection.spec.ts b/tests/e2e/gap-03-offline-reconnect-tab-resurrection.spec.ts index e842f9664a90..44b041d80a61 100644 --- a/tests/e2e/gap-03-offline-reconnect-tab-resurrection.spec.ts +++ b/tests/e2e/gap-03-offline-reconnect-tab-resurrection.spec.ts @@ -84,6 +84,7 @@ function profilePath(userDataDir: string): string { } function readProfile(userDataDir: string): SessionProfile { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: JSON.parse result deserialized into SessionProfile fixture return JSON.parse(readFileSync(profilePath(userDataDir), 'utf8')) as SessionProfile } From 460193ab5094e345ec6ca35e3cf4cbc31ea6fbba Mon Sep 17 00:00:00 2001 From: Lesley Murfin Date: Mon, 28 Sep 2026 04:05:39 -0600 Subject: [PATCH 15/23] fix(GAP-03): resolve multi-agent review findings across write-queue, hydration, and adoption - Guard host write generation advancement by replaceable/explicit-generation flag, preventing non-replaceable partial patches from superseding durable persists. - Support live testimony state resolution via getLiveState thunk and global provider in enqueueHostPartitionWrite to discard stale pre-testimony writes upon testimony arrival. - Normalize tab- and pane-keyed worktree ids in partitionRowsTheWriteWontReturn and resolve worktree claims in attachHostSessionShadow. - Preserve #12723 empty placeholder rows in base tabsByWorktree during stranded adoption. - Extract partitionRowsTheWriteWontReturn to workspace-session-stranded-partition-parking.ts to honor max-lines budget without disabling rules. - Guarantee Docker cleanup in e2e spec via nested finally blocks. - Approved by riley-pr-agent, riley-pr-check, and riley-the-maintainer. --- .../app-shell/use-app-session-persistence.ts | 19 +- .../mobile-terminal-close-ipc-bridge.ts | 5 +- .../lib/workspace-session-host-contention.ts | 29 +++- .../lib/workspace-session-host-hydration.ts | 4 +- ...ace-session-host-offline-reconnect.test.ts | 35 ++++ .../lib/workspace-session-host-persistence.ts | 28 ++- ...workspace-session-host-write-queue.test.ts | 162 +++++++++++++++++- .../lib/workspace-session-host-write-queue.ts | 55 ++++-- .../workspace-session-host-write-race.test.ts | 15 +- ...ace-session-stranded-partition-adoption.ts | 71 ++------ ...pace-session-stranded-partition-parking.ts | 88 ++++++++++ ...offline-reconnect-tab-resurrection.spec.ts | 26 +-- 12 files changed, 431 insertions(+), 106 deletions(-) create mode 100644 src/shared/workspace-session-stranded-partition-parking.ts diff --git a/src/renderer/src/app-shell/use-app-session-persistence.ts b/src/renderer/src/app-shell/use-app-session-persistence.ts index 6574438265de..2d148bbdca73 100644 --- a/src/renderer/src/app-shell/use-app-session-persistence.ts +++ b/src/renderer/src/app-shell/use-app-session-persistence.ts @@ -17,7 +17,8 @@ import { } from '../lib/workspace-session' import { buildWorkspaceSessionHostSnapshots, - patchWorkspaceSessionByHost + patchWorkspaceSessionByHost, + registerLiveTestimonyProvider } from '../lib/workspace-session-host-persistence' import { createShutdownCheckpointBeforeUnloadHandler, @@ -118,7 +119,12 @@ function captureUploadedDirectSshLayoutEdits( */ export function useAppSessionPersistence(): void { useEffect(() => registerUpdaterBeforeUnloadBypass(), []) - + useEffect(() => { + registerLiveTestimonyProvider(() => useAppStore.getState()) + return () => { + registerLiveTestimonyProvider(undefined) + } + }, []) // Why: session persistence only writes to disk; a Zustand subscribe() outside React drops ~15 render-cycle subscriptions and their re-renders on every tab/file/browser change. useEffect(() => { return createSessionWriteSubscriber({ @@ -129,11 +135,10 @@ export function useAppSessionPersistence(): void { persist: ({ patch }) => { const state = useAppStore.getState() // Why: route each host's worktree-scoped slice to its own partition; the upload chain below orders after the local write. - const { localWrite, written } = patchWorkspaceSessionByHost( - window.api.session, - patch, - state - ) + const { localWrite, written } = patchWorkspaceSessionByHost(window.api.session, patch, { + ...state, + getLiveTestimonyState: () => useAppStore.getState() + }) const uploadAuthorities = captureRemoteWorkspaceUploadAuthorities(state) const pendingLayoutEdits = state.pendingDirectSshLayoutEditsByTabId if (uploadAuthorities.length > 0) { diff --git a/src/renderer/src/hooks/ipc-events/mobile-terminal-close-ipc-bridge.ts b/src/renderer/src/hooks/ipc-events/mobile-terminal-close-ipc-bridge.ts index 02c020d09058..0e8420a3612e 100644 --- a/src/renderer/src/hooks/ipc-events/mobile-terminal-close-ipc-bridge.ts +++ b/src/renderer/src/hooks/ipc-events/mobile-terminal-close-ipc-bridge.ts @@ -108,7 +108,10 @@ export function registerMobileAndTerminalCloseIpcBridge( await persistWorkspaceSessionByHost( window.api.session, buildWorkspaceSessionPayload(state), - state + { + ...state, + getLiveTestimonyState: () => useAppStore.getState() + } ) respond() })().catch((error: unknown) => { diff --git a/src/renderer/src/lib/workspace-session-host-contention.ts b/src/renderer/src/lib/workspace-session-host-contention.ts index 161bb6a1644f..fe718de009b9 100644 --- a/src/renderer/src/lib/workspace-session-host-contention.ts +++ b/src/renderer/src/lib/workspace-session-host-contention.ts @@ -9,7 +9,9 @@ import { normalizeWorkspaceSessionKeyToWorkspaceId } from '../../../shared/works import { WORKSPACE_SESSION_FIELD_OWNERSHIP } from '../../../shared/workspace-session-host-field-ownership' import { workspaceSessionPartitionHostId } from '../../../shared/workspace-session-partition-owner' import { + buildWorktreeIdByTabId, isWorkspaceSessionRecord, + worktreeIdForPaneKey, type WorkspaceSessionRecord } from '../../../shared/workspace-session-host-records' import type { WorkspaceRuntimeOwnerProjection } from './workspace-runtime-host-ownership' @@ -291,7 +293,23 @@ export function attachHostSessionShadow( if (!slice || !shadowSlice) { continue } + const worktreeIdByTabId = buildWorktreeIdByTabId(shadowSlice) + for (const [worktreeKey, tabs] of Object.entries(slice.tabsByWorktree ?? {})) { + for (const tab of tabs) { + if (!worktreeIdByTabId.has(tab.id)) { + worktreeIdByTabId.set(tab.id, tab.worktreeId ?? worktreeKey) + } + } + } + for (const tabs of Object.values(slice.unifiedTabs ?? {})) { + for (const tab of tabs) { + if (!worktreeIdByTabId.has(tab.id)) { + worktreeIdByTabId.set(tab.id, tab.worktreeId) + } + } + } for (const field of PARKABLE_HOST_SESSION_FIELDS) { + const ownership = WORKSPACE_SESSION_FIELD_OWNERSHIP[field] const parked = shadowSlice[field] if (!isWorkspaceSessionRecord(parked)) { continue @@ -307,7 +325,16 @@ export function attachHostSessionShadow( ;(slice as WorkspaceSessionRecord)[field] = target } for (const [key, entry] of Object.entries(parked)) { - if (Object.hasOwn(target, key) || !hostStillClaimsKey(claims, key, hostId)) { + if (Object.hasOwn(target, key)) { + continue + } + const worktreeId = + ownership === 'worktreeKeyed' + ? key + : ownership === 'tabKeyed' + ? worktreeIdByTabId.get(key) + : worktreeIdForPaneKey(worktreeIdByTabId, key) + if (!hostStillClaimsKey(claims, worktreeId ?? key, hostId)) { continue } target[key] = entry diff --git a/src/renderer/src/lib/workspace-session-host-hydration.ts b/src/renderer/src/lib/workspace-session-host-hydration.ts index c22059005b8c..35193b836ae2 100644 --- a/src/renderer/src/lib/workspace-session-host-hydration.ts +++ b/src/renderer/src/lib/workspace-session-host-hydration.ts @@ -158,7 +158,7 @@ export async function fetchWorkspaceSessionFromHosts( .session } -function wasTargetConnectedAtLastShutdown( +export function wasTargetConnectedAtLastShutdown( session: WorkspaceSessionState, targetId: string | null ): boolean { @@ -171,6 +171,8 @@ function wasTargetConnectedAtLastShutdown( // When activeConnectionIdsAtShutdown is undefined (older persisted sessions, interrupted // shutdowns), infer connection status from remoteSessionIdsByTabId: if the base session still // holds a remote PTY session for this target, the client was connected to it, so do not decline. + // Residual remoteSessionIdsByTabId on the SSH partition mirror itself are NOT inspected: + // they are the stale cache GAP-03 exists to decline. for (const sessionId of Object.values(session.remoteSessionIdsByTabId ?? {})) { if (parseAppSshPtyId(sessionId)?.connectionId === targetId) { return true diff --git a/src/renderer/src/lib/workspace-session-host-offline-reconnect.test.ts b/src/renderer/src/lib/workspace-session-host-offline-reconnect.test.ts index cd6fd190b411..7cc855cf42b0 100644 --- a/src/renderer/src/lib/workspace-session-host-offline-reconnect.test.ts +++ b/src/renderer/src/lib/workspace-session-host-offline-reconnect.test.ts @@ -109,6 +109,41 @@ describe('GAP-03 regression: canonical-keyed host rows must still be declined', .map((entry) => entry.id) expect(survivingTabIds).toEqual([]) }) + it('normalizes canonical worktree keys so live rows are not falsely parked into the write shadow', async () => { + // When host partition stores tab/pane entries under canonical `worktree:${id}` keys, + // live rows that ARE adopted must not be falsely parked into the write shadow. + const read = await fetchWorkspaceSessionWithRuntimeHostOwners( + partitionedApi({ + local: session({ + activeConnectionIdsAtShutdown: [TARGET_ID] + }), + [SSH_HOST_ID]: session({ + tabsByWorktree: { + [`worktree:${WORKTREE_ID}`]: [tab('tab-1', WORKTREE_ID)] + }, + terminalLayoutsByTabId: { + 'tab-1': { root: null, activeLeafId: null, expandedLeafId: null } + } + }) + }), + [{ id: REPO_ID, connectionId: TARGET_ID, executionHostId: SSH_HOST_ID }] + ) + + // The tab is live and adopted into the session + const survivingTabIds = Object.entries(read.session.tabsByWorktree ?? {}) + .flatMap(([, tabs]) => tabs) + .map((entry) => entry.id) + expect(survivingTabIds).toContain('tab-1') + + // Because the tab is live / adopted, its rows MUST NOT be parked into the write shadow + const shadowTabs = read.contestedHostWorkspaceSessions[SSH_HOST_ID]?.tabsByWorktree ?? {} + expect(shadowTabs[WORKTREE_ID]).toBeUndefined() + expect(shadowTabs[`worktree:${WORKTREE_ID}`]).toBeUndefined() + + const shadowLayouts = + read.contestedHostWorkspaceSessions[SSH_HOST_ID]?.terminalLayoutsByTabId ?? {} + expect(shadowLayouts['tab-1']).toBeUndefined() + }) }) describe('GAP-03 regression: unknown shutdown status must not decline tabs if remoteSessionIdsByTabId indicates connection', () => { diff --git a/src/renderer/src/lib/workspace-session-host-persistence.ts b/src/renderer/src/lib/workspace-session-host-persistence.ts index 681face27079..d67e3ff70888 100644 --- a/src/renderer/src/lib/workspace-session-host-persistence.ts +++ b/src/renderer/src/lib/workspace-session-host-persistence.ts @@ -40,10 +40,11 @@ import type { RemoteWorkspaceTestimonyState } from './remote-workspace-host-test import { shadowRowsTheHostHasNotAnswered } from './workspace-session-host-shadow-testimony' import { enqueueHostPartitionWrite, + registerLiveTestimonyProvider, resetHostPartitionWriteStateForTest } from './workspace-session-host-write-queue' -export { resetHostPartitionWriteStateForTest } +export { registerLiveTestimonyProvider, resetHostPartitionWriteStateForTest } export type HostPersistenceState = WorkspaceTabOwnerCatalog & { repos: readonly Pick[] @@ -61,6 +62,7 @@ export type HostPersistenceState = WorkspaceTabOwnerCatalog & { /** Partition each restored session key was read from. Routing honours it so a write returns rows * to their own partition instead of re-deriving an owner the read never agreed to. */ contestedPrimaryHostBySessionKey?: Record + getLiveTestimonyState?: () => RemoteWorkspaceTestimonyState } & RemoteWorkspaceTestimonyState type SessionApi = { @@ -255,10 +257,18 @@ export function patchWorkspaceSessionByHost( const slices = splitWorkspaceSessionForWrite(patch as WorkspaceSessionState, state, 'patch') const local = (slices[LOCAL_EXECUTION_HOST_ID] ?? patch) as WorkspaceSessionPatch const localWrite = api.patch(local) -const hostWrites = nonLocalHostSessionEntries(slices).map(([hostId, slice]) => - enqueueHostPartitionWrite(hostId, state, () => - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: slice is a partition subset matching WorkspaceSessionPatch - api.patch(slice as WorkspaceSessionPatch, hostId) + const hostWrites = nonLocalHostSessionEntries(slices).map(([hostId, slice]) => + enqueueHostPartitionWrite( + hostId, + state, + () => + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: slice is a partition subset matching WorkspaceSessionPatch + api.patch(slice as WorkspaceSessionPatch, hostId), + { + state, + replaceable: false, + getLiveState: state.getLiveTestimonyState + } ).catch((err: unknown) => { console.warn(`[session] host partition patch failed for ${hostId}:`, err) throw err @@ -289,7 +299,13 @@ export async function persistWorkspaceSessionByHost( const slices = splitWorkspaceSessionForWrite(payload, state, 'replace') const writes: Promise[] = [api.set(slices[LOCAL_EXECUTION_HOST_ID] ?? payload)] for (const [hostId, slice] of nonLocalHostSessionEntries(slices)) { - writes.push(enqueueHostPartitionWrite(hostId, state, () => api.set(slice, hostId))) + writes.push( + enqueueHostPartitionWrite(hostId, state, () => api.set(slice, hostId), { + state, + replaceable: true, + getLiveState: state.getLiveTestimonyState + }) + ) } await Promise.all(writes) await api.flush() diff --git a/src/renderer/src/lib/workspace-session-host-write-queue.test.ts b/src/renderer/src/lib/workspace-session-host-write-queue.test.ts index 8e7d9d712e8a..5ca8152e5af1 100644 --- a/src/renderer/src/lib/workspace-session-host-write-queue.test.ts +++ b/src/renderer/src/lib/workspace-session-host-write-queue.test.ts @@ -1,8 +1,10 @@ import { describe, expect, it, vi } from 'vitest' import type { ExecutionHostId } from '../../../shared/execution-host' +import type { RemoteWorkspaceTestimonyState } from './remote-workspace-host-testimony' import { enqueueHostPartitionWrite, - resetHostPartitionWriteStateForTest + resetHostPartitionWriteStateForTest, + type HostPartitionWriteOptions } from './workspace-session-host-write-queue' describe('workspace-session-host-write-queue', () => { @@ -159,6 +161,164 @@ describe('workspace-session-host-write-queue', () => { expect(writeFn).not.toHaveBeenCalled() }) + it('does not discard a queued write with replaceable: false when a newer generation write is enqueued', async () => { + resetHostPartitionWriteStateForTest() + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: test fixture mock + const hostId = 'ssh:target-1' as ExecutionHostId + + const executionLog: string[] = [] + + let resolveActive: (() => void) | undefined + const activeBlocker = new Promise((resolve) => { + resolveActive = resolve + }) + + const activeWrite = enqueueHostPartitionWrite( + hostId, + async () => { + executionLog.push('active') + await activeBlocker + return 'active-res' + }, + { generation: 1 } + ) + + const nonReplaceableWrite = enqueueHostPartitionWrite( + hostId, + async () => { + executionLog.push('partial-patch') + return 'partial-patch-res' + }, + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Slice 2 test fixture exercising planned replaceable option + { generation: 2, replaceable: false } as HostPartitionWriteOptions + ) + + const latestWrite = enqueueHostPartitionWrite( + hostId, + async () => { + executionLog.push('latest') + return 'latest-res' + }, + { generation: 3 } + ) + + resolveActive?.() + + const [resActive, resNonReplaceable, resLatest] = await Promise.all([ + activeWrite, + nonReplaceableWrite, + latestWrite + ]) + + expect(resActive).toBe('active-res') + expect(resNonReplaceable).toBe('partial-patch-res') + expect(resLatest).toBe('latest-res') + expect(executionLog).toEqual(['active', 'partial-patch', 'latest']) + }) + + it('does not discard a queued replaceable persist when a subsequent non-replaceable patch is enqueued', async () => { + resetHostPartitionWriteStateForTest() + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: test fixture mock + const hostId = 'ssh:target-1' as ExecutionHostId + + const executionLog: string[] = [] + + let resolveActive: (() => void) | undefined + const activeBlocker = new Promise((resolve) => { + resolveActive = resolve + }) + + const activeWrite = enqueueHostPartitionWrite( + hostId, + async () => { + executionLog.push('active') + await activeBlocker + return 'active-res' + }, + { generation: 1 } + ) + + const replaceablePersist = enqueueHostPartitionWrite( + hostId, + async () => { + executionLog.push('replaceable-persist') + return 'replaceable-persist-res' + }, + { generation: 2, replaceable: true } + ) + + const subsequentPatch = enqueueHostPartitionWrite( + hostId, + async () => { + executionLog.push('subsequent-patch') + return 'subsequent-patch-res' + }, + { replaceable: false } + ) + + resolveActive?.() + + const [resActive, resPersist, resPatch] = await Promise.all([ + activeWrite, + replaceablePersist, + subsequentPatch + ]) + + expect(resActive).toBe('active-res') + expect(resPersist).toBe('replaceable-persist-res') + expect(resPatch).toBe('subsequent-patch-res') + expect(executionLog).toEqual(['active', 'replaceable-persist', 'subsequent-patch']) + }) + + it('discards a pre-testimony write when live testimony lands before dequeuing even with an immutable initial state', async () => { + resetHostPartitionWriteStateForTest() + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: test fixture mock + const hostId = 'ssh:target-1' as ExecutionHostId + + const initialTestimonyState: RemoteWorkspaceTestimonyState = Object.freeze({ + remoteWorkspaceHydratedTargetIds: new Set(), + remoteWorkspaceSyncStatusByTargetId: {} + }) + + let liveState: RemoteWorkspaceTestimonyState = initialTestimonyState + + let resolveActive: (() => void) | undefined + const activeBlocker = new Promise((resolve) => { + resolveActive = resolve + }) + + const activeWrite = enqueueHostPartitionWrite(hostId, async () => { + await activeBlocker + return 'active-done' + }) + + const writeFn = vi.fn().mockResolvedValue('testimony-guarded-res') + + const guardedWrite = enqueueHostPartitionWrite( + hostId, + initialTestimonyState, + writeFn, + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Slice 2 test fixture exercising planned getLiveState option + { getLiveState: () => liveState } as HostPartitionWriteOptions + ) + + // Live testimony arrives before the active write finishes, updating liveState + // while initialTestimonyState remains frozen and untouched. + liveState = { + remoteWorkspaceHydratedTargetIds: new Set(['target-1']), + remoteWorkspaceSyncStatusByTargetId: { + 'target-1': { phase: 'idle' } + } + } + + resolveActive?.() + + const [resActive, resGuarded] = await Promise.all([activeWrite, guardedWrite]) + expect(resActive).toBe('active-done') + expect(resGuarded).toBeNull() + expect(writeFn).not.toHaveBeenCalled() + }) + it('resetHostPartitionWriteStateForTest clears write chains and generation counters', async () => { resetHostPartitionWriteStateForTest() // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: test fixture mock diff --git a/src/renderer/src/lib/workspace-session-host-write-queue.ts b/src/renderer/src/lib/workspace-session-host-write-queue.ts index a3feae5aad06..fc41293dd362 100644 --- a/src/renderer/src/lib/workspace-session-host-write-queue.ts +++ b/src/renderer/src/lib/workspace-session-host-write-queue.ts @@ -8,6 +8,16 @@ export type HostPartitionWriteOptions = { generation?: number shouldSkip?: () => boolean state?: RemoteWorkspaceTestimonyState + getLiveState?: () => RemoteWorkspaceTestimonyState + replaceable?: boolean +} + +let globalLiveTestimonyProvider: (() => RemoteWorkspaceTestimonyState) | undefined + +export function registerLiveTestimonyProvider( + provider: (() => RemoteWorkspaceTestimonyState) | undefined +): void { + globalLiveTestimonyProvider = provider } const hostPartitionWriteChains = new Map>() @@ -16,6 +26,7 @@ const hostPartitionWriteGenerations = new Map() export function resetHostPartitionWriteStateForTest(): void { hostPartitionWriteChains.clear() hostPartitionWriteGenerations.clear() + globalLiveTestimonyProvider = undefined } export function enqueueHostPartitionWrite( @@ -68,8 +79,14 @@ export function enqueueHostPartitionWrite( return Promise.resolve(null) } - const assignedGen = options?.generation ?? currentGen + 1 - hostPartitionWriteGenerations.set(hostId, Math.max(currentGen, assignedGen)) + const advancesGeneration = options?.generation !== undefined || (options?.replaceable ?? false) + const assignedGen = options?.generation ?? (advancesGeneration ? currentGen + 1 : currentGen) + if (advancesGeneration) { + hostPartitionWriteGenerations.set(hostId, Math.max(currentGen, assignedGen)) + } + + const resolveLiveState = (): RemoteWorkspaceTestimonyState => + options?.getLiveState?.() ?? globalLiveTestimonyProvider?.() ?? state ?? {} const shouldDiscard = (): boolean => { if (options?.shouldSkip?.()) { @@ -78,12 +95,12 @@ export function enqueueHostPartitionWrite( if ( preparedWithoutTestimony && targetId !== null && - state && - hostHasAnsweredForTarget(state, targetId) + hostHasAnsweredForTarget(resolveLiveState(), targetId) ) { return true } - if ((hostPartitionWriteGenerations.get(hostId) ?? 0) > assignedGen) { + const isReplaceable = options?.replaceable ?? options?.generation !== undefined + if (isReplaceable && (hostPartitionWriteGenerations.get(hostId) ?? 0) > assignedGen) { return true } return false @@ -110,12 +127,14 @@ export function enqueueHostPartitionWrite( return Promise.reject(err) } - void result.finally(() => { - resolveInFlight() - if (hostPartitionWriteChains.get(hostId) === inFlightPromise) { - hostPartitionWriteChains.delete(hostId) - } - }) + void result + .finally(() => { + resolveInFlight() + if (hostPartitionWriteChains.get(hostId) === inFlightPromise) { + hostPartitionWriteChains.delete(hostId) + } + }) + .catch(() => {}) return result } @@ -130,12 +149,14 @@ export function enqueueHostPartitionWrite( const trackedPromise = new Promise((resolve) => { resolveTracked = resolve }) - void task.finally(() => { - resolveTracked() - if (hostPartitionWriteChains.get(hostId) === trackedPromise) { - hostPartitionWriteChains.delete(hostId) - } - }) + void task + .finally(() => { + resolveTracked() + if (hostPartitionWriteChains.get(hostId) === trackedPromise) { + hostPartitionWriteChains.delete(hostId) + } + }) + .catch(() => {}) hostPartitionWriteChains.set(hostId, trackedPromise) return task diff --git a/src/renderer/src/lib/workspace-session-host-write-race.test.ts b/src/renderer/src/lib/workspace-session-host-write-race.test.ts index ac6b59e9a9e8..d1e9736d1fc4 100644 --- a/src/renderer/src/lib/workspace-session-host-write-race.test.ts +++ b/src/renderer/src/lib/workspace-session-host-write-race.test.ts @@ -28,7 +28,7 @@ * unconditionally dropped parked rows without positive host testimony and caused unrecoverable data * loss on Concurrent Active Edits. */ -import { describe, expect, it } from 'vitest' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' import { getDefaultWorkspaceSession } from '../../../shared/constants' import type { ExecutionHostId } from '../../../shared/execution-host' import { normalizeExecutionHostId } from '../../../shared/execution-host' @@ -39,6 +39,7 @@ import { persistWorkspaceSessionByHost, type HostPersistenceState } from './workspace-session-host-persistence' +import { resetHostPartitionWriteStateForTest } from './workspace-session-host-write-queue' const TARGET_ID = 'target-1' const SSH_HOST_ID: ExecutionHostId = `ssh:${TARGET_ID}` @@ -89,6 +90,14 @@ function capturingApi() { } } +beforeEach(() => { + resetHostPartitionWriteStateForTest() +}) + +afterEach(() => { + resetHostPartitionWriteStateForTest() +}) + describe('GAP-03 Write Race Protection', () => { it('a wake-triggered write for a worktree with nothing else on the same host never even touches that host partition, so the declined tabs cannot round-trip back', async () => { const read = await fetchWorkspaceSessionWithRuntimeHostOwners( @@ -408,7 +417,9 @@ describe('GAP-03 regression: a declined tab must park and restore its dependent // worktree-keyed `tabsByWorktree` row for the same declined tabs correctly comes back. const read = await fetchWorkspaceSessionWithRuntimeHostOwners( partitionedApi({ - local: session({}), + local: session({ + activeConnectionIdsAtShutdown: [] + }), [SSH_HOST_ID]: session({ tabsByWorktree: { [WORKTREE_ID]: [tab('tab-2', WORKTREE_ID), tab('tab-3', WORKTREE_ID)], diff --git a/src/shared/workspace-session-stranded-partition-adoption.ts b/src/shared/workspace-session-stranded-partition-adoption.ts index 817adef16b38..a4c6a104b1e4 100644 --- a/src/shared/workspace-session-stranded-partition-adoption.ts +++ b/src/shared/workspace-session-stranded-partition-adoption.ts @@ -3,7 +3,7 @@ import { WORKSPACE_SESSION_FIELD_OWNERSHIP, type WorkspaceSessionFieldOwnership } from './workspace-session-host-field-ownership' -import { normalizeWorkspaceSessionKeyToWorkspaceId } from './workspace-scope' +import { normalizeWorkspaceSessionKeyToWorkspaceId, worktreeWorkspaceKey } from './workspace-scope' import { isWorktreeHostIdentity as isHostQualifiedSessionKey } from './worktree/host-qualified-identity' import { buildWorktreeIdByFileId, @@ -190,64 +190,7 @@ function adoptRecord( ;(next as KeyedRecord)[field] = merged } -/** - * The worktree-keyed rows a partition holds for workspaces the write will not route back to it, - * plus the tab- and pane-keyed rows those declined tabs own (`terminalLayoutsByTabId`, - * `remoteSessionIdsByTabId`, `localOnlyScrollbackByTabId`, `terminalPtyIncarnationsByPaneKey`). - * - * Parked rather than dropped. A partition write replaces each field with exactly what the unified - * session routed there, so a row this read left out — declined as residue, withheld as contested, - * or skipped because the base already holds the live copy — is erased the moment any sibling - * workspace writes the same partition. `attachHostSessionShadow` puts these back into the slice - * first, which is the protection a contested runtime co-claimant already gets. Declining to show a - * row must never mean deleting it: docs/reference/ssh-execution-boundary.md makes leak, never kill, - * the safe direction, and a row no partition holds at all is unrecoverable. - * - * Why the tab/pane sweep and not just `tabsByWorktree`: a declined worktree's `terminalLayoutsByTabId` - * and `remoteSessionIdsByTabId` rows are keyed by tab id, not worktree id, so the worktree-keyed - * walk above never sees them. Without this, a write landing on the same SSH partition before a live - * answer keeps the declined tab (via the worktree-keyed park) but drops its layout and relay-session - * rows, leaving the restored tab with no pane to reattach to. - */ -export function partitionRowsTheWriteWontReturn( - host: WorkspaceSessionState, - adoptedWorkspaceIds: ReadonlySet -): WorkspaceSessionState | null { - let parked: KeyedRecord | null = null - const worktreeIdByTabIdOnHost = buildWorktreeIdByTabId(host) - const isDeclinedWorktree = (worktreeId: string | undefined): boolean => - worktreeId !== undefined && !adoptedWorkspaceIds.has(worktreeId) - for (const field of SESSION_FIELDS) { - const ownership: WorkspaceSessionFieldOwnership = WORKSPACE_SESSION_FIELD_OWNERSHIP[field] - if (ownership !== 'worktreeKeyed' && ownership !== 'tabKeyed' && ownership !== 'paneKeyed') { - continue - } - const record = asRecord(host[field]) - if (!record) { - continue - } - let kept: KeyedRecord | null = null - for (const [key, entry] of Object.entries(record)) { - const rowWorktreeId = - ownership === 'worktreeKeyed' - ? normalizeWorkspaceSessionKeyToWorkspaceId(key) - : ownership === 'tabKeyed' - ? worktreeIdByTabIdOnHost.get(key) - : worktreeIdForPaneKey(worktreeIdByTabIdOnHost, key) - if (!isDeclinedWorktree(rowWorktreeId)) { - continue - } - kept ??= {} - kept[key] = entry - } - if (kept) { - parked ??= {} - parked[field] = kept - } - } - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: every key written here is a session field name taken from the ownership table, and every value is that field's own record copied by reference. - return parked as WorkspaceSessionState | null -} +export { partitionRowsTheWriteWontReturn } from './workspace-session-stranded-partition-parking' /** Which workspace an entry of a scoped field belongs to; null for global and host-private fields. */ function entryWorkspaceResolver( @@ -397,8 +340,16 @@ export function adoptStrandedHostPartitionSession( // client's stale pre-disconnect cache, not evidence of anything live. Declining leaves the row // in place for `partitionRowsTheWriteWontReturn` to park rather than deleting it. const hostTabsByWorktree = host.tabsByWorktree ?? {} + const baseTabs = base.tabsByWorktree ?? {} for (const workspaceId of options.reconciledWorktreeIds ?? []) { - if (!adoptable.has(workspaceId) || Object.hasOwn(base.tabsByWorktree ?? {}, workspaceId)) { + const canonicalKey = worktreeWorkspaceKey(workspaceId) + const hasBaseKey = + Object.hasOwn(baseTabs, workspaceId) || + Object.hasOwn(baseTabs, canonicalKey) || + Object.keys(baseTabs).some( + (key) => normalizeWorkspaceSessionKeyToWorkspaceId(key) === workspaceId + ) + if (!adoptable.has(workspaceId) || hasBaseKey) { continue } for (const [key, hostTabs] of Object.entries(hostTabsByWorktree)) { diff --git a/src/shared/workspace-session-stranded-partition-parking.ts b/src/shared/workspace-session-stranded-partition-parking.ts new file mode 100644 index 000000000000..9ec7d5277371 --- /dev/null +++ b/src/shared/workspace-session-stranded-partition-parking.ts @@ -0,0 +1,88 @@ +import type { WorkspaceSessionState } from './workspace-session-state-types' +import { + WORKSPACE_SESSION_FIELD_OWNERSHIP, + type WorkspaceSessionFieldOwnership +} from './workspace-session-host-field-ownership' +import { normalizeWorkspaceSessionKeyToWorkspaceId } from './workspace-scope' +import { buildWorktreeIdByTabId, worktreeIdForPaneKey } from './workspace-session-host-records' + +type KeyedRecord = Record + +// oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Object.keys over the ownership table yields exactly the session field names it is keyed by. +const SESSION_FIELDS = Object.keys( + WORKSPACE_SESSION_FIELD_OWNERSHIP +) as (keyof WorkspaceSessionState)[] + +function asRecord(value: unknown): KeyedRecord | null { + return value && typeof value === 'object' && !Array.isArray(value) + ? // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: checked object shape + (value as KeyedRecord) + : null +} + +/** + * Return rows from a host partition that the upcoming write will NOT return to it: rows for + * workspaces this partition read but did NOT adopt (because the base owned them, the catalog + * attributed them to another host, or GAP-03 declined them). + * + * Why: writing a session slice back to a host partition replaces that partition wholesale. If this + * client read rows from an SSH host that it did not adopt into its active session, the next write + * to that SSH partition would erase those un-adopted rows from disk unless they are parked here + * first, which is the protection a contested runtime co-claimant already gets. Declining to show a + * row must never mean deleting it: docs/reference/ssh-execution-boundary.md makes leak, never kill, + * the safe direction, and a row no partition holds at all is unrecoverable. + * + * Why the tab/pane sweep and not just `tabsByWorktree`: a declined worktree's `terminalLayoutsByTabId` + * and `remoteSessionIdsByTabId` rows are keyed by tab id, not worktree id, so the worktree-keyed + * walk above never sees them. Without this, a write landing on the same SSH partition before a live + * answer keeps the declined tab (via the worktree-keyed park) but drops its layout and relay-session + * rows, leaving the restored tab with no pane to reattach to. + */ +export function partitionRowsTheWriteWontReturn( + host: WorkspaceSessionState, + adoptedWorkspaceIds: ReadonlySet +): WorkspaceSessionState | null { + let parked: KeyedRecord | null = null + const worktreeIdByTabIdOnHost = buildWorktreeIdByTabId(host) + const isDeclinedWorktree = (worktreeId: string | undefined): boolean => { + if (worktreeId === undefined) { + return false + } + const normId = normalizeWorkspaceSessionKeyToWorkspaceId(worktreeId) + return !adoptedWorkspaceIds.has(normId) + } + for (const field of SESSION_FIELDS) { + const ownership: WorkspaceSessionFieldOwnership = WORKSPACE_SESSION_FIELD_OWNERSHIP[field] + if (ownership !== 'worktreeKeyed' && ownership !== 'tabKeyed' && ownership !== 'paneKeyed') { + continue + } + const record = asRecord(host[field]) + if (!record) { + continue + } + let kept: KeyedRecord | null = null + for (const [key, entry] of Object.entries(record)) { + const rawWorktreeId = + ownership === 'worktreeKeyed' + ? key + : ownership === 'tabKeyed' + ? worktreeIdByTabIdOnHost.get(key) + : worktreeIdForPaneKey(worktreeIdByTabIdOnHost, key) + const rowWorktreeId = + rawWorktreeId !== undefined + ? normalizeWorkspaceSessionKeyToWorkspaceId(rawWorktreeId) + : undefined + if (!isDeclinedWorktree(rowWorktreeId)) { + continue + } + kept ??= {} + kept[key] = entry + } + if (kept) { + parked ??= {} + parked[field] = kept + } + } + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: every key written here is a session field name taken from the ownership table, and every value is that field's own record copied by reference. + return parked as WorkspaceSessionState | null +} diff --git a/tests/e2e/gap-03-offline-reconnect-tab-resurrection.spec.ts b/tests/e2e/gap-03-offline-reconnect-tab-resurrection.spec.ts index 44b041d80a61..878e8f2ea47f 100644 --- a/tests/e2e/gap-03-offline-reconnect-tab-resurrection.spec.ts +++ b/tests/e2e/gap-03-offline-reconnect-tab-resurrection.spec.ts @@ -304,13 +304,16 @@ test.describe('GAP-03: offline client reconnect must not resurrect server-closed '#12721 non-regression: an offline-created, never-synced tab must survive reconnect' ).toContain(OFFLINE_DRAFT_TAB_ID) } finally { - for (const app of [secondApp, firstApp]) { - if (app) { - await session.close(app).catch(() => undefined) + try { + for (const app of [secondApp, firstApp]) { + if (app) { + await session.close(app).catch(() => undefined) + } } + await session.dispose() + } finally { + cleanupDockerSshRelayTarget(target) } - await session.dispose() - cleanupDockerSshRelayTarget(target) } }) }) @@ -383,13 +386,16 @@ test.describe('GAP-03 regression: an ordinary restart must not drop live tabs (D 'GAP-03 regression: an ordinary restart with nothing closed must not drop any live tab' ).toEqual(openedTabs.map((t) => t.id).sort()) } finally { - for (const app of [secondApp, firstApp]) { - if (app) { - await session.close(app).catch(() => undefined) + try { + for (const app of [secondApp, firstApp]) { + if (app) { + await session.close(app).catch(() => undefined) + } } + await session.dispose() + } finally { + cleanupDockerSshRelayTarget(target) } - await session.dispose() - cleanupDockerSshRelayTarget(target) } }) }) From e80cd046d0f84579d6852034bc12c15cb5f30d18 Mon Sep 17 00:00:00 2001 From: Lesley Murfin Date: Wed, 30 Sep 2026 12:34:05 -0600 Subject: [PATCH 16/23] fix(GAP-03): clean redundant type assertion in write queue test --- .../src/lib/workspace-session-host-write-queue.test.ts | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/renderer/src/lib/workspace-session-host-write-queue.test.ts b/src/renderer/src/lib/workspace-session-host-write-queue.test.ts index 5ca8152e5af1..951f452f10b1 100644 --- a/src/renderer/src/lib/workspace-session-host-write-queue.test.ts +++ b/src/renderer/src/lib/workspace-session-host-write-queue.test.ts @@ -294,13 +294,9 @@ describe('workspace-session-host-write-queue', () => { const writeFn = vi.fn().mockResolvedValue('testimony-guarded-res') - const guardedWrite = enqueueHostPartitionWrite( - hostId, - initialTestimonyState, - writeFn, - // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: Slice 2 test fixture exercising planned getLiveState option - { getLiveState: () => liveState } as HostPartitionWriteOptions - ) + const guardedWrite = enqueueHostPartitionWrite(hostId, initialTestimonyState, writeFn, { + getLiveState: () => liveState + }) // Live testimony arrives before the active write finishes, updating liveState // while initialTestimonyState remains frozen and untouched. From 96451242f36155d0de0f093c1fcf011fcd88429b Mon Sep 17 00:00:00 2001 From: Lesley Murfin Date: Wed, 30 Sep 2026 22:57:49 -0600 Subject: [PATCH 17/23] fix(GAP-03): discard pre-testimony write only when carrying parked shadow rows and clamp CLOSE_COUNT - Track whether host partition slices actually receive parked shadow rows in attachHostSessionShadow. - Propagate carriesParkedShadowRows through splitWorkspaceSessionForWrite to enqueueHostPartitionWrite. - Gate pre-testimony write discarding on carriesParkedShadowRows, preventing data loss for live local edits and delta patches. - Add regression test ensuring queued pre-testimony writes without parked shadow rows run when live testimony arrives. - Safely parse TAB_COUNT and clamp CLOSE_COUNT to TAB_COUNT in Docker e2e harness. --- .../lib/workspace-session-host-contention.ts | 10 +++-- .../lib/workspace-session-host-persistence.ts | 26 +++++++---- ...workspace-session-host-write-queue.test.ts | 45 +++++++++++++++++++ .../lib/workspace-session-host-write-queue.ts | 7 ++- ...offline-reconnect-tab-resurrection.spec.ts | 9 ++-- 5 files changed, 79 insertions(+), 18 deletions(-) diff --git a/src/renderer/src/lib/workspace-session-host-contention.ts b/src/renderer/src/lib/workspace-session-host-contention.ts index fe718de009b9..5948ab4beb1c 100644 --- a/src/renderer/src/lib/workspace-session-host-contention.ts +++ b/src/renderer/src/lib/workspace-session-host-contention.ts @@ -275,15 +275,17 @@ function hostStillClaimsKey( export type HostSessionWriteMode = 'patch' | 'replace' /** Write parked entries back into their own host's slice so a write for the primary host cannot - * erase a co-claimant's persisted session. Mutates the slices produced by the split. */ + * erase a co-claimant's persisted session. Mutates the slices produced by the split. + * Returns the set of host IDs that actually received one or more parked shadow entries. */ export function attachHostSessionShadow( slices: HostSessionSlices, shadow: HostSessionSlices | undefined, claims: WorktreeHostClaims, mode: HostSessionWriteMode -): void { +): Set { + const hostsWithAttachedShadow = new Set() if (!shadow) { - return + return hostsWithAttachedShadow } for (const [hostId, shadowSlice] of Object.entries(shadow) as [ ExecutionHostId, @@ -338,7 +340,9 @@ export function attachHostSessionShadow( continue } target[key] = entry + hostsWithAttachedShadow.add(hostId) } } } + return hostsWithAttachedShadow } diff --git a/src/renderer/src/lib/workspace-session-host-persistence.ts b/src/renderer/src/lib/workspace-session-host-persistence.ts index d67e3ff70888..7b1e668052b6 100644 --- a/src/renderer/src/lib/workspace-session-host-persistence.ts +++ b/src/renderer/src/lib/workspace-session-host-persistence.ts @@ -227,7 +227,7 @@ function splitWorkspaceSessionForWrite( payload: WorkspaceSessionState, state: HostPersistenceState, mode: HostSessionWriteMode -): HostSessionSlices { +): { slices: HostSessionSlices; hostsWithAttachedShadow: Set } { const routing = buildHostSessionRouting(state) // Why the live catalogs: a debounced patch carries only the fields that changed, so a park // capture's layouts-only patch names no tab rows. Routed by the payload alone, every tab-keyed @@ -237,13 +237,13 @@ function splitWorkspaceSessionForWrite( const slices = splitWorkspaceSessionByHost(payload, routing.hostIdByWorktreeId, { worktreeIdByTabId }) - attachHostSessionShadow( + const hostsWithAttachedShadow = attachHostSessionShadow( slices, shadowRowsTheHostHasNotAnswered(state.contestedHostWorkspaceSessions, state), routing.claims, mode ) - return slices + return { slices, hostsWithAttachedShadow } } /** Patch path of the debounced session writer: split the partial patch by owner host and patch @@ -254,7 +254,11 @@ export function patchWorkspaceSessionByHost( patch: WorkspaceSessionPatch, state: HostPersistenceState ): { localWrite: Promise; written: Promise } { - const slices = splitWorkspaceSessionForWrite(patch as WorkspaceSessionState, state, 'patch') + const { slices, hostsWithAttachedShadow } = splitWorkspaceSessionForWrite( + patch as WorkspaceSessionState, + state, + 'patch' + ) const local = (slices[LOCAL_EXECUTION_HOST_ID] ?? patch) as WorkspaceSessionPatch const localWrite = api.patch(local) const hostWrites = nonLocalHostSessionEntries(slices).map(([hostId, slice]) => @@ -267,7 +271,8 @@ export function patchWorkspaceSessionByHost( { state, replaceable: false, - getLiveState: state.getLiveTestimonyState + getLiveState: state.getLiveTestimonyState, + carriesParkedShadowRows: hostsWithAttachedShadow.has(hostId) } ).catch((err: unknown) => { console.warn(`[session] host partition patch failed for ${hostId}:`, err) @@ -296,14 +301,19 @@ export async function persistWorkspaceSessionByHost( ): Promise { // Why 'replace': api.set swaps the whole partition, so parked rows must ride along even for // fields nothing else routed to this host. - const slices = splitWorkspaceSessionForWrite(payload, state, 'replace') + const { slices, hostsWithAttachedShadow } = splitWorkspaceSessionForWrite( + payload, + state, + 'replace' + ) const writes: Promise[] = [api.set(slices[LOCAL_EXECUTION_HOST_ID] ?? payload)] for (const [hostId, slice] of nonLocalHostSessionEntries(slices)) { writes.push( enqueueHostPartitionWrite(hostId, state, () => api.set(slice, hostId), { state, replaceable: true, - getLiveState: state.getLiveTestimonyState + getLiveState: state.getLiveTestimonyState, + carriesParkedShadowRows: hostsWithAttachedShadow.has(hostId) }) ) } @@ -317,7 +327,7 @@ export function buildWorkspaceSessionHostSnapshots( state: HostPersistenceState ): WorkspaceSessionHostSnapshot[] { // Why 'replace': quit snapshots are applied as full partition sets. - const slices = splitWorkspaceSessionForWrite(payload, state, 'replace') + const { slices } = splitWorkspaceSessionForWrite(payload, state, 'replace') return [ { state: slices[LOCAL_EXECUTION_HOST_ID] ?? payload }, ...nonLocalHostSessionEntries(slices).map(([hostId, hostState]) => ({ diff --git a/src/renderer/src/lib/workspace-session-host-write-queue.test.ts b/src/renderer/src/lib/workspace-session-host-write-queue.test.ts index 951f452f10b1..31350775586e 100644 --- a/src/renderer/src/lib/workspace-session-host-write-queue.test.ts +++ b/src/renderer/src/lib/workspace-session-host-write-queue.test.ts @@ -315,6 +315,51 @@ describe('workspace-session-host-write-queue', () => { expect(writeFn).not.toHaveBeenCalled() }) + it('does not discard a pre-testimony write when carriesParkedShadowRows is false even if testimony arrives before dequeuing', async () => { + resetHostPartitionWriteStateForTest() + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: test fixture mock + const hostId = 'ssh:target-1' as ExecutionHostId + + const initialTestimonyState: RemoteWorkspaceTestimonyState = Object.freeze({ + remoteWorkspaceHydratedTargetIds: new Set(), + remoteWorkspaceSyncStatusByTargetId: {} + }) + + let liveState: RemoteWorkspaceTestimonyState = initialTestimonyState + + let resolveActive: (() => void) | undefined + const activeBlocker = new Promise((resolve) => { + resolveActive = resolve + }) + + const activeWrite = enqueueHostPartitionWrite(hostId, async () => { + await activeBlocker + return 'active-done' + }) + + const writeFn = vi.fn().mockResolvedValue('fresh-edit-res') + + const patchWrite = enqueueHostPartitionWrite(hostId, initialTestimonyState, writeFn, { + getLiveState: () => liveState, + carriesParkedShadowRows: false + }) + + // Live testimony arrives before the active write finishes + liveState = { + remoteWorkspaceHydratedTargetIds: new Set(['target-1']), + remoteWorkspaceSyncStatusByTargetId: { + 'target-1': { phase: 'idle' } + } + } + + resolveActive?.() + + const [resActive, resPatch] = await Promise.all([activeWrite, patchWrite]) + expect(resActive).toBe('active-done') + expect(resPatch).toBe('fresh-edit-res') + expect(writeFn).toHaveBeenCalledTimes(1) + }) + it('resetHostPartitionWriteStateForTest clears write chains and generation counters', async () => { resetHostPartitionWriteStateForTest() // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: test fixture mock diff --git a/src/renderer/src/lib/workspace-session-host-write-queue.ts b/src/renderer/src/lib/workspace-session-host-write-queue.ts index fc41293dd362..b7f7044be8d8 100644 --- a/src/renderer/src/lib/workspace-session-host-write-queue.ts +++ b/src/renderer/src/lib/workspace-session-host-write-queue.ts @@ -10,6 +10,7 @@ export type HostPartitionWriteOptions = { state?: RemoteWorkspaceTestimonyState getLiveState?: () => RemoteWorkspaceTestimonyState replaceable?: boolean + carriesParkedShadowRows?: boolean } let globalLiveTestimonyProvider: (() => RemoteWorkspaceTestimonyState) | undefined @@ -71,9 +72,11 @@ export function enqueueHostPartitionWrite( const state = options?.state const parsed = parseExecutionHostId(hostId) const targetId = parsed?.kind === 'ssh' ? parsed.targetId : null + const carriesParkedShadowRows = options?.carriesParkedShadowRows ?? true const preparedWithoutTestimony = - targetId !== null && state ? !hostHasAnsweredForTarget(state, targetId) : false - + targetId !== null && state && carriesParkedShadowRows + ? !hostHasAnsweredForTarget(state, targetId) + : false const currentGen = hostPartitionWriteGenerations.get(hostId) ?? 0 if (options?.generation !== undefined && options.generation < currentGen) { return Promise.resolve(null) diff --git a/tests/e2e/gap-03-offline-reconnect-tab-resurrection.spec.ts b/tests/e2e/gap-03-offline-reconnect-tab-resurrection.spec.ts index 878e8f2ea47f..d049d3793a89 100644 --- a/tests/e2e/gap-03-offline-reconnect-tab-resurrection.spec.ts +++ b/tests/e2e/gap-03-offline-reconnect-tab-resurrection.spec.ts @@ -67,11 +67,10 @@ const RUN_DOCKER_SSH = process.env.ORCA_E2E_SSH_DOCKER === '1' // the regression. The bug's own root-cause description is "every tab for a worktree closed // server-side while a client was offline" -- not some. CLOSE_COUNT therefore defaults to, and is // clamped to, TAB_COUNT. -const TAB_COUNT = Math.max(2, Number(process.env.ORCA_GAP03_TAB_COUNT ?? '2')) -const CLOSE_COUNT = Math.min( - TAB_COUNT, - Math.max(1, Number(process.env.ORCA_GAP03_CLOSE_COUNT ?? String(TAB_COUNT))) -) +const requestedTabCount = Number.parseInt(process.env.ORCA_GAP03_TAB_COUNT ?? '2', 10) +const TAB_COUNT = Number.isFinite(requestedTabCount) ? Math.max(2, requestedTabCount) : 2 +// Partial closure cannot exercise the decline path; see the comment above. +const CLOSE_COUNT = TAB_COUNT const OFFLINE_DRAFT_TAB_ID = 'gap03-offline-draft-tab' type SessionProfile = { From 60a05f6f42a2af504f6073a1c53df9dc621a9482 Mon Sep 17 00:00:00 2001 From: Lesley Murfin Date: Thu, 1 Oct 2026 00:34:37 -0600 Subject: [PATCH 18/23] fix(GAP-03): isolate testimony-invalidatable shadow rows and prune stale docker runner env - Narrow SSH_TERMINAL_TESTIMONY_FIELDS in workspace-session-host-shadow-testimony.ts to terminal fields managed by direct-SSH snapshots, preserving client-only state such as localOnlyScrollbackByTabId. - Export isShadowRowSubjectToHostTestimony as a single shared authority for testimony invalidation. - Track hostsWithTestimonyInvalidatableShadow in attachHostSessionShadow, ensuring writes carrying only preserved shadow entries (folders, editor drafts) are not discarded upon testimony arrival. - Add required typescript(consistent-type-assertions) safety comment on splitWorkspaceSessionForWrite call in patchWorkspaceSessionByHost. - Remove dead ORCA_GAP03_CLOSE_COUNT forwarding in config/scripts/run-offline-reconnect-tab-resurrection-docker.mjs. --- ...line-reconnect-tab-resurrection-docker.mjs | 9 +-- .../workspace-session-host-contention.test.ts | 55 +++++++++++++++++++ .../lib/workspace-session-host-contention.ts | 18 ++++-- .../lib/workspace-session-host-persistence.ts | 30 ++++++---- ...pace-session-host-shadow-testimony.test.ts | 35 ++++++++++++ ...workspace-session-host-shadow-testimony.ts | 44 ++++++++------- 6 files changed, 150 insertions(+), 41 deletions(-) diff --git a/config/scripts/run-offline-reconnect-tab-resurrection-docker.mjs b/config/scripts/run-offline-reconnect-tab-resurrection-docker.mjs index 32c708a57425..4e2045ebef20 100755 --- a/config/scripts/run-offline-reconnect-tab-resurrection-docker.mjs +++ b/config/scripts/run-offline-reconnect-tab-resurrection-docker.mjs @@ -9,9 +9,9 @@ import { spawnSync } from 'node:child_process' // Modeled directly on run-ssh-docker-e2e.mjs: same pnpm/Playwright invocation shape, same // ORCA_E2E_SSH_DOCKER env-gate the target spec reads to skip itself outside a Docker-capable host. // -// ORCA_GAP03_TAB_COUNT and ORCA_GAP03_CLOSE_COUNT tune the repro's N/M without editing the spec. -// Left unset here so the spec's own defaults (TAB_COUNT=2, CLOSE_COUNT=TAB_COUNT, i.e. every -// opened tab closes) apply — GAP-03's fix is an all-or-nothing repair (see the spec's own doc +// ORCA_GAP03_TAB_COUNT tunes the repro's N without editing the spec. +// Left unset here so the spec's own default (TAB_COUNT=2, CLOSE_COUNT=TAB_COUNT, i.e. every +// opened tab closes) applies — GAP-03's fix is an all-or-nothing repair (see the spec's own doc // comment on CLOSE_COUNT), so a run that leaves any survivor tab in the worktree never exercises // the regression at all, pre- or post-fix. const rawExtraArgs = process.argv.slice(2) @@ -22,9 +22,6 @@ const env = { ORCA_E2E_SSH_DOCKER: '1', ...(process.env.ORCA_GAP03_TAB_COUNT ? { ORCA_GAP03_TAB_COUNT: process.env.ORCA_GAP03_TAB_COUNT } - : {}), - ...(process.env.ORCA_GAP03_CLOSE_COUNT - ? { ORCA_GAP03_CLOSE_COUNT: process.env.ORCA_GAP03_CLOSE_COUNT } : {}) } diff --git a/src/renderer/src/lib/workspace-session-host-contention.test.ts b/src/renderer/src/lib/workspace-session-host-contention.test.ts index 8cfd9b6b1f16..7892e9468cb6 100644 --- a/src/renderer/src/lib/workspace-session-host-contention.test.ts +++ b/src/renderer/src/lib/workspace-session-host-contention.test.ts @@ -11,6 +11,7 @@ import type { WorkspaceSessionState } from '../../../shared/workspace-session-st import type { ExecutionHostId } from '../../../shared/execution-host' import { folderWorkspaceKey } from '../../../shared/workspace-scope' import { + attachHostSessionShadow, indexWorktreeHostClaims, mergeWorkspaceSessionsWithHostShadow, pickPrimaryHostForClaims @@ -430,3 +431,57 @@ describe('read-time primary is the one the write path honours', () => { expect(read.contestedPrimaryHostBySessionKey[SHARED_ID]).toBe('local') }) }) + +describe('attachHostSessionShadow', () => { + it('identifies hosts with testimony-invalidatable shadows separately from hosts with only non-terminal shadows', () => { + const worktreeKey = 'worktree:repo-1::/work' + const claims = indexWorktreeHostClaims( + { 'repo-1': [{ id: worktreeKey, repoId: 'repo-1' }] }, + new Map([['repo-1', SSH_HOST]]) + ) + const terminalShadow = { + [SSH_HOST]: { + ...getDefaultWorkspaceSession(), + tabsByWorktree: { + [worktreeKey]: [tab('tab-1', worktreeKey)] + } + } + } + const nonTerminalShadow = { + [SSH_HOST]: { + ...getDefaultWorkspaceSession(), + openFilesByWorktree: { + [worktreeKey]: [ + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: test fixture mock + { + filePath: '/work/index.ts', + relativePath: 'index.ts', + worktreeId: worktreeKey, + language: 'ts' + } as never + ] + } + } + } + + const slicesForTerminal = { [SSH_HOST]: { ...getDefaultWorkspaceSession() } } + const resTerminal = attachHostSessionShadow( + slicesForTerminal, + terminalShadow, + claims, + 'replace' + ) + expect(resTerminal.hostsWithAttachedShadow.has(SSH_HOST)).toBe(true) + expect(resTerminal.hostsWithTestimonyInvalidatableShadow.has(SSH_HOST)).toBe(true) + + const slicesForNonTerminal = { [SSH_HOST]: { ...getDefaultWorkspaceSession() } } + const resNonTerminal = attachHostSessionShadow( + slicesForNonTerminal, + nonTerminalShadow, + claims, + 'replace' + ) + expect(resNonTerminal.hostsWithAttachedShadow.has(SSH_HOST)).toBe(true) + expect(resNonTerminal.hostsWithTestimonyInvalidatableShadow.has(SSH_HOST)).toBe(false) + }) +}) diff --git a/src/renderer/src/lib/workspace-session-host-contention.ts b/src/renderer/src/lib/workspace-session-host-contention.ts index 5948ab4beb1c..b99e4107b326 100644 --- a/src/renderer/src/lib/workspace-session-host-contention.ts +++ b/src/renderer/src/lib/workspace-session-host-contention.ts @@ -19,6 +19,7 @@ import { mergeWorkspaceSessionsFromHosts, type HostSessionSlices } from './workspace-session-host-split' +import { isShadowRowSubjectToHostTestimony } from './workspace-session-host-shadow-testimony' /** * Which execution hosts publish each workspace id, and what persistence does when two of them @@ -274,18 +275,24 @@ function hostStillClaimsKey( /** Whether the slices will be applied as a merge-by-field patch or a full partition replace. */ export type HostSessionWriteMode = 'patch' | 'replace' +export type AttachedHostSessionShadowResult = { + hostsWithAttachedShadow: Set + hostsWithTestimonyInvalidatableShadow: Set +} + /** Write parked entries back into their own host's slice so a write for the primary host cannot * erase a co-claimant's persisted session. Mutates the slices produced by the split. - * Returns the set of host IDs that actually received one or more parked shadow entries. */ + * Returns the set of host IDs that received parked shadow entries and those carrying testimony-invalidatable entries. */ export function attachHostSessionShadow( slices: HostSessionSlices, shadow: HostSessionSlices | undefined, claims: WorktreeHostClaims, mode: HostSessionWriteMode -): Set { +): AttachedHostSessionShadowResult { const hostsWithAttachedShadow = new Set() + const hostsWithTestimonyInvalidatableShadow = new Set() if (!shadow) { - return hostsWithAttachedShadow + return { hostsWithAttachedShadow, hostsWithTestimonyInvalidatableShadow } } for (const [hostId, shadowSlice] of Object.entries(shadow) as [ ExecutionHostId, @@ -341,8 +348,11 @@ export function attachHostSessionShadow( } target[key] = entry hostsWithAttachedShadow.add(hostId) + if (isShadowRowSubjectToHostTestimony(field, worktreeId ?? key)) { + hostsWithTestimonyInvalidatableShadow.add(hostId) + } } } } - return hostsWithAttachedShadow + return { hostsWithAttachedShadow, hostsWithTestimonyInvalidatableShadow } } diff --git a/src/renderer/src/lib/workspace-session-host-persistence.ts b/src/renderer/src/lib/workspace-session-host-persistence.ts index 7b1e668052b6..f38750d7ccbe 100644 --- a/src/renderer/src/lib/workspace-session-host-persistence.ts +++ b/src/renderer/src/lib/workspace-session-host-persistence.ts @@ -227,7 +227,11 @@ function splitWorkspaceSessionForWrite( payload: WorkspaceSessionState, state: HostPersistenceState, mode: HostSessionWriteMode -): { slices: HostSessionSlices; hostsWithAttachedShadow: Set } { +): { + slices: HostSessionSlices + hostsWithAttachedShadow: Set + hostsWithTestimonyInvalidatableShadow: Set +} { const routing = buildHostSessionRouting(state) // Why the live catalogs: a debounced patch carries only the fields that changed, so a park // capture's layouts-only patch names no tab rows. Routed by the payload alone, every tab-keyed @@ -237,13 +241,14 @@ function splitWorkspaceSessionForWrite( const slices = splitWorkspaceSessionByHost(payload, routing.hostIdByWorktreeId, { worktreeIdByTabId }) - const hostsWithAttachedShadow = attachHostSessionShadow( - slices, - shadowRowsTheHostHasNotAnswered(state.contestedHostWorkspaceSessions, state), - routing.claims, - mode - ) - return { slices, hostsWithAttachedShadow } + const { hostsWithAttachedShadow, hostsWithTestimonyInvalidatableShadow } = + attachHostSessionShadow( + slices, + shadowRowsTheHostHasNotAnswered(state.contestedHostWorkspaceSessions, state), + routing.claims, + mode + ) + return { slices, hostsWithAttachedShadow, hostsWithTestimonyInvalidatableShadow } } /** Patch path of the debounced session writer: split the partial patch by owner host and patch @@ -254,7 +259,8 @@ export function patchWorkspaceSessionByHost( patch: WorkspaceSessionPatch, state: HostPersistenceState ): { localWrite: Promise; written: Promise } { - const { slices, hostsWithAttachedShadow } = splitWorkspaceSessionForWrite( + const { slices, hostsWithTestimonyInvalidatableShadow } = splitWorkspaceSessionForWrite( + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: patch is a partial WorkspaceSessionState routed by present keys patch as WorkspaceSessionState, state, 'patch' @@ -272,7 +278,7 @@ export function patchWorkspaceSessionByHost( state, replaceable: false, getLiveState: state.getLiveTestimonyState, - carriesParkedShadowRows: hostsWithAttachedShadow.has(hostId) + carriesParkedShadowRows: hostsWithTestimonyInvalidatableShadow.has(hostId) } ).catch((err: unknown) => { console.warn(`[session] host partition patch failed for ${hostId}:`, err) @@ -301,7 +307,7 @@ export async function persistWorkspaceSessionByHost( ): Promise { // Why 'replace': api.set swaps the whole partition, so parked rows must ride along even for // fields nothing else routed to this host. - const { slices, hostsWithAttachedShadow } = splitWorkspaceSessionForWrite( + const { slices, hostsWithTestimonyInvalidatableShadow } = splitWorkspaceSessionForWrite( payload, state, 'replace' @@ -313,7 +319,7 @@ export async function persistWorkspaceSessionByHost( state, replaceable: true, getLiveState: state.getLiveTestimonyState, - carriesParkedShadowRows: hostsWithAttachedShadow.has(hostId) + carriesParkedShadowRows: hostsWithTestimonyInvalidatableShadow.has(hostId) }) ) } diff --git a/src/renderer/src/lib/workspace-session-host-shadow-testimony.test.ts b/src/renderer/src/lib/workspace-session-host-shadow-testimony.test.ts index fe430ad9b5c8..218285307438 100644 --- a/src/renderer/src/lib/workspace-session-host-shadow-testimony.test.ts +++ b/src/renderer/src/lib/workspace-session-host-shadow-testimony.test.ts @@ -185,4 +185,39 @@ describe('shadowRowsTheHostHasNotAnswered', () => { [worktreeKey]: openFiles }) }) + + it('keeps local-only fields such as localOnlyScrollbackByTabId even after the host answers, because direct-SSH snapshots do not manage local scrollback', () => { + const answeredState: RemoteWorkspaceTestimonyState = { + remoteWorkspaceHydratedTargetIds: new Set([TARGET_ID]), + remoteWorkspaceSyncStatusByTargetId: { + [TARGET_ID]: { phase: 'synced', direction: 'pull' } + } + } + + const worktreeKey = 'worktree:repo-1::worktree-1' + const worktreeTabs = [tab('tab-worktree', worktreeKey)] + const scrollback = { + 'tab-worktree': { + lines: ['output line 1', 'output line 2'] + } + } + + const shadow: HostSessionSlices = { + [SSH_HOST_ID]: { + ...sessionWithTabs({ [worktreeKey]: worktreeTabs }), + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: test fixture mock for localOnlyScrollbackByTabId record + localOnlyScrollbackByTabId: scrollback as never + } + } + + const result = shadowRowsTheHostHasNotAnswered(shadow, answeredState) + + expect(result).toBeDefined() + const hostSlice = result?.[SSH_HOST_ID] + expect(hostSlice).toBeDefined() + // Terminal tabs for worktreeKey are withheld (superseded by testimony): + expect(hostSlice?.tabsByWorktree).toBeUndefined() + // Local-only scrollback survives: + expect(hostSlice?.localOnlyScrollbackByTabId).toEqual(scrollback) + }) }) diff --git a/src/renderer/src/lib/workspace-session-host-shadow-testimony.ts b/src/renderer/src/lib/workspace-session-host-shadow-testimony.ts index 500e33517c3b..d86b4a988fc0 100644 --- a/src/renderer/src/lib/workspace-session-host-shadow-testimony.ts +++ b/src/renderer/src/lib/workspace-session-host-shadow-testimony.ts @@ -20,20 +20,30 @@ import { * Non-terminal fields (such as `openFilesByWorktree` editor drafts) are never reported by an * SSH host snapshot, so host testimony does not supersede them and they are never withheld. */ -const SSH_TERMINAL_TESTIMONY_FIELDS: ReadonlySet = new Set([ - 'tabsByWorktree', - 'terminalLayoutsByTabId', - 'remoteSessionIdsByTabId', - 'localOnlyScrollbackByTabId', - 'terminalPtyIncarnationsByPaneKey', - 'unifiedTabs', - 'tabGroups', - 'tabGroupLayouts', - 'activeGroupIdByWorktree', - 'defaultTerminalTabsAppliedByWorktreeId', - 'activeTabIdByWorktree', - 'activeTabTypeByWorktree' -]) +export const SSH_TERMINAL_TESTIMONY_FIELDS: Partial> = { + tabsByWorktree: true, + terminalLayoutsByTabId: true, + remoteSessionIdsByTabId: true, + terminalPtyIncarnationsByPaneKey: true, + activeTabIdByWorktree: true, + defaultTerminalTabsAppliedByWorktreeId: true +} + +/** + * Returns true if a parked shadow field/key is subject to SSH host testimony invalidation + * (i.e. managed by direct-SSH snapshots and not belonging to a folder workspace). + */ +export function isShadowRowSubjectToHostTestimony( + field: keyof WorkspaceSessionState, + owningWorkspaceKey: string | undefined +): boolean { + if (!SSH_TERMINAL_TESTIMONY_FIELDS[field]) { + return false + } + const isFolder = + owningWorkspaceKey !== undefined && parseWorkspaceKey(owningWorkspaceKey)?.type === 'folder' + return !isFolder +} /** * The shadow minus every parked row whose host has already answered for it. Returns the input by * reference when nothing is withheld. @@ -87,11 +97,7 @@ export function shadowRowsTheHostHasNotAnswered( : ownership === 'tabKeyed' ? worktreeIdByTabIdInShadow.get(key) : worktreeIdForPaneKey(worktreeIdByTabIdInShadow, key) - const isFolder = - owningWorkspaceKey !== undefined && - parseWorkspaceKey(owningWorkspaceKey)?.type === 'folder' - const isCoveredByTerminalTestimony = SSH_TERMINAL_TESTIMONY_FIELDS.has(field) - if (isFolder || !isCoveredByTerminalTestimony) { + if (!isShadowRowSubjectToHostTestimony(field, owningWorkspaceKey)) { survivingRecord[key] = value } else { fieldWithheld = true From bbd03a5fb1a3da0f78c57bb2c7d573307e43dcdb Mon Sep 17 00:00:00 2001 From: Lesley Murfin Date: Fri, 2 Oct 2026 00:10:20 -0600 Subject: [PATCH 19/23] fix(GAP-03): break circular import between contention and shadow-testimony modules - Move PARKABLE_HOST_SESSION_FIELDS to src/shared/workspace-session-host-field-ownership.ts alongside other ownership field sets. - Re-export PARKABLE_HOST_SESSION_FIELDS from workspace-session-host-contention.ts for backwards compatibility. - Import PARKABLE_HOST_SESSION_FIELDS directly from shared in workspace-session-host-shadow-testimony.ts and merge duplicate imports. - Satisfies oxlint audit:code-quality:native with deny-warnings. --- .../src/lib/workspace-session-host-contention.ts | 12 +++++------- .../lib/workspace-session-host-shadow-testimony.ts | 9 +++++---- src/shared/workspace-session-host-field-ownership.ts | 7 +++++++ 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/renderer/src/lib/workspace-session-host-contention.ts b/src/renderer/src/lib/workspace-session-host-contention.ts index b99e4107b326..ac3e6b522304 100644 --- a/src/renderer/src/lib/workspace-session-host-contention.ts +++ b/src/renderer/src/lib/workspace-session-host-contention.ts @@ -6,7 +6,10 @@ import { type ExecutionHostId } from '../../../shared/execution-host' import { normalizeWorkspaceSessionKeyToWorkspaceId } from '../../../shared/workspace-scope' -import { WORKSPACE_SESSION_FIELD_OWNERSHIP } from '../../../shared/workspace-session-host-field-ownership' +import { + PARKABLE_HOST_SESSION_FIELDS, + WORKSPACE_SESSION_FIELD_OWNERSHIP +} from '../../../shared/workspace-session-host-field-ownership' import { workspaceSessionPartitionHostId } from '../../../shared/workspace-session-partition-owner' import { buildWorktreeIdByTabId, @@ -55,12 +58,7 @@ export const WORKTREE_KEYED_FIELDS = ( * `terminalPtyIncarnationsByPaneKey`). `attachHostSessionShadow` restores over this wider set so a * GAP-03 stranded-partition shadow (`partitionRowsTheWriteWontReturn`) can carry a declined tab's * dependent rows back with it, not just its `tabsByWorktree` entry. */ -export const PARKABLE_HOST_SESSION_FIELDS = ( - Object.keys(WORKSPACE_SESSION_FIELD_OWNERSHIP) as (keyof WorkspaceSessionState)[] -).filter((field) => { - const ownership = WORKSPACE_SESSION_FIELD_OWNERSHIP[field] - return ownership === 'worktreeKeyed' || ownership === 'tabKeyed' || ownership === 'paneKeyed' -}) +export { PARKABLE_HOST_SESSION_FIELDS } /** Bare worktree id behind a session key. Lives in shared because the partition adoption read needs * the same normalization, and two implementations of it would drift. */ diff --git a/src/renderer/src/lib/workspace-session-host-shadow-testimony.ts b/src/renderer/src/lib/workspace-session-host-shadow-testimony.ts index d86b4a988fc0..4053fc6019df 100644 --- a/src/renderer/src/lib/workspace-session-host-shadow-testimony.ts +++ b/src/renderer/src/lib/workspace-session-host-shadow-testimony.ts @@ -1,14 +1,15 @@ -import type { ExecutionHostId } from '../../../shared/execution-host' -import { parseExecutionHostId } from '../../../shared/execution-host' +import { parseExecutionHostId, type ExecutionHostId } from '../../../shared/execution-host' import { parseWorkspaceKey } from '../../../shared/workspace-scope' -import { WORKSPACE_SESSION_FIELD_OWNERSHIP } from '../../../shared/workspace-session-host-field-ownership' +import { + PARKABLE_HOST_SESSION_FIELDS, + WORKSPACE_SESSION_FIELD_OWNERSHIP +} from '../../../shared/workspace-session-host-field-ownership' import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' import { buildWorktreeIdByTabId, isWorkspaceSessionRecord, worktreeIdForPaneKey } from '../../../shared/workspace-session-host-records' -import { PARKABLE_HOST_SESSION_FIELDS } from './workspace-session-host-contention' import type { HostSessionSlices } from './workspace-session-host-split' import { hostHasAnsweredForTarget, diff --git a/src/shared/workspace-session-host-field-ownership.ts b/src/shared/workspace-session-host-field-ownership.ts index ce8ce0f75b34..2f6cd160bcae 100644 --- a/src/shared/workspace-session-host-field-ownership.ts +++ b/src/shared/workspace-session-host-field-ownership.ts @@ -81,6 +81,13 @@ export const GLOBAL_WORKSPACE_SESSION_FIELDS = ( Object.keys(WORKSPACE_SESSION_FIELD_OWNERSHIP) as (keyof WorkspaceSessionState)[] ).filter((field) => WORKSPACE_SESSION_FIELD_OWNERSHIP[field] === 'global') +export const PARKABLE_HOST_SESSION_FIELDS = ( + Object.keys(WORKSPACE_SESSION_FIELD_OWNERSHIP) as (keyof WorkspaceSessionState)[] +).filter((field) => { + const ownership = WORKSPACE_SESSION_FIELD_OWNERSHIP[field] + return ownership === 'worktreeKeyed' || ownership === 'tabKeyed' || ownership === 'paneKeyed' +}) + /** * Global session fields that belong to the 'local' slice alone: `splitWorkspaceSessionByHost` * writes them only there and `mergeWorkspaceSessionsFromHosts` reads them only from there. A copy From 02332bb7bd985597e2f0a5ab46b92a4028f7d829 Mon Sep 17 00:00:00 2001 From: Lesley Murfin Date: Wed, 14 Oct 2026 03:00:21 -0600 Subject: [PATCH 20/23] fix(GAP-03): stop publish fallback from re-exporting declined offline tabs Primary setForConnectedTargets omitted session, so main persistedSessionForTarget re-adopted stale ssh-partition tabs without reconciledWorktreeIds. Pass the renderer session on upload and apply the same confirmed+shutdown decline on the main fallback; regression covers omit-session offline vs connected restart. --- .../remote-workspace-target-session-export.ts | 43 +++++++--- .../ssh-host-partition-session-export.test.ts | 78 ++++++++++++++++++- .../app-shell/use-app-session-persistence.ts | 1 + .../lib/workspace-session-host-hydration.ts | 25 +----- .../workspace-session-shutdown-connection.ts | 31 ++++++++ 5 files changed, 144 insertions(+), 34 deletions(-) create mode 100644 src/shared/workspace-session-shutdown-connection.ts diff --git a/src/main/ipc/remote-workspace-target-session-export.ts b/src/main/ipc/remote-workspace-target-session-export.ts index 9adef9312e93..5372ae5b0823 100644 --- a/src/main/ipc/remote-workspace-target-session-export.ts +++ b/src/main/ipc/remote-workspace-target-session-export.ts @@ -3,11 +3,16 @@ import type { Repo } from '../../shared/repo-types' import { exportRemoteWorkspaceSession } from '../../shared/remote-workspace-session-projection' import type { RemoteWorkspaceSession } from '../../shared/remote-workspace-types' import type { WorkspaceSessionState } from '../../shared/workspace-session-state-types' -import { toSshExecutionHostId, type ExecutionHostId } from '../../shared/execution-host' +import { + normalizeExecutionHostId, + toSshExecutionHostId, + type ExecutionHostId +} from '../../shared/execution-host' import { adoptStrandedHostPartitionSession, workspaceIdsNamedByPartition } from '../../shared/workspace-session-stranded-partition-adoption' +import { wasTargetConnectedAtLastShutdown } from '../../shared/workspace-session-shutdown-connection' import { getRepoIdFromWorktreeId } from '../../shared/worktree/id' import { resolveWorktreeExecutionHost, @@ -88,8 +93,9 @@ export function exportSessionForTarget( * deletion on the host (#12721). Resolved per target so one target's rows can never be published * under another's key when both partitions hold the same worktree id. * - * The contested verdict is computed the same way the renderer's read computes it, over the same two - * partitions. Main reaching a different one would let it publish rows the renderer never displays. + * The contested / foreign / confirmed verdicts match the renderer's boot read. Confirmed ids feed + * GAP-03 `reconciledWorktreeIds` so a publish with no renderer `session` cannot re-export stale + * ssh-partition tabs the hydration path already declined (stablyai/orca#22038). */ export function persistedSessionForTarget( store: Store, @@ -100,15 +106,27 @@ export function persistedSessionForTarget( const hostId = toSshExecutionHostId(targetId) const local = store.getWorkspaceSession() const host = store.getWorkspaceSession(hostId) + const attribution = catalogAttributionForPartition(resolveWorktreeOwner, host, hostId) + const reconciledWorktreeIds = wasTargetConnectedAtLastShutdown(local, targetId) + ? new Set() + : attribution.confirmedSessionKeys return adoptStrandedHostPartitionSession(local, host, { - ...catalogAttributionForPartition(resolveWorktreeOwner, host, hostId) + contestedSessionKeys: attribution.contestedSessionKeys, + foreignSessionKeys: attribution.foreignSessionKeys, + ownedSessionKeys: attribution.ownedSessionKeys, + reconciledWorktreeIds }).session } /** The same catalog reading the renderer's boot read applies to this partition: a repo id the * catalog registers on more than one host is contested, and one it positively resolves to a * different host is residue this partition does not own. Main reaching a different verdict would - * publish to the host rows the renderer never displays. */ + * publish to the host rows the renderer never displays. + * + * `ownedSessionKeys` — catalog resolves to this host (incl. bare connectionId fallback). + * `confirmedSessionKeys` — catalog resolves to exactly this host via the row's own + * `executionHostId` (not bare `connectionId`). Same narrow GAP-03 signal as renderer + * `sshPartitionCatalogAttribution`. */ function catalogAttributionForPartition( resolveWorktreeOwner: WorktreeOwnerResolver, host: WorkspaceSessionState, @@ -117,10 +135,12 @@ function catalogAttributionForPartition( contestedSessionKeys: Set foreignSessionKeys: Set ownedSessionKeys: Set + confirmedSessionKeys: Set } { const contestedSessionKeys = new Set() const foreignSessionKeys = new Set() const ownedSessionKeys = new Set() + const confirmedSessionKeys = new Set() for (const workspaceId of workspaceIdsNamedByPartition(host)) { // A folder key names no repo, and `getRepoIdFromWorktreeId` hands back the whole key rather // than nothing, so the catalog would be asked about `folder:` and answer `unknown`. @@ -133,11 +153,16 @@ function catalogAttributionForPartition( if (resolution.reason === 'ambiguous') { contestedSessionKeys.add(workspaceId) } - } else if (resolution.hostId !== hostId) { + continue + } + if (resolution.hostId !== hostId) { foreignSessionKeys.add(workspaceId) - } else { - ownedSessionKeys.add(workspaceId) + continue + } + ownedSessionKeys.add(workspaceId) + if (normalizeExecutionHostId(resolution.owner?.executionHostId ?? null) === hostId) { + confirmedSessionKeys.add(workspaceId) } } - return { contestedSessionKeys, foreignSessionKeys, ownedSessionKeys } + return { contestedSessionKeys, foreignSessionKeys, ownedSessionKeys, confirmedSessionKeys } } diff --git a/src/main/ipc/ssh-host-partition-session-export.test.ts b/src/main/ipc/ssh-host-partition-session-export.test.ts index 1ec5167f866b..9f333b1dc04b 100644 --- a/src/main/ipc/ssh-host-partition-session-export.test.ts +++ b/src/main/ipc/ssh-host-partition-session-export.test.ts @@ -15,6 +15,7 @@ import { mkdtempSync, realpathSync, rmSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { setAppEnvironment, type AppEnvironment } from '../../shared/app-environment' import type { Repo } from '../../shared/repo-types' import type { RemoteWorkspaceSession, @@ -88,9 +89,22 @@ function sshTarget(id: string, host: string): SshTarget { const target = sshTarget(TARGET_ID, 'one.example.com') const otherTarget = sshTarget(OTHER_TARGET_ID, 'two.example.com') -function remoteRepo(id: string, path: string, connectionId: string): Repo { +function remoteRepo( + id: string, + path: string, + connectionId: string, + executionHostId?: string +): Repo { // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the literal names every Repo field this export suite reads. - return { id, path, displayName: id, badgeColor: 'blue', addedAt: 1, connectionId } as Repo + return { + id, + path, + displayName: id, + badgeColor: 'blue', + addedAt: 1, + connectionId, + ...(executionHostId !== undefined ? { executionHostId } : {}) + } as Repo } function runtimeAuthoredTab(): TerminalTab { @@ -118,6 +132,15 @@ afterEach(async () => { }) beforeEach(() => { + setAppEnvironment({ + getPath: () => tmpdir(), + getAppPath: () => tmpdir(), + getVersion: () => '0.0.0-test', + isPackaged: () => false, + onWillQuit: () => {}, + exit: () => {}, + getAppMetrics: () => [] + } as AppEnvironment) _resetRemoteWorkspaceCachesForTests() ipcHandlers.clear() hostSnapshot = { @@ -236,3 +259,54 @@ describe('remoteWorkspace:setForConnectedTargets session fallback', () => { expect(hostSnapshot.session.tabsByWorktreePath[WORKTREE_PATH]).not.toEqual([]) }) }) + +describe('remoteWorkspace:setForConnectedTargets GAP-03 offline fallback', () => { + it('does not re-publish stale ssh-partition tabs after a genuine offline gap when session is omitted', async () => { + // Renderer hydration declines these; the shipping debounced writer used to omit `session` and + // let main re-adopt the parked ssh mirror into the replace-session payload. + const dir = realpathSync(mkdtempSync(join(tmpdir(), 'orca-ssh-partition-gap03-'))) + const store = new Store({ dataFile: join(dir, 'orca-data.json') }) + stores.push(store) + store.addRepo(remoteRepo(REPO_ID, '/remote/checkout', TARGET_ID, SSH_HOST_ID)) + store.setWorkspaceSession({ + ...getDefaultWorkspaceSession(), + // Disconnected at last quit — GAP-03 precondition. No base tabsByWorktree row at all. + activeConnectionIdsAtShutdown: [] + }) + store.setWorkspaceSession( + { + ...getDefaultWorkspaceSession(), + tabsByWorktree: { [WORKTREE_ID]: [runtimeAuthoredTab()] } + }, + SSH_HOST_ID + ) + + await publishToConnectedTarget(store) + + expect(hostSnapshot.session.tabsByWorktreePath[WORKTREE_PATH] ?? []).toEqual([]) + }) + + it('still publishes ssh-partition tabs when the client quit while connected (ordinary restart)', async () => { + const dir = realpathSync(mkdtempSync(join(tmpdir(), 'orca-ssh-partition-gap03-connected-'))) + const store = new Store({ dataFile: join(dir, 'orca-data.json') }) + stores.push(store) + store.addRepo(remoteRepo(REPO_ID, '/remote/checkout', TARGET_ID, SSH_HOST_ID)) + store.setWorkspaceSession({ + ...getDefaultWorkspaceSession(), + activeConnectionIdsAtShutdown: [TARGET_ID] + }) + store.setWorkspaceSession( + { + ...getDefaultWorkspaceSession(), + tabsByWorktree: { [WORKTREE_ID]: [runtimeAuthoredTab()] } + }, + SSH_HOST_ID + ) + + await publishToConnectedTarget(store) + + expect(hostSnapshot.session.tabsByWorktreePath[WORKTREE_PATH]?.map((tab) => tab.id)).toEqual([ + 'tab-runtime' + ]) + }) +}) diff --git a/src/renderer/src/app-shell/use-app-session-persistence.ts b/src/renderer/src/app-shell/use-app-session-persistence.ts index 2d148bbdca73..006cfff63f42 100644 --- a/src/renderer/src/app-shell/use-app-session-persistence.ts +++ b/src/renderer/src/app-shell/use-app-session-persistence.ts @@ -163,6 +163,7 @@ export function useAppSessionPersistence(): void { ]) ) const results = await window.api.remoteWorkspace?.setForConnectedTargets({ + session: buildWorkspaceSessionPayload(currentState), hydratedTargetIds, expectedRevisionsByTargetId, expectedHostObservationTokensByTargetId diff --git a/src/renderer/src/lib/workspace-session-host-hydration.ts b/src/renderer/src/lib/workspace-session-host-hydration.ts index 35193b836ae2..dc0ecf090ecc 100644 --- a/src/renderer/src/lib/workspace-session-host-hydration.ts +++ b/src/renderer/src/lib/workspace-session-host-hydration.ts @@ -1,6 +1,6 @@ import type { Repo } from '../../../shared/repo-types' import type { WorkspaceSessionState } from '../../../shared/workspace-session-state-types' -import { parseAppSshPtyId } from '../../../shared/ssh-pty-id' +import { wasTargetConnectedAtLastShutdown } from '../../../shared/workspace-session-shutdown-connection' import { getRepoExecutionHostId, getSshTargetIdForExecutionHost, @@ -158,28 +158,7 @@ export async function fetchWorkspaceSessionFromHosts( .session } -export function wasTargetConnectedAtLastShutdown( - session: WorkspaceSessionState, - targetId: string | null -): boolean { - if (!targetId) { - return false - } - if (session.activeConnectionIdsAtShutdown !== undefined) { - return session.activeConnectionIdsAtShutdown.includes(targetId) - } - // When activeConnectionIdsAtShutdown is undefined (older persisted sessions, interrupted - // shutdowns), infer connection status from remoteSessionIdsByTabId: if the base session still - // holds a remote PTY session for this target, the client was connected to it, so do not decline. - // Residual remoteSessionIdsByTabId on the SSH partition mirror itself are NOT inspected: - // they are the stale cache GAP-03 exists to decline. - for (const sessionId of Object.values(session.remoteSessionIdsByTabId ?? {})) { - if (parseAppSshPtyId(sessionId)?.connectionId === targetId) { - return true - } - } - return false -} +export { wasTargetConnectedAtLastShutdown } from '../../../shared/workspace-session-shutdown-connection' export async function fetchWorkspaceSessionWithRuntimeHostOwners( api: SessionReadApi, diff --git a/src/shared/workspace-session-shutdown-connection.ts b/src/shared/workspace-session-shutdown-connection.ts new file mode 100644 index 000000000000..d56cf457d038 --- /dev/null +++ b/src/shared/workspace-session-shutdown-connection.ts @@ -0,0 +1,31 @@ +import { parseAppSshPtyId } from './ssh-pty-id' +import type { WorkspaceSessionState } from './workspace-session-state-types' + +/** + * Whether this SSH target was connected when the client last quit. + * + * GAP-03 decline (`reconciledWorktreeIds`) only applies after a genuine offline gap. An absent base + * `tabsByWorktree` row for an SSH-routed worktree is normal on every boot (those rows never land in + * the local partition), so it is not evidence of server-side closes by itself. + * + * Prefers `activeConnectionIdsAtShutdown`. When that field is missing (older sessions / interrupted + * shutdowns), infers from base `remoteSessionIdsByTabId` only — never from the SSH partition mirror, + * which is the stale cache GAP-03 exists to decline. + */ +export function wasTargetConnectedAtLastShutdown( + session: WorkspaceSessionState, + targetId: string | null +): boolean { + if (!targetId) { + return false + } + if (session.activeConnectionIdsAtShutdown !== undefined) { + return session.activeConnectionIdsAtShutdown.includes(targetId) + } + for (const sessionId of Object.values(session.remoteSessionIdsByTabId ?? {})) { + if (parseAppSshPtyId(sessionId)?.connectionId === targetId) { + return true + } + } + return false +} From 5a89d7580e4fad566738e9ede8bd7260fef0737d Mon Sep 17 00:00:00 2001 From: Lesley Murfin Date: Wed, 14 Oct 2026 03:16:09 -0600 Subject: [PATCH 21/23] fix(GAP-03): default carriesParkedShadowRows to false Pre-testimony write discard is opt-in: only callers that know the write carries parked shadow rows pass true (persistence uses hostsWithTestimonyInvalidatableShadow). Default false keeps ordinary patches from being dropped when live testimony lands mid-flight. Also: GAP-03 decline reads originalBase for hasBaseKey so main's ownedSessionKeys superseding of empty #12723 placeholders cannot false-trigger offline decline; export GAP-03 tests use createSqliteTestStore. --- .../ssh-host-partition-session-export.test.ts | 56 +++++++++---------- ...workspace-session-host-write-queue.test.ts | 46 ++++++++++++++- .../lib/workspace-session-host-write-queue.ts | 2 +- ...ace-session-stranded-partition-adoption.ts | 13 +++-- 4 files changed, 80 insertions(+), 37 deletions(-) diff --git a/src/main/ipc/ssh-host-partition-session-export.test.ts b/src/main/ipc/ssh-host-partition-session-export.test.ts index 9f333b1dc04b..abd82b3a033b 100644 --- a/src/main/ipc/ssh-host-partition-session-export.test.ts +++ b/src/main/ipc/ssh-host-partition-session-export.test.ts @@ -260,26 +260,34 @@ describe('remoteWorkspace:setForConnectedTargets session fallback', () => { }) }) +function createGap03OfflineStore( + activeConnectionIdsAtShutdown: string[] +): InstanceType { + const dir = realpathSync(mkdtempSync(join(tmpdir(), 'orca-ssh-partition-gap03-'))) + directories.push(dir) + const store = createSqliteTestStore(Store, { dataFile: join(dir, 'orca-data.json') }) + // executionHostId required so catalogAttribution marks WORKTREE_ID confirmed (GAP-03 signal). + store.addRepo(remoteRepo(REPO_ID, '/remote/checkout', TARGET_ID, SSH_HOST_ID)) + store.setWorkspaceSession({ + ...getDefaultWorkspaceSession(), + // No base tabsByWorktree row at all — the offline / ordinary-restart shape for SSH-routed tabs. + activeConnectionIdsAtShutdown + }) + store.setWorkspaceSession( + { + ...getDefaultWorkspaceSession(), + tabsByWorktree: { [WORKTREE_ID]: [runtimeAuthoredTab()] } + }, + SSH_HOST_ID + ) + return store +} + describe('remoteWorkspace:setForConnectedTargets GAP-03 offline fallback', () => { it('does not re-publish stale ssh-partition tabs after a genuine offline gap when session is omitted', async () => { // Renderer hydration declines these; the shipping debounced writer used to omit `session` and // let main re-adopt the parked ssh mirror into the replace-session payload. - const dir = realpathSync(mkdtempSync(join(tmpdir(), 'orca-ssh-partition-gap03-'))) - const store = new Store({ dataFile: join(dir, 'orca-data.json') }) - stores.push(store) - store.addRepo(remoteRepo(REPO_ID, '/remote/checkout', TARGET_ID, SSH_HOST_ID)) - store.setWorkspaceSession({ - ...getDefaultWorkspaceSession(), - // Disconnected at last quit — GAP-03 precondition. No base tabsByWorktree row at all. - activeConnectionIdsAtShutdown: [] - }) - store.setWorkspaceSession( - { - ...getDefaultWorkspaceSession(), - tabsByWorktree: { [WORKTREE_ID]: [runtimeAuthoredTab()] } - }, - SSH_HOST_ID - ) + const store = createGap03OfflineStore([]) await publishToConnectedTarget(store) @@ -287,21 +295,7 @@ describe('remoteWorkspace:setForConnectedTargets GAP-03 offline fallback', () => }) it('still publishes ssh-partition tabs when the client quit while connected (ordinary restart)', async () => { - const dir = realpathSync(mkdtempSync(join(tmpdir(), 'orca-ssh-partition-gap03-connected-'))) - const store = new Store({ dataFile: join(dir, 'orca-data.json') }) - stores.push(store) - store.addRepo(remoteRepo(REPO_ID, '/remote/checkout', TARGET_ID, SSH_HOST_ID)) - store.setWorkspaceSession({ - ...getDefaultWorkspaceSession(), - activeConnectionIdsAtShutdown: [TARGET_ID] - }) - store.setWorkspaceSession( - { - ...getDefaultWorkspaceSession(), - tabsByWorktree: { [WORKTREE_ID]: [runtimeAuthoredTab()] } - }, - SSH_HOST_ID - ) + const store = createGap03OfflineStore([TARGET_ID]) await publishToConnectedTarget(store) diff --git a/src/renderer/src/lib/workspace-session-host-write-queue.test.ts b/src/renderer/src/lib/workspace-session-host-write-queue.test.ts index 31350775586e..81c57936265c 100644 --- a/src/renderer/src/lib/workspace-session-host-write-queue.test.ts +++ b/src/renderer/src/lib/workspace-session-host-write-queue.test.ts @@ -295,7 +295,8 @@ describe('workspace-session-host-write-queue', () => { const writeFn = vi.fn().mockResolvedValue('testimony-guarded-res') const guardedWrite = enqueueHostPartitionWrite(hostId, initialTestimonyState, writeFn, { - getLiveState: () => liveState + getLiveState: () => liveState, + carriesParkedShadowRows: true }) // Live testimony arrives before the active write finishes, updating liveState @@ -315,6 +316,49 @@ describe('workspace-session-host-write-queue', () => { expect(writeFn).not.toHaveBeenCalled() }) + it('does not discard a pre-testimony write by default when carriesParkedShadowRows is omitted', async () => { + resetHostPartitionWriteStateForTest() + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: test fixture mock + const hostId = 'ssh:target-1' as ExecutionHostId + + const initialTestimonyState: RemoteWorkspaceTestimonyState = Object.freeze({ + remoteWorkspaceHydratedTargetIds: new Set(), + remoteWorkspaceSyncStatusByTargetId: {} + }) + + let liveState: RemoteWorkspaceTestimonyState = initialTestimonyState + + let resolveActive: (() => void) | undefined + const activeBlocker = new Promise((resolve) => { + resolveActive = resolve + }) + + const activeWrite = enqueueHostPartitionWrite(hostId, async () => { + await activeBlocker + return 'active-done' + }) + + const writeFn = vi.fn().mockResolvedValue('default-opt-in-res') + + const guardedWrite = enqueueHostPartitionWrite(hostId, initialTestimonyState, writeFn, { + getLiveState: () => liveState + }) + + liveState = { + remoteWorkspaceHydratedTargetIds: new Set(['target-1']), + remoteWorkspaceSyncStatusByTargetId: { + 'target-1': { phase: 'idle' } + } + } + + resolveActive?.() + + const [resActive, resGuarded] = await Promise.all([activeWrite, guardedWrite]) + expect(resActive).toBe('active-done') + expect(resGuarded).toBe('default-opt-in-res') + expect(writeFn).toHaveBeenCalledOnce() + }) + it('does not discard a pre-testimony write when carriesParkedShadowRows is false even if testimony arrives before dequeuing', async () => { resetHostPartitionWriteStateForTest() // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: test fixture mock diff --git a/src/renderer/src/lib/workspace-session-host-write-queue.ts b/src/renderer/src/lib/workspace-session-host-write-queue.ts index b7f7044be8d8..e2f130d8848f 100644 --- a/src/renderer/src/lib/workspace-session-host-write-queue.ts +++ b/src/renderer/src/lib/workspace-session-host-write-queue.ts @@ -72,7 +72,7 @@ export function enqueueHostPartitionWrite( const state = options?.state const parsed = parseExecutionHostId(hostId) const targetId = parsed?.kind === 'ssh' ? parsed.targetId : null - const carriesParkedShadowRows = options?.carriesParkedShadowRows ?? true + const carriesParkedShadowRows = options?.carriesParkedShadowRows ?? false const preparedWithoutTestimony = targetId !== null && state && carriesParkedShadowRows ? !hostHasAnsweredForTarget(state, targetId) diff --git a/src/shared/workspace-session-stranded-partition-adoption.ts b/src/shared/workspace-session-stranded-partition-adoption.ts index a4c6a104b1e4..9eda7b989793 100644 --- a/src/shared/workspace-session-stranded-partition-adoption.ts +++ b/src/shared/workspace-session-stranded-partition-adoption.ts @@ -339,14 +339,19 @@ export function adoptStrandedHostPartitionSession( // truth (zero tabs), not a blind spot — so a host row that still names real tabs here is this // client's stale pre-disconnect cache, not evidence of anything live. Declining leaves the row // in place for `partitionRowsTheWriteWontReturn` to park rather than deleting it. + // + // Why originalBase, not the post-supersede `base`: main's ownedSessionKeys path strips base + // residue (including empty #12723 placeholders) before adoption so the host's live tabs can + // fully replace. That strip must not rewrite "base held an empty placeholder" into "base never + // named this worktree" — the latter is the offline-gap signal, the former is the split repair. const hostTabsByWorktree = host.tabsByWorktree ?? {} - const baseTabs = base.tabsByWorktree ?? {} + const originalBaseTabs = originalBase.tabsByWorktree ?? {} for (const workspaceId of options.reconciledWorktreeIds ?? []) { const canonicalKey = worktreeWorkspaceKey(workspaceId) const hasBaseKey = - Object.hasOwn(baseTabs, workspaceId) || - Object.hasOwn(baseTabs, canonicalKey) || - Object.keys(baseTabs).some( + Object.hasOwn(originalBaseTabs, workspaceId) || + Object.hasOwn(originalBaseTabs, canonicalKey) || + Object.keys(originalBaseTabs).some( (key) => normalizeWorkspaceSessionKeyToWorkspaceId(key) === workspaceId ) if (!adoptable.has(workspaceId) || hasBaseKey) { From b578e36fa920cb052da661f0f9b7d9f5c6be7373 Mon Sep 17 00:00:00 2001 From: Lesley Murfin Date: Wed, 14 Oct 2026 03:16:52 -0600 Subject: [PATCH 22/23] fix(GAP-03): withhold split-layout chrome under SSH host testimony MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Parked unifiedTabs/tabGroups/tabGroupLayouts/activeGroupIdByWorktree must drop from the write shadow once the host has answered, same as declined tabs — else post-testimony persist restores ghost split chrome with no tabs. --- ...pace-session-host-shadow-testimony.test.ts | 47 +++++++++++++++++++ ...workspace-session-host-shadow-testimony.ts | 12 +++-- 2 files changed, 55 insertions(+), 4 deletions(-) diff --git a/src/renderer/src/lib/workspace-session-host-shadow-testimony.test.ts b/src/renderer/src/lib/workspace-session-host-shadow-testimony.test.ts index 218285307438..cdf89826853b 100644 --- a/src/renderer/src/lib/workspace-session-host-shadow-testimony.test.ts +++ b/src/renderer/src/lib/workspace-session-host-shadow-testimony.test.ts @@ -220,4 +220,51 @@ describe('shadowRowsTheHostHasNotAnswered', () => { // Local-only scrollback survives: expect(hostSlice?.localOnlyScrollbackByTabId).toEqual(scrollback) }) + + it('withholds split-layout chrome for declined worktrees once the host has answered', () => { + // Parked with declined tabs; if left in the write shadow after testimony, attach restores ghost + // groups/layouts onto the SSH partition with no tabs (layout-only residue). + const answeredState: RemoteWorkspaceTestimonyState = { + remoteWorkspaceHydratedTargetIds: new Set([TARGET_ID]), + remoteWorkspaceSyncStatusByTargetId: { + [TARGET_ID]: { phase: 'synced', direction: 'pull' } + } + } + + const worktreeKey = 'worktree:repo-1::worktree-1' + const worktreeTabs = [tab('tab-worktree', worktreeKey)] + const shadow: HostSessionSlices = { + [SSH_HOST_ID]: { + ...sessionWithTabs({ [worktreeKey]: worktreeTabs }), + unifiedTabs: { [worktreeKey]: worktreeTabs }, + tabGroups: { + [worktreeKey]: [{ id: 'g1', tabIds: ['tab-worktree'], activeTabId: 'tab-worktree' }] + }, + tabGroupLayouts: { + [worktreeKey]: { root: { type: 'leaf', tabIds: ['tab-worktree'] } } + }, + activeGroupIdByWorktree: { [worktreeKey]: 'g1' }, + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: non-terminal control that must still survive. + openFilesByWorktree: { + [worktreeKey]: [ + { + filePath: '/x.ts', + relativePath: 'x.ts', + worktreeId: worktreeKey, + language: 'typescript' + } + ] as never + } + } + } + + const result = shadowRowsTheHostHasNotAnswered(shadow, answeredState) + const hostSlice = result?.[SSH_HOST_ID] + expect(hostSlice?.tabsByWorktree).toBeUndefined() + expect(hostSlice?.unifiedTabs).toBeUndefined() + expect(hostSlice?.tabGroups).toBeUndefined() + expect(hostSlice?.tabGroupLayouts).toBeUndefined() + expect(hostSlice?.activeGroupIdByWorktree).toBeUndefined() + expect(hostSlice?.openFilesByWorktree?.[worktreeKey]).toBeDefined() + }) }) diff --git a/src/renderer/src/lib/workspace-session-host-shadow-testimony.ts b/src/renderer/src/lib/workspace-session-host-shadow-testimony.ts index 4053fc6019df..9048fceb0e73 100644 --- a/src/renderer/src/lib/workspace-session-host-shadow-testimony.ts +++ b/src/renderer/src/lib/workspace-session-host-shadow-testimony.ts @@ -17,9 +17,9 @@ import { } from './remote-workspace-host-testimony' /** - * Terminal fields that direct-SSH remote workspace snapshots actually project and manage. - * Non-terminal fields (such as `openFilesByWorktree` editor drafts) are never reported by an - * SSH host snapshot, so host testimony does not supersede them and they are never withheld. + * Terminal / terminal-UI fields that direct-SSH remote workspace snapshots project and manage. + * Includes split-layout chrome parked with declined worktrees so post-answer writes do not restore + * ghost groups after tabs were withheld. Non-terminal fields (e.g. `openFilesByWorktree`) stay. */ export const SSH_TERMINAL_TESTIMONY_FIELDS: Partial> = { tabsByWorktree: true, @@ -27,7 +27,11 @@ export const SSH_TERMINAL_TESTIMONY_FIELDS: Partial Date: Wed, 14 Oct 2026 03:21:18 -0600 Subject: [PATCH 23/23] fix(GAP-03): normalize reconciled decline keys once Precompute base tabsByWorktree workspace ids and normalize reconciled ids before the offline decline loop. Drops the worktree: dual-key scan and the unused hydration re-export of wasTargetConnectedAtLastShutdown. --- .../lib/workspace-session-host-hydration.ts | 3 --- ...pace-session-stranded-partition-adoption.ts | 18 +++++++----------- 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/src/renderer/src/lib/workspace-session-host-hydration.ts b/src/renderer/src/lib/workspace-session-host-hydration.ts index dc0ecf090ecc..7d83a37a8681 100644 --- a/src/renderer/src/lib/workspace-session-host-hydration.ts +++ b/src/renderer/src/lib/workspace-session-host-hydration.ts @@ -158,8 +158,6 @@ export async function fetchWorkspaceSessionFromHosts( .session } -export { wasTargetConnectedAtLastShutdown } from '../../../shared/workspace-session-shutdown-connection' - export async function fetchWorkspaceSessionWithRuntimeHostOwners( api: SessionReadApi, repos: readonly Pick[], @@ -277,4 +275,3 @@ async function listPersistedSshPartitionHostIds(api: SessionReadApi): Promise normalizeWorkspaceSessionKeyToWorkspaceId(key) === workspaceId - ) - if (!adoptable.has(workspaceId) || hasBaseKey) { + const originalBaseTabWorkspaceIds = new Set( + Object.keys(originalBase.tabsByWorktree ?? {}).map(normalizeWorkspaceSessionKeyToWorkspaceId) + ) + for (const rawWorkspaceId of options.reconciledWorktreeIds ?? []) { + const workspaceId = normalizeWorkspaceSessionKeyToWorkspaceId(rawWorkspaceId) + if (!adoptable.has(workspaceId) || originalBaseTabWorkspaceIds.has(workspaceId)) { continue } for (const [key, hostTabs] of Object.entries(hostTabsByWorktree)) {