Repository navigation
feat(wsl-mover): the ledgered desktop rewrite stage that finishes a committed move (D1b-7b) - #29597
Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ➖ Normal Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 51.72% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 29 functions across 23 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly identifies the main change: adding the ledgered desktop rewrite stage for committed WSL moves. It is concise and specific. Description check The description covers the required sections, explains the change and rationale, documents testing, provides the linked-issue exception for a maintainer, and marks visual proof as not applicable. The … Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description covers the required sections, explains the change and rationale, documents testing, provides the linked-issue exception for a maintainer, and marks visual proof as not applicable. The AI Disclosure and Review sections are empty, but they are not critical omissions for this maintainer PR.
-
✨ Finishing Touches: 💡 2
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
…ommitted move (D1b-7b)
… registry completeness, stores time out (D1b-7b)
dbc0564 to
0c65c48
Compare
ELI5
After a WSL project's move commits, the desktop still has to update some of its own saved state, for example "last opened worktree" and sidebar host filters, so it points at the WSL host's copy. That update spans several separate stores, and the app could crash halfway through. This PR adds the bookkeeping: a ledger of which stores are done, a startup pass that finishes a move that was interrupted, and two IPC calls for stores that live in the window. It registers no actual stores yet; those come in later PRs. Everything sits behind the internal WSL host gate, so nothing changes for anyone.
What Changed
This is step D1b-7b of the lossless WSL project mover.
wsl-move-cutover-journal.ts):rewrittenStoresonly grows (append-only, deduplicated), and only while the move is atdestination-committed.client-rewrittenuntil every registered store is recorded and the store registry is declared complete.WSL_MOVE_DESKTOP_REWRITE_REGISTRY_COMPLETEisfalseuntil the last store PR (7f) flips it, so a move can't skip stores that land later.wsl-move-desktop-rewrite-stores.ts):{id, side: 'main' | 'renderer', run, flush}. Registration order is run order for main stores. Renderer stores are independent of that order.wsl-move-desktop-rewrite.ts):wslMove:listPendingRendererRewritesandwslMove:recordStoreRewritten. Recording refuses unknown, main-side, already-recorded or not-pending stores. A preload bridge, a web stub (empty) and a typed client with no callers yet are included.workspaceSessionsByHostIdis now rewrite-desktop (seeded later). A new test pins item 52 to not-applicable only; items 19, 32 and 38 legitimately mix moved and not-applicable fields.Why
The rewrite spans stores that flush at different times, including localStorage in the window. A crash can land between any two of them. Recording each store only after its own durable flush lets the next start redo just the missing ones, with no store applied twice.
Merge policy
This PR qualifies to merge under the owner's policy. With
ORCA_INTERNAL_WSL_HOSTSoff, none of it is reachable: the startup pass returns before any file work, and both IPCs answer empty orgated-offbefore touching disk. It touches no shared user paths (SSH, terminal launch, runtime lists, sidebar, session persistence). Outside the mover's own modules it only adds the IPC registration line, the preload key and the web stub.Linked Issue
N/A (maintainer; tracked in the WSL-via-orcad implementation tracker, step D1b-7).
Visual Proof
N/A: no UI.
Testing
I manually tested these changes locally
Automated tests added/updated, or explained why not below
Ledger: the ledger only grows; the advance is refused until every store is recorded and the registry is complete; a new record with entries is refused.
Driver:
Renderer IPC: checks for an unknown store, an already-recorded store, a main-side store and a pending move, plus the gate-off answers.
Startup: the pass is gated.
Dispositions: coverage holds, and item 52 is not-applicable only.
pnpm tc:node,pnpm tc:web, both oxlint configs, the focus-read and layout-writer ratchets, and the changed-code quality gate pass.Adversarial review: no HIGH findings, and it confirmed the merge-policy claim. One MEDIUM (half of decision C applied to the chat-journal tables) and five LOWs are fixed here.
AI Disclosure
Review
Agent skill upstream boundary
Notes
The typed renderer client has no caller until 7d registers the first renderer store. The dead-code scan isn't run in CI, and main already has 36 unused files and 137 unused exports.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)