Repository navigation
fix(session): give an SSH workspace one owning partition so its tabs stop round-tripping as deletions - #19572
Conversation
47ac2ce to
73735c1
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change gives SSH execution hosts dedicated session partitions. Renderer hydration reads and adopts SSH partitions. Remote-workspace publishing resolves worktrees per target and reads the target partition when no session is supplied. Tests cover routing, hydration, round trips, and IPC fallback publishing. Fixed issue severity: Medium Severity of issue fixed: Medium Merge Risk: 🟡 Moderate · up to SSH session recovery now uses host-owned partitions, but edge cases around unified terminal state and conflicting host data can still strand or replace terminal rows, while restart checks may miss an ownership regression. Merge readiness is moderate until these cases are addressed or explicitly accepted. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Linked Issues checkExplanation The PR satisfies the objectives of ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
73735c1 to
d31e30c
Compare
There was a problem hiding this comment.
ℹ️ No critical issues — the convergence is sound and well-tested. One adoption-coverage question worth confirming before merge, plus a minor comment fix inline.
Reviewed changes
- Partition-owner convergence —
workspaceSessionPartitionHostIddrops its secondsshOwnershipargument so every non-localhost owns its own partition, and the renderer write path, the runtime controller, andsession-owner-removal.tsall share the one answer. - Stranded-partition adoption — new
adoptStrandedHostPartitionSessiongap-fills a workspace the base holds no tabs for from the host partition, treating an empty tab row as a gap rather than an answer. - Boot hydration reads SSH partitions —
fetchWorkspaceSessionWithRuntimeHostOwnersnow fetchesssh:*partitions and adopts them out of band instead of feeding them to the contention split as rival claimants. - Per-target export fallback —
remoteWorkspace:setForConnectedTargetsresolves the no-sessionfallback per target fromlocal+ssh:<targetId>viapersistedSessionForTarget, instead of publishing the local blob alone. - Contention primary ranking —
pickPrimaryHostForClaimsranks rotatingruntime:ids last so an SSH claimant keeps its rows out of the runtime partition. - Tests — two new integration tests drive the real
Store/IPC handler and the real projection + pull-side merge; the PR documents mutation testing per regression.
ℹ️ Adoption skips sleeping-agent sessions, unlike the sibling pane-keyed field
adoptStrandedHostPartitionSession folds in worktreeKeyed, tabKeyed, paneKeyed, and worktreeArray fields, but not sleepingAgentKeyed (sleepingAgentSessionsByPaneKey), browserWorkspaceKeyed, or fileKeyed. The surface-tombstone exclusions are covered by the main-side sanitizeWorkspaceSessionTerminalRetirements fencing, and browser/editor rows are renderer-authored (so they still live in local). But sleepingAgentSessionsByPaneKey is also authored main-side (e.g. src/main/runtime/runtime-legacy-worker-terminal-recovery-persistence.ts) and is keyed by pane key exactly like terminalPtyIncarnationsByPaneKey, which the adoption does fold in — so the asymmetry is worth confirming is deliberate. The renderer is now a full-replace writer of the ssh:<targetId> partition, and there is no preserveRuntimeAuthoredWorkspaceSessionFields-style fence for sleeping agents. If a stranded SSH workspace can hold a runtime-authored sleeping agent in that partition, adoption would not recover it and the next replace write could drop it.
Technical details
# Adoption field-coverage asymmetry
## Affected sites
- src/shared/workspace-session-stranded-partition-adoption.ts:37-42 — `TAB_SCOPED_FIELDS` restricts to `tabKeyed` + `paneKeyed`, excluding `sleepingAgentKeyed`.
- src/shared/workspace-session-host-field-ownership.ts:50 — `sleepingAgentSessionsByPaneKey` is `sleepingAgentKeyed`, keyed by pane key like `terminalPtyIncarnationsByPaneKey` (`paneKeyed`).
- src/main/persistence/loading-store/session-host-partitions.ts:173-209 — `setHostWorkspaceSession` fences tombstones/pty incarnations/runtime browser pages but not sleeping agents.
## Required outcome
- Confirm whether sleeping-agent sessions for SSH workspaces can live in `ssh:<targetId>` (i.e. are authored by the runtime there). If yes, either fold `sleepingAgentSessionsByPaneKey` into the adoption for stranded workspaces, or add a fence analogous to `sanitizeWorkspaceSessionTerminalRetirements` so the renderer's replace write cannot drop them.
## Open questions for the human (optional)
- Is the exclusion of `sleepingAgentKeyed` from `TAB_SCOPED_FIELDS` intentional, or should a stranded workspace also recover its hibernated agent sessions?Note: 1 inline comment(s) dropped because they did not anchor to lines inside the PR diff:
src/renderer/src/lib/workspace-session-host-persistence.ts:176(RIGHT) — line 176 (RIGHT) is not inside a diff hunk
DeepSeek Pro (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: dc9a7b99-dac9-4803-a19e-96af126efe67
📥 Commits
Reviewing files that changed from the base of the PR and between 12f53da and 73735c179be8848644b46501719e2e7cd09cac8f.
📒 Files selected for processing (16)
src/main/ipc/remote-workspace-target-session-export.tssrc/main/ipc/remote-workspace.tssrc/main/ipc/ssh-host-partition-session-export.test.tssrc/main/persistence/restoring-sessions/session-owner-removal.tssrc/main/runtime/runtime-workspace-session-controller.tssrc/renderer/src/lib/workspace-session-host-contention.test.tssrc/renderer/src/lib/workspace-session-host-contention.tssrc/renderer/src/lib/workspace-session-host-hydration.tssrc/renderer/src/lib/workspace-session-host-persistence.tssrc/renderer/src/lib/workspace-session-host-split.test.tssrc/renderer/src/lib/workspace-session-host-split.tssrc/renderer/src/lib/workspace-session-ssh-partition-round-trip.test.tssrc/shared/workspace-session-host-field-ownership.tssrc/shared/workspace-session-partition-owner.test.tssrc/shared/workspace-session-partition-owner.tssrc/shared/workspace-session-stranded-partition-adoption.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review.
There was a problem hiding this comment.
ℹ️ The follow-up closes the adoption-coverage gap cleanly — no new issues in these changes. One trivial comment-accuracy nit remains, noted below.
Reviewed changes
- Self-describing field adoption —
adoptStrandedHostPartitionSessionnow also folds insleepingAgentSessionsByPaneKeyand the two surface-tombstone fields viaSELF_DESCRIBING_FIELDS, matching records whoseworktreeIdnames a stranded workspace. This resolves the gap raised in the prior review: a hibernated agent or tombstone held only inssh:<targetId>would otherwise be dropped by the renderer's next full-replace write. - Hibernated-agent regression test — new
adopts a hibernated agent record the host partition alone holdsassertion in the round-trip suite drives the real boot read and asserts the exact provider session id. - Reliability gate — registers
workspace-session.ssh-host-partition-round-tripinconfig/reliability-gates.jsoncwith accurateknownGaps(no live relay, folder workspaces, stranded-beside-populated rows).
ℹ️ Nitpicks
src/renderer/src/lib/workspace-session-host-persistence.ts:176still reads "'local' and every ssh host share one blob", which is no longer true once every non-localhost owns its own partition. Thepartitions.length > 1guard that comment justifies is now always true whenever more than one claimant exists, so the "collapse to a single partition" case it describes can no longer occur.
DeepSeek Pro (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 01c61ab0-23ab-4ea7-ac26-354cd76f717a
📒 Files selected for processing (5)
config/reliability-gates.jsoncsrc/renderer/src/lib/workspace-session-host-contention.tssrc/renderer/src/lib/workspace-session-ssh-partition-round-trip.test.tssrc/shared/workspace-scope.tssrc/shared/workspace-session-stranded-partition-adoption.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review.
| const adoptedTabIds = new Set<string>() | ||
| for (const [key, tabs] of Object.entries(host.tabsByWorktree ?? {})) { | ||
| if (!adopts(key) || !Array.isArray(tabs)) { | ||
| continue | ||
| } | ||
| next.tabsByWorktree[key] = tabs | ||
| for (const tab of tabs) { | ||
| adoptedTabIds.add(tab.id) | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
#!/bin/bash
# Description: Trace unified tab persistence and tab-keyed layout ownership.
set -euo pipefail
fd -t f 'workspace-session-state-types.ts' src/shared --exec sed -n '1,140p'
# Where unifiedTabs is written and whether tabsByWorktree is written alongside it.
rg -n -C 6 '\bunifiedTabs\b' --type=ts -g '!**/*.test.ts' src | head -200
# Which tab ids key terminalLayoutsByTabId / remoteSessionIdsByTabId.
rg -n -C 4 'terminalLayoutsByTabId|remoteSessionIdsByTabId|terminalPtyIncarnationsByPaneKey' --type=ts -g '!**/*.test.ts' src | head -200Repository: stablyai/orca
Length of output: 47206
🤖 get_repo_knowledge executed:
get_repo_knowledge stablyai/orca /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4/learnings /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4/conventions
Length of output: 37883
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- adoption implementation ---'
cat -n src/shared/workspace-session-stranded-partition-adoption.ts | sed -n '1,290p'
printf '%s\n' '--- unified tab and terminal tab definitions ---'
rg -n -C 8 'export (type|interface) Tab|type Tab =|contentType.*terminal|TerminalTab' src/shared/tab-types.ts src/shared/terminal-tab-types.ts src/shared -g '*.ts' | head -240
printf '%s\n' '--- unified tab persistence writers ---'
rg -n -C 10 'unifiedTabs\s*[:=]|unifiedTabsByWorktree|terminal.*unifiedTabs|contentType:\s*['\"']terminal' src/renderer src/shared -g '*.ts' -g '*.tsx' -g '!**/*.test.ts' -g '!**/*.test.tsx' | head -280Repository: stablyai/orca
Length of output: 50369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- direct callers and restore consumers ---'
rg -n -C 10 'adoptStranded|stranded.*partition|adoptedTabIds|terminalPtyIncarnationsByPaneKey|remoteSessionIdsByTabId' src/shared src/main src/relay -g '*.ts' | head -320
printf '%s\n' '--- terminal tab identity in unified-tab construction ---'
rg -n -C 10 'contentType:\s*['\"']terminal['\"']|contentType\s*===\s*['\"']terminal['\"']|isTerminal.*Tab|terminalTab' src/renderer/src src/shared -g '*.ts' -g '*.tsx' -g '!**/*.test.ts' -g '!**/*.test.tsx' | head -320Repository: stablyai/orca
Length of output: 50369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n src/shared/workspace-session-stranded-partition-adoption.ts | sed -n '130,275p'
printf '%s\n' '--- type declarations ---'
rg -n -C 12 'export type Tab|export interface Tab|export type TerminalTab|contentType' src/shared/tab-types.ts src/shared/terminal-tab-types.ts
printf '%s\n' '--- adoption symbols ---'
rg -n -C 12 'adoptedTabIds|workspace-session-stranded-partition-adoption|adopts\(' src -g '*.ts' -g '*.tsx' | head -260Repository: stablyai/orca
Length of output: 41149
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- unified terminal tab construction ---'
rg -n -C 12 'createUnifiedTab|entityId:.*terminal|contentType:.*terminal|unifiedTabsByWorktree.*terminal|terminalTabId.*entityId' src/renderer/src src/shared -g '*.ts' -g '*.tsx' -g '!**/*.test.ts' -g '!**/*.test.tsx' | head -260
printf '%s\n' '--- persistence mappings between unified and legacy terminal ids ---'
rg -n -C 12 'unifiedTabs.*tabsByWorktree|tabsByWorktree.*unifiedTabs|entityId.*tab.id|tab.id.*entityId|terminalTab\.id|terminalTabId' src/renderer/src src/shared -g '*.ts' -g '*.tsx' -g '!**/*.test.ts' -g '!**/*.test.tsx' | head -320Repository: stablyai/orca
Length of output: 50369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- focused terminal tab identity definitions ---'
rg -n -C 20 'function createUnifiedTab|createUnifiedTab\s*[:=]|const createUnifiedTab|entityId.*terminal tab ID|id:.*terminals' src/renderer/src src/shared -g '*.ts' -g '*.tsx' | head -220
printf '%s\n' '--- focused terminal synchronization ---'
rg -n -C 16 'unifiedTabsByWorktree|tabsByWorktree' src/renderer/src/store src/renderer/src/runtime -g '*.ts' -g '*.tsx' | rg -C 8 'terminal|entityId|unifiedTabs|tabsByWorktree' | head -260Repository: stablyai/orca
Length of output: 43897
Seed adoptedTabIds from terminal unifiedTabs using entityId.
adoptedTabIds currently includes only IDs from host.tabsByWorktree. A terminal Tab can exist only in unifiedTabs. Terminal state uses the backing terminal ID, entityId, while Tab.id is the unified-tab ID. The filters at Lines 208-217 can therefore drop its layout, relay session ID, and PTY incarnation.
🐛 Proposed fix
for (const [key, tabs] of Object.entries(host.tabsByWorktree ?? {})) {
if (!adopts(key) || !Array.isArray(tabs)) {
continue
}
next.tabsByWorktree[key] = tabs
for (const tab of tabs) {
adoptedTabIds.add(tab.id)
}
}
+ for (const [key, tabs] of Object.entries(asRecord(host.unifiedTabs) ?? {})) {
+ if (!adopts(key)) {
+ continue
+ }
+ for (const tab of Array.isArray(tabs) ? tabs : []) {
+ const record = asRecord(tab)
+ if (record?.contentType === 'terminal' && typeof record.entityId === 'string') {
+ adoptedTabIds.add(record.entityId)
+ }
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const adoptedTabIds = new Set<string>() | |
| for (const [key, tabs] of Object.entries(host.tabsByWorktree ?? {})) { | |
| if (!adopts(key) || !Array.isArray(tabs)) { | |
| continue | |
| } | |
| next.tabsByWorktree[key] = tabs | |
| for (const tab of tabs) { | |
| adoptedTabIds.add(tab.id) | |
| } | |
| } | |
| const adoptedTabIds = new Set<string>() | |
| for (const [key, tabs] of Object.entries(host.tabsByWorktree ?? {})) { | |
| if (!adopts(key) || !Array.isArray(tabs)) { | |
| continue | |
| } | |
| next.tabsByWorktree[key] = tabs | |
| for (const tab of tabs) { | |
| adoptedTabIds.add(tab.id) | |
| } | |
| } | |
| for (const [key, tabs] of Object.entries(asRecord(host.unifiedTabs) ?? {})) { | |
| if (!adopts(key)) { | |
| continue | |
| } | |
| for (const tab of Array.isArray(tabs) ? tabs : []) { | |
| const record = asRecord(tab) | |
| if (record?.contentType === 'terminal' && typeof record.entityId === 'string') { | |
| adoptedTabIds.add(record.entityId) | |
| } | |
| } | |
| } |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Adoption broadened to every named workspace —
adoptStrandedHostPartitionSessionnow discovers workspaces via an exhaustiveswitchoverWORKSPACE_SESSION_FIELD_OWNERSHIPrather than gating on terminal tabs, so an editor-only or browser-only SSH workspace is adopted as completely as a tabbed one. Exhaustiveness is actually lint-enforced here bytypescript/switch-exhaustiveness-check(allowDefaultCaseForExhaustiveSwitch: false), so a new ownership kind fails CI — the docstring's "compile-time decision" claim checks out. - Shared key normalization —
normalizeWorkspaceSessionKeyToWorkspaceIdmoved toshared/workspace-scope.tsand re-exported from the contention module, so the adoption read and the write side share one normalization (including host-qualified recency keys). - Write/read round-trip + editor/browser coverage — new tests feed the shipping
buildWorkspaceSessionHostSnapshotsoutput back through the real boot read (instead of a hand-built fixture), and pin open editor files, unsaveddirtyDraftContent, browser workspaces, and the no-tabsByWorktree-key shape. - Older-client skew direction — a test pins that a pre-fix client's empty
replace-sessionpublish no longer deletes the tabs this build has hydrated, closing the one-hop data-loss direction from Rule 3. - Reliability gate — invariant/oracle/evidence updated to reflect the broader adoption and the skew-direction assertion.
The read-side adoption was cross-checked field-by-field against splitWorkspaceSessionByHost; the routing is symmetric, and the replace: true semantics for worktreeKeyed fields are safe in the real transition shapes (the runtime never wrote editor/browser state to ssh:<targetId>, so there is no path that overwrites current local state with stale host rows).
DeepSeek Pro (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Empty-host-row guard —
adoptStrandedHostPartitionSessionnow treats[],{}and null/undefined host entries as "nothing to say" (hostHasNothingFor), so an empty hostopenFilesByWorktreerow can no longer replace a populated base row and destroy an unsaveddirtyDraftContent.adoptRecord'sreplaceflag becamemayReplace: boolean | ((key) => boolean), with the guard folded into the replace predicate. - Contested-id awareness —
mergeWorkspaceSessionsWithHostShadownow surfacescontestedSessionKeys, passed through hydration into adoption, so a contested bare id is gap-filled rather than replaced (that id is wherelocalandssh:<targetId>are not one workspace written twice). - Shared routing indexes — tab-, pane- and file-keyed rows are recovered through the same
buildWorktreeIdByTabId/worktreeIdForPaneKey/buildWorktreeIdByFileIdindexes the split routed them by (moved toshared/), which also restores unified-only-tab layouts and keeps read/write symmetric. - Recency-key safety — bare
lastVisitedAtByWorktreeIdkeys only gap-fill (indistinguishable from another host's entry) while host-qualified keys replace, closing the cross-workspace Cmd+J recency move. - Tombstone-boundary tests — new assertions pin the closed-last-terminal empty-row semantics against the real
shouldAutoCreateInitialTerminalpredicate, plus the write-side precondition that the tombstone lands in the owning partition and leaves nolocalrow. - Reliability gate — invariant, oracle, and
knownGapsupdated to record the adoption guards and the tombstone boundary.
The guard and contested-id changes are correctly scoped: the replace predicate only applies to worktreeKeyed fields, the empty-entry check is deliberately asymmetric with the terminal tombstone (an empty tabsByWorktree row must be preserved, an empty editor row must not replace), and the recency special-casing keys off isHostQualifiedSessionKey rather than a bare field name. All 76 relevant tests pass locally, and the import graph for the moved workspace-session-host-records module is clean.
DeepSeek Pro (free via Pullfrog for OSS) | 𝕏
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 78347662-9e78-4574-b864-cb5539ce2ec2
📒 Files selected for processing (7)
config/reliability-gates.jsoncsrc/renderer/src/lib/workspace-session-host-contention.tssrc/renderer/src/lib/workspace-session-host-hydration.tssrc/renderer/src/lib/workspace-session-host-split.tssrc/renderer/src/lib/workspace-session-ssh-partition-round-trip.test.tssrc/shared/workspace-session-host-records.tssrc/shared/workspace-session-stranded-partition-adoption.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.
| } | ||
| }) | ||
| ), | ||
| Promise.all(sshHostIds.map((hostId) => readPartition(hostId))) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Preserve contention between SSH partitions.
This code discards each SSH slice's host ID and adopts the slices sequentially. If two SSH targets contain the same bare workspace ID, merged.contestedSessionKeys does not include that conflict.
The first slice creates the populated base row. The next slice is then skipped and is not added to contestedHostWorkspaceSessions. A later replacement write can erase the omitted host partition.
Keep each SSH slice paired with its host ID. Detect normalized SSH-to-SSH claims and preserve non-primary rows in the contention shadow.
Also applies to: 191-194
| for (const [key, tabs] of Object.entries(base.tabsByWorktree ?? {})) { | ||
| if (Array.isArray(tabs) && tabs.length > 0) { | ||
| owned.add(normalizeWorkspaceSessionKeyToWorkspaceId(key)) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Treat terminal unifiedTabs as authoritative base ownership.
workspacesTheBaseOwns checks only tabsByWorktree. A session can hold live terminal tabs only in unifiedTabs, which readers prefer over the legacy field.
In that state, adoption treats the workspace as empty. The host row can then replace unifiedTabs and restore stale or closed tabs. Add worktree IDs from populated terminal unifiedTabs to owned. Add a regression test with an empty legacy row and a populated unified terminal row.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 261ccd5d-d10a-4e0b-8a44-066c72bf083a
📒 Files selected for processing (1)
tests/e2e/ssh-cold-activation-restore.spec.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.
| [ | ||
| ...(local.tabsByWorktree[worktreeId] ?? []), | ||
| ...(host.tabsByWorktree[worktreeId] ?? []) | ||
| ].map((tab) => tab.id) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert SSH tabs in the owning partition.
The merged arrays make both tests pass when tabsByWorktree[worktreeId] remains only in local. This does not detect a regression in SSH partition routing. Require the tab assertion to read host.tabsByWorktree[worktreeId] for this SSH worktree.
tests/e2e/ssh-cold-activation-restore.spec.ts#L97-L100: buildpersistedTabIdsfromhost.tabsByWorktree[worktreeId].tests/e2e/ssh-cold-activation-restore.spec.ts#L272-L275: searchhost.tabsByWorktree[worktreeId]fortabId.
📍 Affects 1 file
tests/e2e/ssh-cold-activation-restore.spec.ts#L97-L100(this comment)tests/e2e/ssh-cold-activation-restore.spec.ts#L272-L275
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
ssh-cold-activation-restorepartition read — both persistence checks now unionlocalandssh:<targetId>fortabsByWorktreeand readactiveConnectionIdsAtShutdownfromlocal, so they assert the persisted-where-the-boot-read-finds-it invariant instead of the now-correct partition layout.ssh-restart-tab-accumulationpartition read —flushSessionBeforeQuitapplies the same union to its pre-quit flush check.
Both commits are e2e-spec-only, and the behavioral half of each test (full app restart, active-worktree resolution, eager terminal remount, PTY-owner reclaim) is unchanged, so the fix is still exercised end to end rather than only via the white-box read.
DeepSeek Pro (free via Pullfrog for OSS) | 𝕏
4b0d984 to
f6cad61
Compare
…stop round-tripping as deletions `workspaceSessionPartitionHostId` answered differently depending on who asked: the renderer mapped an SSH worktree's session to the `local` blob, the main-process runtime read-modify-wrote `ssh:<targetId>`. One workspace's session lived in two stores and no reader reunited them, so whatever landed on the unread side did not read as unknown — it round-tripped as absence. The remote-workspace upload is a `replace-session` patch, which turned that absence into deletion on the host, and the next pull applied the deletion locally and re-poisoned the snapshot. Collapse the two answers into one: every non-'local' host owns its partition. Boot hydration and the export fallback now read the SSH partition, and rows a shipping build left in `local` are folded back in once, gap-filling only — an empty tab row is a gap, never proof that anything was closed. Folder workspaces deliberately keep their existing 'local' routing: boot discovers SSH partitions from the repo catalog, so an SSH target that owns only a folder workspace has no partition any reader enumerates. They are still adopted back out of an SSH partition when a repo does name the host. Fixes #12721 Supersedes #12722 Co-authored-by: Robert Nisipeanu <github@nisipeanu.com> Co-authored-by: Jinwoo-H <Jinwoo-H@users.noreply.github.com>
…y tabbed ones Review caught that gating adoption on `host.tabsByWorktree[key].length > 0` traded the #12721 deletion for a narrower one. The write path routes EVERY worktree-scoped field to the owning partition, so an SSH workspace with open editor files or browser tabs and no terminals had all of it dropped on every restart — and unlike terminal state it cannot be recovered from the host snapshot, which carries terminal fields only, so an unsaved `dirtyDraftContent` was destroyed outright. The defect was not a missing field. It was a hand-maintained field list deciding what the read recovers while the write used the ownership table, so the two could disagree. Adoption now walks `WORKSPACE_SESSION_FIELD_OWNERSHIP` with an exhaustive switch, and a new ownership kind is a compile-time decision rather than a silent omission. Session keys are normalized through the shared `normalizeWorkspaceSessionKeyToWorkspaceId` so host-qualified visit recency (`ssh:target|worktreeId`) reaches its workspace, and the regression is pinned by feeding the shipping split's own output back through the real boot read rather than a hand-built fixture. Co-authored-by: Robert Nisipeanu <github@nisipeanu.com> Co-authored-by: Jinwoo-H <Jinwoo-H@users.noreply.github.com>
Three losses, one cause: the reader walks its own description of the partition layout while the writer walks another, so the two agree on which ownership kinds exist and not on what a kind means. - An empty host row replaced a populated base row, destroying an unsaved dirtyDraftContent the header comment says must never be destroyed. The host holding nothing is not evidence the base is wrong. - A contested bare id was adopted as if local and ssh:<target> were one workspace written twice, which is exactly the id where that premise is false. The read already reached that verdict and adoption could not ask for it, so it is passed in; contested keys are gap-filled, never replaced. mergeWorkspaceSessionsWithHostShadow now reports the real contested set, which primaryHostBySessionKey never was. - Tab-, pane- and file-keyed rows are adopted through the split's own indexes, so unified-only tabs come back and the pane key is parsed once. - A bare lastVisitedAtByWorktreeId key only fills a gap; the split has a dedicated branch for that field and the reader had none.
An explicit empty tabsByWorktree row means the user closed the last terminal; adoption reads an empty base row as a gap to fill. Same value, opposite readings, so the boundary is asserted rather than argued: the tombstone lands in the owning partition, restores as a present empty row rather than a deleted key, is declined by the real seeding predicate, is published as an empty list, and the legacy-transition resurrection happens once and cannot recur.
…ns them ssh-cold-activation-restore asserted persistence through session.get() with no host, which is the local partition an SSH worktree's rows no longer live in. The invariant it means to check is that the state is persisted where the boot read will find it, so it now unions local and ssh:<targetId> and stays correct on both layouts. Confirmed the product invariant separately rather than by the edit: the behavioural half of both tests - the full app restart, the active worktree, the eager terminal remount and the PTY-owner reclaim against a real Docker OpenSSH host - runs after this check and passes. 2 passed in 48.9s.
…n too Same layout-coupled read as ssh-cold-activation-restore: the pre-quit flush asserted through session.get() with no host. Verified against a real Docker OpenSSH target - both repeated quit/relaunch cycles keep exactly the restored SSH tabs, no accumulation and no loss. 2 passed in 52.9s.
f6cad61 to
e37ff5a
Compare
main tightened typescript/consistent-type-assertions to assertionStyle: never, which the rebase brings onto these added lines. Most of the round-trip fixtures did not need a cast at all -- three were hiding wrong-shaped literals (a browser workspace keyed 'name', a unified tab keyed 'type', a layout keyed 'direction'), now written as the types they stand for. The adoption reads narrow through an isRecord predicate instead of casting, which also stops a null entry throwing out of Object.keys. What is left is dynamic-field writes and unknown-typed IPC returns, each with its own SAFETY rationale.
…can find The partition owner rule already names `ssh:<targetId>` for a repo-backed worktree, but `getFolderWorkspacePartitionHostId` still answered 'local' for a folder workspace while main's `RuntimeWorkspaceSessionController.getPreferredHostId` answered `ssh:<targetId>` for the same key. That is #12723 unfixed for folder workspaces, and once the renderer started writing `ssh:*` at all it got worse: a save's field-level patch carries only the rows routed to that partition, so a `tabsByWorktree` write without the folder row erased the row main had put there. The reason the renderer could not route there was real - boot discovered SSH partitions from the repo catalog, which cannot name a target whose only workspace is a folder. So persistence now answers that directly over `session:list-host-ids`, and boot reads the partitions that exist rather than the ones a catalog implies. Removing a folder workspace prunes its rows from the owning partition too, or the census would adopt them back on the next launch as a workspace the user already deleted. Adoption now decides from the repo catalog instead of from co-presence. Two partitions holding one bare `repoId::path` is not evidence of a collision - that is the exact shape the repair exists for - so the verdict comes from `resolveWorktreeExecutionHost`: a repo id registered on more than one host is contested and may only be gap-filled, and one the catalog positively resolves to a different host is residue this partition does not own and is not adopted at all. Without the second rule a stale partition sorting first won the read and was then written into the live one. Nothing is deleted either way; the rows stay where they are. Finally, a workspace adopted out of a partition now routes back to that partition. Routing used to re-derive an owner from the catalog, so a boot whose repos had not hydrated moved the rows it had just reunited back into 'local' and re-stranded them. Contested ids are withheld from that override, because routing the whole bare id to one host is the loss the gap-fill prevents. The publish path resolves each workspace's owner once for the whole publish, shared with the projection, so the per-target catalog attribution does not repeat it per connected host.
There was a problem hiding this comment.
ℹ️ No critical issues — the folder-workspace convergence and partition census are sound and well-tested. One routing-parity question worth confirming before merge, inline.
Reviewed changes
- Folder workspaces own their host partition —
getFolderWorkspacePartitionHostIdnow returns any parsed host id rather than onlyruntime:, andremoveFolderWorkspacesweepslocalplus the owning partition throughworkspaceSessionPartitionIdsForHost. - Boot enumerates partitions from persistence — the new
session:list-host-idschannel (backed byStore.getWorkspaceSessionHostIds) replaces catalog inference for SSH targets in both desktop hydration and the web client, and stays optional so an older main still boots. - Catalog attribution for adopted SSH rows — hydration and the main publish fallback compute
contested(ambiguous repo id, or two SSH partitions naming one id — never barelocal+sshco-presence) andforeign(catalog attributes the id elsewhere) keys; foreign keys are withheld from adoption, andadoptedWorkspaceIdslets the write return rows to the partition they were read from. - Coverage and gate — new
workspace-session-ssh-partition-ownership.test.tsplus updated reliability-gate invariant/oracle/knownGaps; all PR suites pass locally (78 tests across 6 files, re-run this session).
ℹ️ The folder routing resolves a narrower owner than main does
getFolderWorkspacePartitionHostId derives the host from the optional executionHostId alone. For a folder record that carries a connectionId but no stamp (the shape workspace-session-host-persistence.test.ts:254 models), it answers local, while main's RuntimeWorkspaceSessionController.getPreferredHostId falls back to resolveFolderConnectionId — connectionId and path inference — and answers ssh:<connectionId>. The sibling resolvers this file sits beside (getFolderWorkspaceHostId, getExecutionHostIdForFolderWorkspace) both apply that fallback, so the invariant "the same partition main writes it to" holds only when the catalog stamp is present.
Technical details
# Folder partition resolution omits main's owner fallbacks
## Affected sites
- `src/renderer/src/lib/workspace-session-host-persistence.ts:89-98` — resolves the host only from `workspace?.executionHostId ?? group?.executionHostId`, then returns it.
- `src/renderer/src/lib/workspace-session-host-persistence.ts:100-104` — `workspace && group` returns `local` before `state.contestedPrimaryHostBySessionKey[key]` is consulted.
- `src/main/runtime/runtime-workspace-session-controller.ts:28-54` — main's `getPreferredHostId` falls back to `resolveFolderConnectionId` (which uses `inferFolderWorkspacePathConnection`, i.e. `connectionId` and repo-path inference).
- `src/renderer/src/store/folder-workspaces/folder-workspace-catalog.ts:76-100` and `src/renderer/src/lib/folder-workspace-runtime-owner.ts:137-168` — the renderer's own folder-owner resolvers do apply the `connectionId` fallback.
## Required outcome
- Either confirm no unstamped folder record can reach `buildHostSessionRouting` (in which case the catalog stamp is the sole source of truth and this is inert), or have `getFolderWorkspacePartitionHostId` apply the same `connectionId` fallback the sibling resolvers use.
- Decide the intended precedence for a folder the read adopted out of `ssh:<targetId>` when the folder/group catalog is hydrated but names no host: today the write returns it to `local`, which re-homes rows the read just reunited.
## Open questions for the human
- Is the `executionHostId` stamp always present on `AppState.folderWorkspaces` before the first session write, including a boot that has not completed the folder catalog fetch?DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
| // `ssh:<targetId>` at all, a save's field-level patch erased the folder rows main had put | ||
| // there. Boot reads these partitions from persistence's own census, not the repo catalog, so | ||
| // a target whose only workspace is a folder is no longer unenumerated. | ||
| return parsed.id |
There was a problem hiding this comment.
getFolderWorkspacePartitionHostId now trusts any parsed host, but it resolves that host from the optional executionHostId alone. A folder record carrying a connectionId but no stamp routes local here while main's RuntimeWorkspaceSessionController.getPreferredHostId falls back to resolveFolderConnectionId (connectionId and repo-path inference) and routes ssh:<connectionId> — the two-store split this PR closes for repo-backed worktrees. The sibling resolvers this function sits beside (getFolderWorkspaceHostId, getExecutionHostIdForFolderWorkspace) both apply that fallback, and the workspace && group branch just below returns local before contestedPrimaryHostBySessionKey, so a folder adopted from ssh:<targetId> can be routed back to local on the next write.
Technical details
# Folder partition resolution omits main's owner fallbacks
## Affected sites
- `src/renderer/src/lib/workspace-session-host-persistence.ts:89-98` — resolves only `executionHostId`.
- `src/renderer/src/lib/workspace-session-host-persistence.ts:100-104` — `local` returned before the read-source override.
- `src/main/runtime/runtime-workspace-session-controller.ts:28-54` — main applies `resolveFolderConnectionId`.
- `src/renderer/src/store/folder-workspaces/folder-workspace-catalog.ts:76-100`, `src/renderer/src/lib/folder-workspace-runtime-owner.ts:137-168` — sibling resolvers apply the `connectionId` fallback.
## Required outcome
- Confirm no unstamped folder record reaches `buildHostSessionRouting`, or apply the same `connectionId` fallback.
- Decide precedence for an adopted folder when the catalog is hydrated but names no host (today the write re-homes it to `local`).… the local blob Adversarial review of the previous commit found three ways the partition census - which now reads whatever persistence holds rather than what the repo catalog implies - keeps rows alive that nothing should keep alive. `deleteProjectGroup` pruned only the local blob, so every folder workspace under a deleted group left its rows in `ssh:<targetId>`; the next boot adopted them back, named that partition their owner and wrote them there again, forever. `removeFolderWorkspace` had the same hole for a workspace whose partition its host expression could not name: main never persists a folder workspace's `executionHostId`, and `RuntimeWorkspaceSessionController` can infer a connection from the group's repos that the workspace row itself does not carry. Deriving the partition at delete time is the wrong question - a deleted workspace owns nothing anywhere - so both paths now remove it from every partition. The third is on the read side. A contested id is deliberately withheld from the read-source override so the write cannot carry one host's rows into another's partition, but the routing that then re-derives an owner answers 'local' for an id the catalog cannot name. Adopting such a row moved it out of the partition that owns it and into the blob: the two-store split this change exists to remove. A contested id the assembled session holds no row for is therefore not adopted at all. Gap-filling stays available for a contested id the session already names, since that row's own partition is what the write follows. Declining to adopt leaves a row invisible for one boot; it never deletes one. Also: the folder-key guard in both catalog attributions was dead, because `getRepoIdFromWorktreeId` hands back the whole key rather than nothing when there is no `::`. The verdict was right and the resolution wasted; it now skips by shape. And the two type assertions the changed-code casting gate rejected are gone rather than suppressed.
There was a problem hiding this comment.
Caution
This delta closes real gaps, but the new unownedSessionKeys rule removes the only thing that previously kept a contested row alive: a contested id the assembled session does not name is now neither adopted nor carried by the write, so the next full snapshot write to that SSH partition erases it. Details inline.
Reviewed changes
- Deletion sweeps every partition — new
removeWorkspaceSessionOwnerEverywhereis used by bothremoveFolderWorkspaceanddeleteProjectGroup, so a deleted folder workspace no longer leaves rows inssh:<targetId>that the next boot would adopt back as a workspace that no longer exists. - Contested ids the session does not name are withheld from adoption —
unownedSessionKeysfolds such ids into theforeignSessionKeysthe read already declines, so they cannot be routed intolocal. - Dead folder guard replaced — both catalog attributions now skip non-worktree keys by
includes('::')shape rather than the never-falsegetRepoIdFromWorktreeIdcheck (which returns the whole key when there is no separator). - Tests — a project-group deletion sweep assertion, a contested-id non-adoption assertion, and a normalized
listHostIdsfake that mirrors the real persistence census.
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
…ing the next write erase them A partition write replaces each field with exactly what the unified session routed there. So a row the read left out of that session is erased from its own partition the moment any sibling workspace writes the same one - and with SSH partitions now the owning store, that row is then in no partition at all. Three separate decisions produce such rows: residue the catalog attributes to another host, a contested id withheld so the write cannot carry one host's rows into another's partition, and a workspace the base already holds the live copy of. Declining to show a row was quietly deleting it. The machinery for this already exists. `attachHostSessionShadow` writes a contested runtime co-claimant's parked rows straight back into its own slice before the write, so the primary's write cannot erase them; the ssh partitions simply were not among the slices the contention split arbitrates. The read now parks everything it is not returning to an ssh partition into that same shadow, and the existing re-attach puts it back. Leak, never kill - docs/reference/ssh-execution- boundary.md - and a row no partition holds is unrecoverable. Second, the contested branch of the tab adoption read `Object.hasOwn` as "the base has tabs here". An empty list satisfies it, so whenever a legacy id happened to be contested, #12721's empty local row won over the host's real one - the exact reading the module's own header, and the gate invariant it is pinned by, say is wrong. An empty row is the gap this repair fills, so it is now treated as one.
Mutation testing found the assertion missing: reverting the gate to `Object.hasOwn` left all 39 assertions passing, which makes the fix that reads an empty base tab row as a gap unguarded. The #12721 shape does not stop being a gap because the id happens to be contested.
There was a problem hiding this comment.
Important
The parking added here closes the deletion the prior review flagged — a declined row now survives attachHostSessionShadow on the next partition write. But nothing drops a parked key when its workspace is later deleted, so the same shadow re-adds the rows removeWorkspaceSessionOwnerEverywhere (from 90bd71f) just swept, and the next boot adopts the deleted workspace back. Details inline, plus two smaller notes on the parking's scope and an untested contested branch.
Reviewed changes
- Declined ssh rows are parked, not dropped —
partitionRowsTheWriteWontReturnreturns everyworktreeKeyedrow anssh:<targetId>partition holds for a workspace the read did not adopt (foreign residue, withheld contested id, or a workspace the base already holds the live copy of), hydration stores it inshadow[hostId], and the existingattachHostSessionShadowre-attaches it before the write. A new test drives the shipping patch writer with a sibling workspace routing to the same partition. - Contested tab adoption treats an empty base row as a gap — the contested branch switched from
!Object.hasOwn(next.tabsByWorktree, key)tohostHasNothingFor(next.tabsByWorktree[key]), so a contested id whose base row is[]no longer blocks the host's real tab list. - New deletion-survival test —
keeps a declined row in its own partition when a sibling workspace is writtenasserts the ssh patch carries both the adopted sibling and the declined row.
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
| if (slice) { | ||
| const parked = partitionRowsTheWriteWontReturn(slice, adoption.adoptedWorkspaceIds) | ||
| if (parked) { | ||
| shadow[hostId] = parked | ||
| } |
There was a problem hiding this comment.
Parking a declined row keeps it alive across the next write, but nothing removes it when the workspace is later deleted. contestedHostWorkspaceSessions is written only at boot (use-app-startup-hydration.ts:204); no worktree/folder/group removal path clears it, and hostStillClaimsKey keeps a row whenever the catalog has no claim — which is exactly the post-delete state. So removeWorkspaceSessionOwnerEverywhere (added in 90bd71f to sweep every partition) is undone: the next write re-adds the key to ssh:<targetId> and the following boot adopts the deleted workspace back.
Technical details
# A deleted workspace's parked ssh row is re-added after the deletion sweep
## Affected sites
- `src/renderer/src/lib/workspace-session-host-hydration.ts:232-236` — parks the declined row into `shadow[hostId]`.
- `src/renderer/src/lib/workspace-session-host-contention.ts:297-302` — `attachHostSessionShadow` writes parked keys back into the slice.
- `src/renderer/src/lib/workspace-session-host-contention.ts:249-258` — `hostStillClaimsKey` returns true when `claims` has no entry for the key.
- `src/renderer/src/app-shell/use-app-startup-hydration.ts:204` — the only writer of `contestedHostWorkspaceSessions`; `workspace-terminal-hydration.ts:177-179` only overwrites it when an option is passed (startup read).
- `src/main/persistence/restoring-sessions/session-owner-removal.ts:71-88` — `removeWorkspaceSessionOwnerEverywhere`, the sweep this defeats.
- `src/renderer/src/lib/workspace-session-host-persistence.ts:218` — the write that applies the shadow.
## Concrete shape
`local = { tabsByWorktree: { W: [tab] } }`, `ssh:target-1 = { tabsByWorktree: { W: [tab-stale] } }`, plus another workspace `S` on `target-1`.
- Boot: the base owns `W`, so `W`'s ssh row is parked (the "skipped because the base already holds the live copy" case in the new docblock).
- Delete `W`. Main sweeps `local` + `ssh:target-1`; the renderer purges its tab maps.
- Next debounced write routes `S` to `ssh:target-1`; `attachHostSessionShadow` re-adds `W` (`claims` has no entry, so the guard passes).
- Next boot: `local` no longer owns `W`, so the ssh row is adoptable and `W` is resurrected.
## Required outcome
- A deleted workspace's key must not be re-added from the shadow. Clear the key from `contestedHostWorkspaceSessions` on worktree/folder/group removal in the renderer, or make the re-attach require positive evidence the workspace still exists rather than treating absence of a claim as permission.
## Open question for the human
- Is the shadow expected to be recomputed after a removal (I found no delete-path writer), or is this resurrection considered acceptable because it is non-destructive?| ): WorkspaceSessionState | null { | ||
| let parked: KeyedRecord | null = null | ||
| for (const field of SESSION_FIELDS) { | ||
| if (WORKSPACE_SESSION_FIELD_OWNERSHIP[field] !== 'worktreeKeyed') { |
There was a problem hiding this comment.
The docblock above promises the invariant in general — "declining to show a row must never mean deleting it" — but this guard parks only worktreeKeyed fields. The split also routes tabKeyed, paneKeyed, fileKeyed, sleepingAgentKeyed, browserWorkspaceKeyed, surfaceTombstoneKeyed and worktreeArray rows, so a declined workspace's terminalLayoutsByTabId, remoteSessionIdsByTabId and terminalPtyIncarnationsByPaneKey entries are still erased by the next full-replace partition write. This is pre-existing for the local/runtime shadow, but the new docblock now states it more broadly than the code holds; either narrow the doc or extend the parking to every non-global/non-hostPrivate kind.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Contested empty-base-row coverage — added
still reads an empty base tab row as a gap when the id is contestedtoworkspace-session-ssh-partition-ownership.test.ts, drivingcollidingPartitions(baselocal.tabsByWorktree[K] = [],ssh:partition holdingtab-rival) through the real boot read. It pins the contested tab branch the prior review flagged as untested: reverting toObject.hasOwnnow fails it.
The two points from the prior review (the parked-row/deletion interaction and the parking-scope docblock) are untouched by this test-only commit and remain open.
DeepSeek Flash (free via Pullfrog for OSS) | 𝕏
…stop round-tripping as deletions (stablyai#19572) * fix(session): give an SSH workspace one owning partition so its tabs stop round-tripping as deletions `workspaceSessionPartitionHostId` answered differently depending on who asked: the renderer mapped an SSH worktree's session to the `local` blob, the main-process runtime read-modify-wrote `ssh:<targetId>`. One workspace's session lived in two stores and no reader reunited them, so whatever landed on the unread side did not read as unknown — it round-tripped as absence. The remote-workspace upload is a `replace-session` patch, which turned that absence into deletion on the host, and the next pull applied the deletion locally and re-poisoned the snapshot. Collapse the two answers into one: every non-'local' host owns its partition. Boot hydration and the export fallback now read the SSH partition, and rows a shipping build left in `local` are folded back in once, gap-filling only — an empty tab row is a gap, never proof that anything was closed. Folder workspaces deliberately keep their existing 'local' routing: boot discovers SSH partitions from the repo catalog, so an SSH target that owns only a folder workspace has no partition any reader enumerates. They are still adopted back out of an SSH partition when a repo does name the host. Fixes stablyai#12721 Supersedes stablyai#12722 Co-authored-by: Robert Nisipeanu <github@nisipeanu.com> Co-authored-by: Jinwoo-H <Jinwoo-H@users.noreply.github.com> * test(session): pin the old-client empty-publish skew direction * fix(session): adopt every workspace the host partition names, not only tabbed ones Review caught that gating adoption on `host.tabsByWorktree[key].length > 0` traded the stablyai#12721 deletion for a narrower one. The write path routes EVERY worktree-scoped field to the owning partition, so an SSH workspace with open editor files or browser tabs and no terminals had all of it dropped on every restart — and unlike terminal state it cannot be recovered from the host snapshot, which carries terminal fields only, so an unsaved `dirtyDraftContent` was destroyed outright. The defect was not a missing field. It was a hand-maintained field list deciding what the read recovers while the write used the ownership table, so the two could disagree. Adoption now walks `WORKSPACE_SESSION_FIELD_OWNERSHIP` with an exhaustive switch, and a new ownership kind is a compile-time decision rather than a silent omission. Session keys are normalized through the shared `normalizeWorkspaceSessionKeyToWorkspaceId` so host-qualified visit recency (`ssh:target|worktreeId`) reaches its workspace, and the regression is pinned by feeding the shipping split's own output back through the real boot read rather than a hand-built fixture. Co-authored-by: Robert Nisipeanu <github@nisipeanu.com> Co-authored-by: Jinwoo-H <Jinwoo-H@users.noreply.github.com> * fix(session): stop adoption overwriting rows it was never told about Three losses, one cause: the reader walks its own description of the partition layout while the writer walks another, so the two agree on which ownership kinds exist and not on what a kind means. - An empty host row replaced a populated base row, destroying an unsaved dirtyDraftContent the header comment says must never be destroyed. The host holding nothing is not evidence the base is wrong. - A contested bare id was adopted as if local and ssh:<target> were one workspace written twice, which is exactly the id where that premise is false. The read already reached that verdict and adoption could not ask for it, so it is passed in; contested keys are gap-filled, never replaced. mergeWorkspaceSessionsWithHostShadow now reports the real contested set, which primaryHostBySessionKey never was. - Tab-, pane- and file-keyed rows are adopted through the split's own indexes, so unified-only tabs come back and the pane key is parsed once. - A bare lastVisitedAtByWorktreeId key only fills a gap; the split has a dedicated branch for that field and the reader had none. * test(session): pin the tombstone/gap boundary the two readings meet at An explicit empty tabsByWorktree row means the user closed the last terminal; adoption reads an empty base row as a gap to fill. Same value, opposite readings, so the boundary is asserted rather than argued: the tombstone lands in the owning partition, restores as a present empty row rather than a deleted key, is declined by the real seeding predicate, is published as an empty list, and the legacy-transition resurrection happens once and cannot recur. * docs(reliability): record the adoption guards and the tombstone boundary in the gate * test(e2e): read the SSH restart assertions from the partition that owns them ssh-cold-activation-restore asserted persistence through session.get() with no host, which is the local partition an SSH worktree's rows no longer live in. The invariant it means to check is that the state is persisted where the boot read will find it, so it now unions local and ssh:<targetId> and stays correct on both layouts. Confirmed the product invariant separately rather than by the edit: the behavioural half of both tests - the full app restart, the active worktree, the eager terminal remount and the PTY-owner reclaim against a real Docker OpenSSH host - runs after this check and passes. 2 passed in 48.9s. * test(e2e): read ssh-restart-tab-accumulation from the owning partition too Same layout-coupled read as ssh-cold-activation-restore: the pre-quit flush asserted through session.get() with no host. Verified against a real Docker OpenSSH target - both repeated quit/relaunch cycles keep exactly the restored SSH tabs, no accumulation and no loss. 2 passed in 52.9s. * fix(lint): clear the casting gate on the partition adoption main tightened typescript/consistent-type-assertions to assertionStyle: never, which the rebase brings onto these added lines. Most of the round-trip fixtures did not need a cast at all -- three were hiding wrong-shaped literals (a browser workspace keyed 'name', a unified tab keyed 'type', a layout keyed 'direction'), now written as the types they stand for. The adoption reads narrow through an isRecord predicate instead of casting, which also stops a null entry throwing out of Object.keys. What is left is dynamic-field writes and unknown-typed IPC returns, each with its own SAFETY rationale. * fix(session): give an SSH folder workspace one owning partition boot can find The partition owner rule already names `ssh:<targetId>` for a repo-backed worktree, but `getFolderWorkspacePartitionHostId` still answered 'local' for a folder workspace while main's `RuntimeWorkspaceSessionController.getPreferredHostId` answered `ssh:<targetId>` for the same key. That is stablyai#12723 unfixed for folder workspaces, and once the renderer started writing `ssh:*` at all it got worse: a save's field-level patch carries only the rows routed to that partition, so a `tabsByWorktree` write without the folder row erased the row main had put there. The reason the renderer could not route there was real - boot discovered SSH partitions from the repo catalog, which cannot name a target whose only workspace is a folder. So persistence now answers that directly over `session:list-host-ids`, and boot reads the partitions that exist rather than the ones a catalog implies. Removing a folder workspace prunes its rows from the owning partition too, or the census would adopt them back on the next launch as a workspace the user already deleted. Adoption now decides from the repo catalog instead of from co-presence. Two partitions holding one bare `repoId::path` is not evidence of a collision - that is the exact shape the repair exists for - so the verdict comes from `resolveWorktreeExecutionHost`: a repo id registered on more than one host is contested and may only be gap-filled, and one the catalog positively resolves to a different host is residue this partition does not own and is not adopted at all. Without the second rule a stale partition sorting first won the read and was then written into the live one. Nothing is deleted either way; the rows stay where they are. Finally, a workspace adopted out of a partition now routes back to that partition. Routing used to re-derive an owner from the catalog, so a boot whose repos had not hydrated moved the rows it had just reunited back into 'local' and re-stranded them. Contested ids are withheld from that override, because routing the whole bare id to one host is the loss the gap-fill prevents. The publish path resolves each workspace's owner once for the whole publish, shared with the projection, so the per-target catalog attribution does not repeat it per connected host. * fix(session): drop a deleted workspace from every partition, not just the local blob Adversarial review of the previous commit found three ways the partition census - which now reads whatever persistence holds rather than what the repo catalog implies - keeps rows alive that nothing should keep alive. `deleteProjectGroup` pruned only the local blob, so every folder workspace under a deleted group left its rows in `ssh:<targetId>`; the next boot adopted them back, named that partition their owner and wrote them there again, forever. `removeFolderWorkspace` had the same hole for a workspace whose partition its host expression could not name: main never persists a folder workspace's `executionHostId`, and `RuntimeWorkspaceSessionController` can infer a connection from the group's repos that the workspace row itself does not carry. Deriving the partition at delete time is the wrong question - a deleted workspace owns nothing anywhere - so both paths now remove it from every partition. The third is on the read side. A contested id is deliberately withheld from the read-source override so the write cannot carry one host's rows into another's partition, but the routing that then re-derives an owner answers 'local' for an id the catalog cannot name. Adopting such a row moved it out of the partition that owns it and into the blob: the two-store split this change exists to remove. A contested id the assembled session holds no row for is therefore not adopted at all. Gap-filling stays available for a contested id the session already names, since that row's own partition is what the write follows. Declining to adopt leaves a row invisible for one boot; it never deletes one. Also: the folder-key guard in both catalog attributions was dead, because `getRepoIdFromWorktreeId` hands back the whole key rather than nothing when there is no `::`. The verdict was right and the resolution wasted; it now skips by shape. And the two type assertions the changed-code casting gate rejected are gone rather than suppressed. * fix(session): park the rows a partition read declines instead of letting the next write erase them A partition write replaces each field with exactly what the unified session routed there. So a row the read left out of that session is erased from its own partition the moment any sibling workspace writes the same one - and with SSH partitions now the owning store, that row is then in no partition at all. Three separate decisions produce such rows: residue the catalog attributes to another host, a contested id withheld so the write cannot carry one host's rows into another's partition, and a workspace the base already holds the live copy of. Declining to show a row was quietly deleting it. The machinery for this already exists. `attachHostSessionShadow` writes a contested runtime co-claimant's parked rows straight back into its own slice before the write, so the primary's write cannot erase them; the ssh partitions simply were not among the slices the contention split arbitrates. The read now parks everything it is not returning to an ssh partition into that same shadow, and the existing re-attach puts it back. Leak, never kill - docs/reference/ssh-execution- boundary.md - and a row no partition holds is unrecoverable. Second, the contested branch of the tab adoption read `Object.hasOwn` as "the base has tabs here". An empty list satisfies it, so whenever a legacy id happened to be contested, stablyai#12721's empty local row won over the host's real one - the exact reading the module's own header, and the gate invariant it is pinned by, say is wrong. An empty row is the gap this repair fills, so it is now treated as one. * test(session): pin the empty-base-row gap for a contested id Mutation testing found the assertion missing: reverting the gate to `Object.hasOwn` left all 39 assertions passing, which makes the fix that reads an empty base tab row as a gap unguarded. The stablyai#12721 shape does not stop being a gap because the id happens to be contested. --------- Co-authored-by: Robert Nisipeanu <github@nisipeanu.com> Co-authored-by: Jinwoo-H <Jinwoo-H@users.noreply.github.com>
…orkspace's own copy (#26098) * fix(session): at startup a local copy never overrides an SSH-owned workspace's own copy Rows the local partition holds for a workspace whose repo the catalog places on an SSH target are residue (pre-#19572 builds, relay reattach). Startup used to keep them whenever they held a tab and skip the SSH partition for that workspace, dropping live tabs, restoring closed ones and erasing agent-resume records on the first save. Boot hydration now drops those local rows (keeping open files and visit recency) so adoption takes the SSH partition whole. * refactor(session): let adoption take a catalog-owned SSH workspace instead of pre-filtering local Replaces the sentinel-partition split with one rule in adoption: the base's terminal tabs no longer keep out the partition the repo catalog places the workspace on (contested ids keep today's behavior). * fix(session): an SSH-owned workspace's records win under shared keys; unsaved local drafts survive Addresses review: a stale local tab sharing an id with the SSH tab kept its layout and resume records (fill-only), and a replaced open-files row dropped local-only unsaved drafts. * fix(session): an SSH copy with no tabs never replaces local tabs Addresses review: an owned workspace the SSH partition holds no tabs for keeps today's rule, so its empty tab row cannot wipe the local tabs. * refactor(session): drop a superseded local copy before adoption; publish path passes owned ids Simplifies the rule: for a workspace the catalog places on this SSH host, uncontested and with host tabs, the base's rows are dropped and the existing gap-fill adoption runs unchanged; unsaved local drafts survive. One resolver says which workspace each scoped entry belongs to, shared by the census, the drop and adoption. The upload path (persistedSessionForTarget) now passes owned ids too, so main cannot publish the stale local copy to the host. * fix(session): match host tab rows by workspace id; a local draft beats a clean host entry Addresses review: a host tab row under a workspace key now counts toward superseding the local copy, and a local unsaved draft for a path the host holds clean is kept instead of dropped. * fix(session): scope catalog-owned ids to the ssh partition the catalog names A workspace homed on one SSH target with leftover rows in another target's partition no longer has the owner's adopted rows removed by the leftover's pass.
…orkspace's own copy (stablyai#26098) * fix(session): at startup a local copy never overrides an SSH-owned workspace's own copy Rows the local partition holds for a workspace whose repo the catalog places on an SSH target are residue (pre-stablyai#19572 builds, relay reattach). Startup used to keep them whenever they held a tab and skip the SSH partition for that workspace, dropping live tabs, restoring closed ones and erasing agent-resume records on the first save. Boot hydration now drops those local rows (keeping open files and visit recency) so adoption takes the SSH partition whole. * refactor(session): let adoption take a catalog-owned SSH workspace instead of pre-filtering local Replaces the sentinel-partition split with one rule in adoption: the base's terminal tabs no longer keep out the partition the repo catalog places the workspace on (contested ids keep today's behavior). * fix(session): an SSH-owned workspace's records win under shared keys; unsaved local drafts survive Addresses review: a stale local tab sharing an id with the SSH tab kept its layout and resume records (fill-only), and a replaced open-files row dropped local-only unsaved drafts. * fix(session): an SSH copy with no tabs never replaces local tabs Addresses review: an owned workspace the SSH partition holds no tabs for keeps today's rule, so its empty tab row cannot wipe the local tabs. * refactor(session): drop a superseded local copy before adoption; publish path passes owned ids Simplifies the rule: for a workspace the catalog places on this SSH host, uncontested and with host tabs, the base's rows are dropped and the existing gap-fill adoption runs unchanged; unsaved local drafts survive. One resolver says which workspace each scoped entry belongs to, shared by the census, the drop and adoption. The upload path (persistedSessionForTarget) now passes owned ids too, so main cannot publish the stale local copy to the host. * fix(session): match host tab rows by workspace id; a local draft beats a clean host entry Addresses review: a host tab row under a workspace key now counts toward superseding the local copy, and a local unsaved draft for a path the host holds clean is kept instead of dropped. * fix(session): scope catalog-owned ids to the ssh partition the catalog names A workspace homed on one SSH target with leftover rows in another target's partition no longer has the owner's adopted rows removed by the leftover's pass. (cherry picked from commit e05c69f)

Supersedes #12722 by @robertnisipeanu (co-authored with @Jinwoo-H), whose diagnosis this carries. The original contributors traced the whole chain — two writers, two partitions, no reader for one of them, and a
replace-sessionupload that converts absence into deletion. That analysis is theirs and it holds; both are credited as co-authors on the commit. Their branch was based on amainfrom before the host-blind ownership fix (#11163), the revision/hostObservationTokenfencing and the contested-id shadow machinery all landed, so much of its 32-file diff is now superseded. What remained load-bearing is carried here.Fixes #12721.
References #18173 — deliberately without an auto-closing keyword. I reproduced the mechanism that issue is attributed to, not that issue's own symptom. #18173 reports an SSH worktree card that is dead on arrival after a restart and whose click is a complete client-side no-op; I did not launch the app, create that card, restart, and watch it recover. The inference that it is the same defect is the reporters' and #12721's, and it is plausible — a card with an empty tab list is what a dead card looks like — but it is an inference, and auto-closing a live data-loss bug on an inference is not a risk worth taking. Someone should verify it against a build of this branch before closing it.
ELI5
Orca remembers which terminal tabs you had open in each workspace. For a workspace on an SSH box, it was writing that memory into two different notebooks, and each half of the app only ever read one of them.
The app window wrote your tabs into the "local" notebook. The background process — the one the
orcaCLI and the pane-retirement paths go through — wrote them into the "ssh:that-box" notebook. Nothing ever read both.That alone would just make some tabs invisible. What made it destructive is the next step. When Orca uploads your workspace to the SSH host, it sends a wholesale replacement, and it built that upload from the local notebook only. So a workspace whose real tab list lived in the other notebook got uploaded as "this workspace has zero tabs". The host believed it. On the next launch Orca pulled that back down and applied the zero, deleting the tabs for real — then uploaded the wiped state again, so every launch from then on repeated the wipe.
The fix is to stop keeping two notebooks. Each host now owns exactly one, both readers use it, and anything an older build left in the wrong notebook is moved over on the next read.
Root cause
src/shared/workspace-session-partition-owner.tstook a second argument that made one function give two different answers for the same workspace:That is one value carrying two meanings, and its own doc comment said the exit was to converge on the host partition. This PR does that and deletes the argument. Both readers then follow the single owner:
fetchWorkspaceSessionWithRuntimeHostOwnersreads the SSH partitions, not justlocal+runtime:*.remoteWorkspace:setForConnectedTargets's no-sessionfallback resolves the session per target fromlocal+ssh:<targetId>, instead ofstore.getWorkspaceSession()(the local blob).Worth stating plainly, because it reframes what this fix is:
mergeDirectSshRemoteWorkspaceSessionalready refused to delete tabs the host had never been told about. That defence only covers tabs the pulling client actually holds — and pre-fix, for exactly these workspaces, it held none, because its local row was the empty one. So the empty remote won by default. Hydrating the SSH partition is what restores the precondition that existing safety mechanism depended on. This is not a new reader bolted on top of a broken merge; it is the merge being given the state it was always written to defend.What carried over from #12722, and what did not
Carried: the diagnosis, and the two facts it rests on — boot hydration filters
parseExecutionHostId(...)?.kind === 'runtime', and the export fallback reads the local partition alone.Dropped: the five new
workspace-session-partition-*modules (~900 lines of pane-level authority resolution, pty-bound vetoes, tombstone reconciliation and retirement recomputation). They exist to reconcile two partitions that both stay authoritative forever. Converging ownership means that reconciliation is a one-shot, gap-filling read instead, which is 130 lines and needs no authority model:The walk switches exhaustively over
WORKSPACE_SESSION_FIELD_OWNERSHIPrather than listing fields, so a new ownership kind cannot silently fall out of the read. That is not incidental — see below.An empty tab row counts as a gap, not as an answer — the point the whole bug turns on, and the same rule
mergeDirectSshRemoteWorkspaceSessionalready applies anddocs/reference/ssh-execution-boundary.mdgeneralises: absence isunverifiable, never proof.Also dropped as superseded by current
main: thessh-provider-authorityfencing on notifications and patches,getRepo(id, hostId)host-scoping, and thesessionTargetId/sessionAuthorityRPC params. Their branch also moved SSH state intolocalvia a two-phaseflushOrThrowwith rollback in theStore— the opposite direction from the one the shared module argues for, and it leaves the runtime still writingssh:<targetId>, so the divergence regenerates and the adoption has to re-run forever.A regression this review caught, and what it changed
The first version of this PR gated adoption on
host.tabsByWorktree[key].length > 0— because tabs were the field I was thinking about. But the write side routes every worktree-scoped field to the owning partition. So an SSH workspace with open editor files or browser tabs and zero terminal tabs — an ordinary state, since Orca will not auto-create a terminal while other tabs exist — had all of it dropped on every restart. Worse than the bug being fixed:RemoteWorkspaceSessioncarries terminal fields only, so the SSH host snapshot cannot recover editor state, and an unsaveddirtyDraftContenthot-exit draft is user-authored content that would have been destroyed outright. Onmainthis works, becausemainroutes SSH workspaces tolocal, which is merged.That is a fix trading one data loss for a narrower one on the same topology, which is exactly what "root cause, not symptom" is meant to prevent. Credit to the review for catching it; I reproduced it independently before changing anything.
The correction is not "also handle editor files". The defect was that a hand-maintained field list decided what the read recovers while the write used the ownership table, so the two could disagree. Adoption now derives its field handling from
WORKSPACE_SESSION_FIELD_OWNERSHIPwith an exhaustiveswitch, so every ownership kind is handled explicitly and a newly added one is a compile-time decision rather than a silent omission. Two regression tests pin it where it actually broke: the shipping split (buildWorkspaceSessionHostSnapshots) feeding its own output back through the real boot read, rather than a hand-built fixture that asserts the shape the read already expected.A second, smaller finding from the same review:
lastVisitedAtByWorktreeIdis keyedssh:target|worktreeId, but adoption keyed by bare workspace id, so every SSH workspace lost its Cmd+J recency across restart. Session keys are now normalized through the sharednormalizeWorkspaceSessionKeyToWorkspaceId— moved toshared/for this, rather than reimplemented, so the two callers cannot drift.Known: one-shot resurrection in the legacy-transition shape
Where an older build left
local = {WT: []}while the runtime partition still holds that workspace's tabs, boot adopts those tabs back once. That is the repair working as intended, but it means a tab closed on an old build immediately before upgrading can reappear once. It is non-destructive, and it does not recur: a workspace this build empties deliberately leavesssh = {WT: []}in the owning partition, which is not adoptable. Recorded in the gate'sknownGapsrather than left implicit.Proof of fix
Both new test files fail on unmodified
origin/mainand pass here. Re-verified against current main (12f53da542d) after merging it in, not against the base I started from — 9 of the 11 new assertions fail there.Fixture is the shape reported in #12721: the local blob carries the worktree key with an empty list while
ssh:<targetId>holds the real one. The main-side test drives the realStoreand the real IPC handler, not agetWorkspaceSessionfake — the defect is which partition the read reaches, and a fake answers whatever the test tells it to.Against pristine
main:The two that pass on main are the guard assertions —
leaves a workspace the local partition already holds tabs for untouchedandleaves that workspace's other rows alone as well. They should pass there: main never adopts, so it trivially leaves a populated row alone. They exist to catch the fix over-reaching, and the "host partition overwrites a workspace the base already has tabs for" mutation is what proves they are not vacuous.The round-trip case runs the real projection and the real pull-side merge, so it captures the deletion itself rather than only the missing read. It is not a transient window: republishing does not mask it, because the renderer never had the rows to republish.
On this branch: 11 passed, and
src/renderer src/shared src/main src/preloadare green (the one failure insrc/main/runtime/orchestration/db-stopping-worker-task-guard.test.tsreproduces identically on pristinemainand is unrelated).Mutation testing
Each assertion was checked against a mutation of the specific regression it claims to catch, not against reverting the change.
localblob againpickPrimaryHostForClaimsfalls back to plain sort orderlocalagainOne mutation survived, and I want to be explicit that it is inert, not uncovered: making the fallback fold in every SSH partition instead of the target's own changes no observable behaviour, because
exportRemoteWorkspaceSession's ownership filter already prevents cross-target publication. I had written an assertion for that property; since no mutation of this change can fail it, I removed it rather than keep a passenger, and kept the second populated SSH partition in the fixture — which is what makes "reads a different target's partition" catchable at all.One earlier mutation of mine was unfaithful and I want that on the record too: my first attempt at the tabs-only gate still discovered the workspace through an empty
tabsByWorktreekey, so it passed and looked like coverage. The real gate waslength > 0. Rewriting the mutation to match the original defect turned it red immediately — a mutation that survives is a claim to check, not a result to file.Reliability gate
Registered as
workspace-session.ssh-host-partition-round-tripinconfig/reliability-gates.jsonc(experimental/partial), since a permanent, self-sustaining tab deletion is the kind of invariant that manifest exists to pin. ItsknownGapsstate plainly what it does not cover: no live SSH host or relay (the multiplexer is faked at the request boundary), folder workspaces, and tabs stranded beside an already-populated local row.Cross-cutting
Wire compatibility — Rule 3, both directions
docs/reference/remote-wire-compatibility.md. No frame, no opcode, no field:RemoteWorkspaceSessionis untouched and theworkspace.patchpayload is the same{ kind: 'replace-session', session }. But Rule 3 applies anyway, because the content a client publishes to~/.orca/sessions/<ns>.jsonchanges, and a second client paired to the same host reads it. Mixed versions are the normal state, so both directions are stated here rather than assumed. Note the partitions themselves are client-local — the SSH host stores only the projected snapshot — so the skew is entirely about published content.Old client ← new host snapshot. A worktree that used to be published with an empty tab list is now published with its real one. Purely additive: nothing stops being published, no field changes meaning or nullability, and an old client runs the same
importRemoteWorkspaceSessionover the same shape, because no field was added for it to not understand. It gains the tabs through the normal merge — which is the repair, not a hazard. The other maps that grow the same way (terminalLayoutsByTabId,remoteSessionIdsByTabId,activeTabIdByWorktreePath,lastVisitedAtByWorktreePath,defaultTerminalTabsAppliedByWorktreePath) are all keyed by ids the old client already handles;defaultTerminalTabsApplied…in particular now correctly suppresses re-applying the default terminal template over a workspace that demonstrably had tabs. No capability gate is needed because there is no behaviour an old client must opt into — only entries it already knows how to read.New client ← old client's publish. This is the direction that could have been the same data-loss shape one hop away, so it is pinned by a test rather than argued. A client that predates this fix still reads only the local partition, so its
replace-sessionnames the workspace's path with no tabs — the very upload this PR stops producing.mergeDirectSshRemoteWorkspaceSessionalready refuses to delete what the host has never been told about (itshostUnknownbranch), but that defence only covers tabs the pulling client actually holds. Pre-fix it held none for this workspace — its local row was the empty one — so the empty remote won by default and the deletion stuck. Hydrating the SSH partition is precisely what arms that existing defence, and this client then republishes the real list and repairs the poisoned snapshot for every client on that host.keeps the tabs when an older client publishes an empty list for themasserts exactly that, running the real importer and the real merge. It fails on pristinemainfor the same reason the bug exists, and it is caught by the "hydration stops fetching SSH partitions" mutation.So neither direction can present an empty tab list as authoritative: the new client no longer publishes one, and it no longer believes one.
Folder workspaces. Deliberately left on
local. Boot discovers SSH partitions from the repo catalog, so converging a folder workspace's writes ontossh:<targetId>would strand it whenever that target owns no repo — trading this bug for a worse one. They still benefit from the read side: a stranded folder workspace is adopted back out of the SSH partition whenever a repo also names that host, and the gap-fill rule means a populated local row is never re-merged from the stale partition. The residual case is an SSH target that owns no repo at all. Converging it needs a partition census the renderer can consult at boot — main already knows the answer inStore.getWorkspaceSessionHostIds(), it is just not exposed. Scoped out explicitly and recorded on #12723, which tracks the writer-side divergence and stays open for it.Rollback. No schema change and no migration, so a downgrade still parses everything, and nothing is erased from disk — the
ssh:<targetId>partition survives and re-upgrading adopts it back.But the "return to the prior bug, not new loss" framing is only true for terminal state, and it is worth being exact about the difference. For terminal tabs, a downgraded build sees what it always saw and the host snapshot can re-supply them. For editor and browser state written by this build into the SSH partition, a downgraded build reads neither the partition nor a snapshot that carries those fields, so that state is invisible for as long as the user stays downgraded — recoverable by re-upgrading, but not by anything the old build can do. A user who downgrades, works, and never returns keeps whatever the old build wrote to
localinstead.Platforms. No paths, processes or shells;
toSshExecutionHostIdencodes identically everywhere. The paired web client already stores non-local partitions under a host-suffixed key and needed no change.Side effect worth noting.
sessionPartitionHostFornow gives every non-localhost its own partition, which closes one of the two documented gaps inworkspace-session-host-contention.ts:localandssh:*were previously inseparable. That exposed a latent bug inpickPrimaryHostForClaims— oncelocalis not a claimant it sorted alphabetically, so a workspace claimed by both an SSH and a runtime host would have had its SSH rows written into the runtime partition. Fixed by ranking rotatingruntime:ids last; both existing tests for that hazard are kept and tightened.A note on verifying this
Two measurement traps hit while validating this branch, both worth recording because they make a red run look green:
[ELIFECYCLE] Test failedwith 14 failed files. Every other misreport shape distorts a reported status; this one inverts the process exit code, so any automation trusting$?is silently wrong. It was only caught by reading the log. Read job steps and log tails, not conclusions.Verification
pnpm tc·pnpm test src/renderer src/shared src/main src/preload·pnpm run check:code-quality:changed(0 new findings across 16 files) ·oxlint(0 errors) ·pnpm run check:max-lines-ratchet·pnpm run check:reliability-gates·pnpm format. Nomax-linessuppression added —remote-workspace.tscrossed the cap, so the target-export cluster moved to its own module.Second review pass: three losses inside the loss fix
Adoption was called with strictly less information than its caller holds, and its walk described the partition layout a second time rather than reusing the writer's. Both showed up as data loss, all three reproduced before being changed.
1. An empty host row overwrote a populated base row (P1)
Adoption's empty-row semantics are asymmetric on purpose: on the base side
tabs.length > 0makes an empty row a gap, on the host sideObject.keys()makes an empty host row name the workspace. Combined with the replacing write, a workspace whose base copy held only editor files was both adoptable and clobberable — and the row that clobbered it was empty.That is the same shape as the P1 this PR already fixed, reappearing on the write side rather than the discovery side: the earlier fix taught adoption to find those workspaces, and this let an empty host row overwrite them once found.
RemoteWorkspaceSessioncarries terminal fields only, so nothing recovers the draft.The host holding nothing is never evidence the base is wrong — the same reading the base side already gives an empty tab row, and the one
docs/reference/ssh-execution-boundary.mdgeneralises. An empty host value now only fills a gap. The symmetric cost is that a row the host really did empty stays visible for one more launch; a resurrected editor tab is non-destructive, a destroyed draft is not.2. A contested bare id was adopted as if it named one workspace
Every rule in the module rests on
localandssh:<targetId>being one workspace written twice. A barerepoId::pathclaimed by more than one host (STA-4343) is exactly where that premise is false — and the contention split cannot see it, because ssh slices are deliberately kept out of the claimant set.So the read decides
localis primary for a contested id, parks the co-claimant in the shadow so the primary's write cannot erase it, and then adoption overwrites the primary's own rows with the SSH workspace's. Routing honourscontestedPrimaryHostBySessionKey, so those rows are written into local's own partition and the originals are gone from both the session and the store.The root cause is one sentence: adoption's signature is
(base, host)while its caller holdsmerged.primaryHostBySessionKeyandmerged.shadowin the same function. The verdict is now passed in, and a contested key is gap-filled, never replaced.One trap worth recording:
primaryHostBySessionKeyis not the contested set — it names a primary for every key including uncontested single-partition ones, so using it as the contested set silently disables adoption entirely.mergeWorkspaceSessionsWithHostShadownow reportscontestedSessionKeys(owners.length > 1), which is the thing that was actually wanted.3. The reader walked its own description of the layout
adoptedTabIdscame fromhost.tabsByWorktreealone, while the split'sbuildWorktreeIdByTabIdalso walksunifiedTabs, with a comment saying unified-only tabs need their layout and PTY records routed. SoterminalLayoutsByTabId,remoteSessionIdsByTabIdandterminalPtyIncarnationsByPaneKeyfor a unified-only tab were routed into the ssh partition and never adopted back. Writer walked two sources, reader walked one.split(':', 1)in adoption andlastIndexOf(':')in the split.lastVisitedAtByWorktreeIdis classifiedworktreeKeyedbut keys on<hostId>|<worktreeId>unlike its thirteen siblings. The split has a dedicated branch for it; adoption had none and normalised the host prefix away, so a legacy bare key in an SSH partition replaced the local workspace's key of the same id and moved its Cmd+J position permanently.The fix is the shared description rather than three patches:
buildWorktreeIdByTabId/buildWorktreeIdByFileId/worktreeIdForPaneKeymoved tosrc/shared/workspace-session-host-records.tsand both sides now resolve through them. A bare recency key only fills a gap; a host-qualified one still replaces.The durable finding, worth stating plainly: the exhaustive switch guarantees both sides enumerate the same kinds. Nothing guaranteed they agreed on what a kind means — a special-cased field, a two-source tab discovery, and an asymmetric empty-row reading all slipped through it. That is the limit of the compile-time check, and it is why the routing indexes are now shared code rather than two parallel walks.
The shared-semantics boundary with #18513
#18513 writes an explicit empty
tabsByWorktreerow to mean the user closed the last terminal and reserves an absent row for never initialized. This PR's adoption reads an empty base row as a gap to fill. Same value, opposite readings, so the boundary is asserted rather than reasoned about — a mature product ships exactly this defect, with a correct write side and one reader that decides seeding on a count and never consults the record.Six new assertions in
workspace-session-ssh-partition-round-trip.test.tspin it through the shipping write path and the real seeding predicate:ssh:<targetId>and leaves no row inlocal— the precondition the whole non-recurrence claim rests on;shouldAutoCreateInitialTerminaldeclines it — asserted through the real predicate, because the row being right is worth nothing if the reader that acts on it disagrees;ssh:row does not win over a tombstone in the owning partition;knownGaps, now driven: boot 1 adopts the stranded tabs back, the user empties the workspace on this build, and boot 2 holds the tombstone because there is nothing left to resurrect from;Both merge orders were built and run. Because the two PRs share no file,
main + #19572 + #18513andmain + #18513 + #19572produce a byte-identical tree (c02d18c18386f51689b18763290ce1f0b4e2e521), and the combined suites pass identically on both — 56 tests.Rule 3, re-confirmed on the new shape
Partitions are client-local; the SSH host stores only the projected snapshot, so the surface is client-publishes → other-client-reads, not host→client. Neither direction can present an empty tab list as authoritative:
publishes the stranded tabs rather than an empty list), and a workspace the user emptied publishes[], which is the state it observed rather than a gap;keeps the tabs when an older client publishes an empty list for themruns the real importer and the real merge; hydrating the SSH partition is what armsmergeDirectSshRemoteWorkspaceSession'shostUnknowndefence, which pre-fix had nothing to defend with.No frame, no opcode, no field:
RemoteWorkspaceSessionis untouched and the payload is the same{ kind: 'replace-session', session }. The moved module is client-internal.Mutation testing, second pass
unifiedTabsEach of the first four breaks exactly its own assertion, which is what says they are not covering for each other.
One mutation survived and the test was weak, not the mutation inert — recorded because that distinction is the whole point: reverting the branch-split's respawn arm in the sibling #18513 change broke nothing until an assertion was added for it. Details in that PR.
Findings filed rather than fixed
tabsByWorktreerow into an empty one, plus a genuine design tension inRuntimeWorkspaceSessionController.resolveHostId. That one looks like a mechanical idiom swap (?.length ?? 0 > 0→!== undefined, the idiompartitionOwnsWorktreeTabsalready uses on the same data) and is not: it breaks two existing tests whose fixture explicitly says "a prior close can leave an empty retained row in the stale partition". Both behaviours are pinned today and are structurally indistinguishable inside that function. Filed rather than guessed.Residual, recorded in
knownGapsworkspace-session-host-contention.tsalready records.lastVisitedAtByWorktreeIdcan still discover a workspace as adoptable from a recency entry alone. Harmless now that a recency row cannot replace another host's entry for the same bare id, but it is the one field of its kind that is a discovery trigger.buildRuntimeHostIdByWorkspaceSessionKeyis computed from the pre-adoption slices. Checked and not a defect: it maps session keys to runtime hosts, and a workspace adoption adds from an ssh partition has no runtime owner by construction — a runtime slice naming it would already have put it in the map.Verification, this pass
pnpm tc·pnpm test src/renderer src/shared src/main src/preload(2 failures, both unrelated:ssh-posix-command-wrappersurvives the real /bin/zsh parser reproduces on pristinemain, andstructured-agent-session-lease-renewerpasses in isolation and only fails under full-suite contention) ·oxlint0 ·pnpm run check:code-quality:changed0 new findings across 18 files ·pnpm run check:max-lines-ratchet·pnpm run check:reliability-gates·pnpm format. Nomax-linessuppression added.The only auto-closing keyword in this body is the one in the header above, naming #12721. #18173, #12723, #19899 and every STA reference are deliberately plain references — #18173 in particular, because the mechanism was reproduced and that issue's own symptom was not.
Verified against a real SSH host
e2e / changed e2e specswas red on this branch — onssh-cold-activation-restore.spec.ts, i.e. exactly the surface this PR changes, which is the one place a red e2e must not be waved through. It was already red on the previous head, but that only rules out the latest commits, not the PR.Reproduced locally against a real Docker OpenSSH target, so the diagnosis is measured rather than inferred. The spec asserted the partition layout, not the invariant:
Moving those rows to
ssh:<targetId>is the fix, solocalis empty there by design and the check fails on a correct build. It now unionslocalandssh:<targetId>and is correct on both layouts.Editing a test until it passes is exactly what a reviewer should distrust, so the product invariant was confirmed separately rather than by the edit. That white-box check sits in the middle of each test; everything after it is behavioural and layout-independent — the full app restart, active-worktree resolution, the eager remount of all six restored remote terminals, and the authenticated PTY-owner reclaim verified by a marker file written inside the container. If SSH state genuinely were not persisted, those would fail regardless of which partition the assertion read.
The
e2e / ssh docker watcher isolationjob had four failures, and all four are the same cause in two specs.ssh-restart-tab-accumulation.spec.tshad the identical layout-coupled read in its pre-quit flush, and it is the more pointed test of the two — it drives repeated quit/relaunch cycles and asserts the workspace keeps exactly its restored SSH tabs, which is the accumulate-or-delete symptom class this PR exists for. Also fixed, also verified on a real host:Every other e2e reader of
window.api.session.get()was checked and left alone:ssh-cold-hydration-gap-tab-seedingreads onlyactiveConnectionIdsAtShutdown, which is a global and stays inlocal;ssh-lost-kill-tab-resurrection's use is a diagnostic dump, not an assertion; and the remaining four specs are local, WSL or runtime-owned and never touch anssh:partition.This is the first real-host evidence on this branch, and it is the closest thing yet to the check #18173 needs. It still does not close it: #18173 reports a dead SSH worktree card whose click is a client-side no-op, and this exercises terminal remount and PTY ownership rather than that card interaction. The reference stays deliberately non-closing.
A measurement trap worth recording, because it is the one the PR body already warns about and it fired again here: the failing Playwright run printed
2 failedand then[exited with code 0]. Any automation trusting$?would have read this whole investigation as green.CI, read from job steps
Final run on
4b0d984: every check green, 21 non-skipped.typecheck,static analysis,package,package (windows), all 8test / tests node 24shards,pullfrog,verify,test vs non-test LoC,root directory guard, and both e2e jobs —e2e / changed e2e specsande2e / ssh docker watcher isolation, the two that were red.Confirmed from the job log that the e2e job ran the SSH specs rather than skipping them, since a job that skips the dimension it exists to check reads as green:
ssh-reconnect-tab-destructionis in that list and was not touched — it exercises the same partition on a real host and passed unmodified, which is the check on whether the two spec edits were fixes or cosmetics. Skips (cross-version wire compatibility,Git compatibility,real WSL terminal,shell contracts, …) are path-filtered on dimensions this PR does not touch. Worth naming one:cross-version wire compatibilityis skipped and correctly so — no wire schema changed — which is precisely why the Rule 3 risk here is published content rather than wire shape, and why it is pinned bykeeps the tabs when an older client publishes an empty list for theminstead of by that job.Rebase record (2026-09-11)
Rebased onto
mainat5fa62feda7c264c6eda99ba4788ffebab9aae26c.4b0d9841934c5796edcd82926886901afcf53071f6cad61f7605e19c33a45cf656e414706ef22630The two
Merge remote-tracking branch origin/maincommits the branch carried were flattened by the rebase, so the branch is now a linear 8 commits onmain.One conflict, in
config/reliability-gates.jsonc:main(#20003) and this branch each prepend a new entry to thegatesarray at the same position. Resolved as a union — bothruntime.connection-owned-host-statusandworkspace-session.ssh-host-partition-round-tripare present, with no entry dropped or reordered.Gates on the rebased head
node config/scripts/check-reliability-gates.mjs— exit 0,Reliability gate manifest check passed for 122 gate(s).vitest run --config config/vitest.config.ts src/main/ipc/ssh-host-partition-session-export.test.ts src/renderer/src/lib/workspace-session-ssh-partition-round-trip.test.ts src/renderer/src/lib/workspace-session-host-contention.test.ts src/shared/workspace-session-partition-owner.test.ts(the four suites this PR s own gate entry names) — 4 files / 56 tests passed, log grepped forFAIL(none).Rebased onto
main@231e805b1e1e49c765e96b9409f88e09d117346a(2026-09-15).f6cad61f7605e19c33a45cf656e414706ef226306d5cee1f4d8dacac00188a9fd5410853d5c5bef7Content unchanged apart from one added commit,
fix(lint): clear the casting gate on the partition adoption, whichmain's newly tightenedtypescript/consistent-type-assertionsrequires. Verified withgit range-diff; prior CI results predate this rebase.