Repository navigation
fix(session): at startup, a local copy never overrides an SSH-owned workspace's own copy - #26098
Conversation
…rkspace'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.
|
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:
📝 WalkthroughWalkthroughStartup hydration now passes catalog-resolved SSH workspace IDs to stranded-partition adoption. Adoption can select SSH state for those workspaces when local rows also exist, while rejecting empty host rows and retaining unsaved drafts missing from adopted open-file rows. Unit tests cover conflicting state and unresolved ownership. A Docker-backed restart test checks restored tabs and persisted partition state. The E2E configuration routes that test to the dedicated Docker SSH lane. Priority: ➖ Normal Merge Risk: 🟡 Moderate · up to Startup now prefers the SSH workspace's own session over a stale local copy. Some edge cases remain. The SSH tabs can stay hidden when the workspace key is stored in canonical form. An unsaved local edit can be lost when the SSH copy has the same file open without changes. Resolve these before merging. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
…stead 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).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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:
0533df69-6a3b-4ed1-82a5-a312716881f3
📒 Files selected for processing (3)
src/renderer/src/lib/workspace-session-host-hydration.tssrc/renderer/src/lib/workspace-session-ssh-partition-round-trip.test.tssrc/shared/workspace-session-stranded-partition-adoption.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.
… 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.
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:
8babc4eb-0455-43cf-bae7-b4308d9d03d5
📒 Files selected for processing (2)
src/renderer/src/lib/workspace-session-ssh-partition-round-trip.test.tssrc/shared/workspace-session-stranded-partition-adoption.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 3 remain after this review.
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.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Trigger the dedicated Docker-SSH job for… · ci-e2e-job-selection.mjs:27-28
config/scripts/ci-e2e-job-selection.mjs:27-28
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTrigger the dedicated Docker-SSH job for
ssh-startup-local-shadow.spec.ts.
selectPrE2eSpecsincludes the changed spec, butselectGeneralE2eSpecsremoves it because it is Docker-SSH-only. The changed workspace-session files do not match either SSH source route, sossh_source_changedisfalse. The dedicated job condition does not include this spec. Therefore, this PR has no CI job that invokes the runner for the new regression.Suggested fix
contains(inputs.test_files, 'tests/e2e/ssh-stale-resume-execution-host-scope.spec.ts') || + contains(inputs.test_files, 'tests/e2e/ssh-startup-local-shadow.spec.ts') || contains(inputs.test_files, 'tests/e2e/ssh-terminal-window-wake-stale-grid-repro.spec.ts') ||
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
d0003feb-0324-4387-b3ba-56cd2dd3c903
📒 Files selected for processing (2)
src/renderer/src/lib/workspace-session-ssh-partition-round-trip.test.tssrc/shared/workspace-session-stranded-partition-adoption.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/renderer/src/lib/workspace-session-ssh-partition-round-trip.test.ts
- src/shared/workspace-session-stranded-partition-adoption.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.
…ish 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.
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
a00de8eb-052d-4604-b088-6baf077fb038
📒 Files selected for processing (3)
src/main/ipc/remote-workspace-target-session-export.test.tssrc/main/ipc/remote-workspace-target-session-export.tssrc/shared/workspace-session-stranded-partition-adoption.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.
…s 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.
…g 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.
…space, as before the core #26098 (on main) relies on the window's first save erasing a stray local copy of a workspace the repo catalog places on an SSH target. Window saves now keep main's rows, so that copy stayed. In an owner partition main keeps only the rows of workspaces homed there (or of unknown home); residue homed elsewhere is the window's to keep or drop, as before. Fixes ssh-startup-local-shadow.
…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)
1 participant
ELI5
Orca can hold two saved copies of an SSH workspace: an old one in the "local" bucket and the current one in that SSH host's own bucket. At startup the old copy won. Now, for a workspace whose repo lives on an SSH host, the SSH host's copy wins.
What Changed
Before: if the local bucket still held any terminal tab for an SSH workspace, startup used that stale copy and skipped the SSH bucket for the workspace. Users saw tabs they closed long ago come back as empty shells and lost live agent tabs. The SSH bucket's agent-resume records were left out of the restored session, so the first save deleted them from disk, and after a reboot those agents were no longer offered for resume.
After: the SSH bucket's tabs, active tab, layouts and agent-resume records are restored as saved. The next save writes the workspace to its SSH bucket, so the stray local tabs disappear from disk.
Mechanism: startup already folds each SSH bucket into the local session (
adoptStrandedHostPartitionSession), filling gaps and skipping any workspace the local session holds terminal tabs for. Startup also already asks the repo list who owns each workspace an SSH bucket names. That check now also returns the workspaces it resolves to that bucket. Before the fold, the local rows for such a workspace are dropped, as long as the id is not claimed by another host and the SSH bucket holds tabs for it. The existing fold then runs unchanged and takes the workspace whole from the SSH bucket: tabs, layouts, agent-resume records, open files, browser tabs, even when a stale local tab shares an id with the live one.persistedSessionForTarget), which publishes the session to the SSH host when the renderer sent none. Without it main could still upload the stale local copy to the host.Unchanged: folder workspaces, repos the repo list cannot name yet, ids two hosts both claim, and workspaces the SSH bucket does not mention keep today's rule.
Why
The skip assumed the local copy was the live one. After #19572 the opposite is true for SSH workspaces: the renderer and the runtime both write an SSH workspace to its SSH bucket, so a local row for it is residue, either from builds before #19572 or written by relay reattach (#25616; #26088 stops that writer). Fixing only the writer leaves existing residue winning on every boot, so the read has to change.
Changing the one existing skip, rather than filtering the local copy out before the fold, keeps a single rule and reuses the fold's field handling (including its draft protection) instead of a second code path. Alternatives considered:
The open community PR #23394 targets the same rule with a broader carry-over approach.
Linked Issue
Part of #23390 — fixes the boot-time choice between the two copies (live tabs dropped, closed tabs restored). It does not change the separate request there to re-surface detached relay leases.
Part of #25616 — existing stray local rows no longer shadow the SSH bucket. #26088 stops new ones being written.
Visual Proof
N/A — this changes which saved copy startup restores, with no new UI. The Docker SSH e2e below is the before/after: on main the agent-resume record is gone from the SSH bucket after a restart; with this change it survives and the stray local tabs are cleaned up.
Testing
I manually tested these changes locally
Automated tests added/updated, or explained why not below
New e2e
tests/e2e/ssh-startup-local-shadow.spec.ts(Docker SSH lane), run on macOS: connect to a real SSH target, open two terminals and quit. Then seed a stray local copy (one live tab under an old title, plus a long-closed tab) and an agent-resume record in the SSH bucket, and relaunch. Fails on main (resume record erased from the SSH bucket after the first save). Passes with this change (SSH tabs restored, resume record kept, stray local tabs gone from disk after a save).Unit (
workspace-session-ssh-partition-round-trip.test.ts): the SSH bucket's tabs, active tab and resume records win over a stray local copy; a stale local tab sharing the SSH tab's id does not keep its layout; a stray local copy's unsaved draft is kept beside, or over a clean copy of, the SSH bucket's own open files; tabs the SSH bucket keys by workspace key still supersede the local copy (all fail without the fix). Ownership suite: a leftover copy in a second SSH bucket does not remove the owner's rows, in both read orders; an SSH bucket with no tabs keeps local tabs; a local copy whose repo the repo list cannot name keeps today's behavior. Existing draft-protection and contested-id tests pass.Unit (
remote-workspace-target-session-export.test.ts): main's upload fallback publishes the SSH bucket's tabs, not a stray local copy (fails without the owned-ids input).pnpm run test:e2e:terminal-layout-parity -- --base origin/main --head HEAD: PASS (9 scenarios, no differences). That suite has no SSH scenario yet, so it shows only that local layouts are unchanged.pnpm tc,pnpm run check:code-quality:changed, targeted vitest (workspace-session, adoption,remote-workspace: 853 tests).Review
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)