Skip to content

fix(wsl-mover): no move of a distro may start while its move is being undone - #35369

Merged
nwparker merged 1 commit into
mainfrom
nwparker/wsl-move-undo-blocks-claims
Oct 28, 2026
Merged

nwparker merged 1 commit into
mainfrom
nwparker/wsl-move-undo-blocks-claims

Conversation

@nwparker

@nwparker nwparker commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 3 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​168 $\color{#cf222e}{\Huge{\mathbf{−}}}$​5 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​163
Prod 2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​31 $\color{#cf222e}{\Huge{\mathbf{−}}}$​15 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​16

ELI5

Bringing a moved project back to this computer ("Use on this computer again") reconnects to the WSL host, and that wakes Orca's automatic mover. The mover could then start moving that same distro while the project was still on its way back. The undo would fail after it had already stopped the project's terminals on the host. Now no move of a distro can start while that distro's move is being undone.

What Changed

Before:

  1. The undo connects to the WSL host. After a relaunch, or whenever the host wasn't connected, that makes the host ready.
  2. The automatic mover runs for that distro (its "connect" trigger). Because the distro's move had finished, the mover may preview and start a new move straight away. Opening a project of the distro mid-undo, or the mover's re-check timer, does the same.
  3. The undo then stops the host's copies of the project's terminals.
  4. The undo finds a move running ("move running"), or a newer move committed (superseded), and refuses. The host's terminals stay stopped and the project stays moved.

After:

  • While a distro's move is being undone, no move or automatic preview of that distro can start. The mover gets "already running", which it treats as try-later, so the undo completes. Afterwards the mover parks the distro as declined, as before.
  • The undo also re-checks, right before it stops the host's terminals, whether it may still go ahead. If anything changed since its first check, it refuses with the host's terminals still running.

Mechanism (wsl-move-running-moves.ts, wsl-move-release-here.ts):

Why

  • Why refuse the claim, not exempt previews. The orchestrator's preview claims the run while it settles an interrupted move. Left unblocked, that claim alone would make the undo refuse "move running" after the stop.
  • Why the mark comes before the guards. The undo's own connectUndoHost is what wakes the orchestrator. Taking the mark first means no run can claim after the undo's first check.
  • Alternatives considered:
    • Making the undo's guards ignore moves would let a newer move supersede it mid-flight.
    • Pausing the orchestrator globally would affect other distros.

Known gap (follow-up): a refusal that comes after stopHostCopies still leaves the host's terminals stopped. This PR makes the refusals this race caused impossible, and the late re-check catches anything else before the stop. What's left is a failure after the stop: a journal write error in releaseWslMove, or, in R38 Restore, an older move in the chain refused after a newer one stopped and released. Restarting the host's copies on that path needs a "start host copies" port that doesn't exist yet.

Flag-off: moves and undo exist only behind the WSL hosts gate (undo needs wslHostsServing(), the orchestrator needs the gate open). With ORCA_INTERNAL_WSL_HOSTS unset and no saved WSL work, no undo mark is ever taken, so claimWslMoveRun behaves exactly as before.

User-risk: low. While a project comes back (seconds), a move of that same distro started from the dialog is refused as already running and can be retried. Other distros are unaffected.

Linked Issue

Follow-up to #35357 (merged), raised in its review.

Visual Proof

N/A. No UI change; it is ordering between background operations, covered by tests.

Testing

  • I manually tested these changes locally

  • Automated tests added/updated, or explained why not below

  • New wsl-move-release-here-move-race.test.ts, using the real driver, journal, undo path and orcad profile store (the kill rig):

    • With an undo paused while it stops the host's terminals, the orchestrator's move and its settling preview of that distro are refused wsl_move_already_running, another distro still claims, and the undo releases. With the claim refusal removed, this fails.
    • A move already running when the undo starts makes the undo refuse move-running before the host's terminals are stopped.
  • wsl-move-release-here.test.ts: a newer move journaled while host automations stop makes the undo refuse superseded without stopping the host's terminals, and the automations are restored. This fails without the late re-check.

  • updater-wsl-move-install-hold.test.ts:

    • the undo mark blocks only its own distro's claims (case-insensitive) until it ends
    • the install-hold cases still pass

Checks run: pnpm tc, pnpm run typecheck:ci, 111 test files (1181 tests passing: every updater test, the mover IPC tests and every moving-wsl-projects test), pnpm run check:code-quality:changed, anti-slop oxlint, oxlint, ast-grep scan, the flaky-test filter and gates.

Review

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

  • SSH: not involved; WSL host moves only.
  • Folder workspaces: undo of a moved folder workspace takes the same path.
  • Wire compatibility: no change.

Status

Ready for review. Two reviewers.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

@nwparker

Copy link
Copy Markdown
Contributor Author

STATUS: ready. Head abe2f5f, stacked on #35357 (fb1e131) and #35345 (8a57edc); I'll rebase onto main once they merge. The undo mark is now keyed by distro and taken before any guard or host connect. claimWslMoveRun (driver and the orchestrator's settling preview) refuses that distro while it runs, and the undo re-checks its guards just before stopping the host's terminals. Known gap noted in the body: a refusal after the stop still leaves host terminals stopped. Checks: pnpm tc, typecheck:ci, 111 test files (1181 tests passing), code-quality, anti-slop, oxlint, ast-grep scan, flaky filter and gates. Reviewers: fix-reviewer, e9-reviewer.

@greptile-apps

greptile-apps Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 4/5

[Medium impact] The changes affect WSL project moves, undo, and update timing during undo.

The move-versus-undo race is addressed, but the previous update-versus-undo race still blocks merging.

Findings

  1. P1 Update can interrupt an undo ▶

Summary

Blocks moves and settling previews of a distro while its undo runs.

  • A WSL project undo blocks new move claims for its distro.
  • Undo checks the move record again before stopping host terminals.

nwparker explicitly deferred recovery from failures after stopHostCopies, including journal-write failures and partial chain restores, because a “start host copies” port does not exist yet.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Start undo] --> B[Mark distro as undoing]
  B --> C[Check whether undo may proceed]
  C --> D[Connect host and stop automations]
  D --> E[Check again]
  E -->|Refused| F[Restore automations]
  E -->|Allowed| G[Stop host terminals and release move]
  F --> H[End undo mark]
  G --> H
  B -. Same distro move claims are refused .-> H
Loading

Reviews (3) · Last reviewed commit: "fix(wsl-mover): no move of a distro may ..." · Reviewed by Greptile

Comment on lines +62 to +63
const wslHold = wslMoveInstallHold()
if (wslHold) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Update can interrupt an undo

wslMoveInstallHold() is checked only before the updater waits for quit cleanup. On Windows, that cleanup saves agent state and waits for pending writes. An undo request arriving during that wait can still enter releaseWslMovedProjectHere; its handler does not check whether an update is installing. The updater then calls quitAndInstall without checking the undo mark again, so it can close Orca while the undo is stopping the host's terminals, before the project has returned.

Block new undos once installation starts, or re-check the hold immediately before calling the native installer and abort cleanly.

@nwparker
nwparker force-pushed the nwparker/wsl-move-undo-blocks-claims branch from abe2f5f to 9e77093 Compare October 28, 2026 21:19
@nwparker

Copy link
Copy Markdown
Contributor Author

STATUS: rebased onto the rebased #35357 (144ba18). Head 9e77093; the commit is identical to abe2f5f (range-diff '='). Approved by e9-reviewer; waiting on fix-reviewer. Not merged.

@coderabbitai

coderabbitai Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 9ef76c2d-ba24-4f15-a997-08555c9cb861
Commits

Reviewing files that changed from the base of the PR and between abe2f5f and 9e77093.

Files selected for processing: 1
  • src/main/updater.wsl-move-install-hold-driver.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.

Walkthrough

The changes coordinate WSL project moves with “Use on this computer again” undo operations. Undo runs block move claims for the same distro, and release checks for supersession before stopping host copies. The updater now holds installation during active moves or undos, records the corresponding lifecycle event, and reports a retryable status. Tests cover move and undo races, install retries, and localized hold messages.

Priority: Normal

Merge Risk: Minimal · up to 9e770

Updates are held for an already-running undo. An undo can still start during update preparation, a pre-existing limitation.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: preventing a distro move from starting while that distro is being undone.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description check Passed The description is complete and well aligned with the template. It explains the problem, user-visible behavior, implementation, rationale, testing, known gap, compatibility considerations, and visual-…
Full details: Description check

Explanation

The description is complete and well aligned with the template. It explains the problem, user-visible behavior, implementation, rationale, testing, known gap, compatibility considerations, and visual-proof status. The AI Disclosure section is omitted, but the template states that internal contributors should leave it empty. The linked issue is referenced as #35357 rather than using a Fixes keyword, but it identifies the relevant issue and follow-up context.

Full details: Docstring Coverage

Explanation

Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 7 functions across 8 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


Review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 153a465b-2f2a-42c5-9b90-49f3487d8267
Commits

Reviewing files that changed from the base of the PR and between 88e23db and abe2f5f.

Files selected for processing: 19
  • src/main/ipc/wsl-move-release-here-move-race.test.ts
  • src/main/ipc/wsl-move-release-here.test.ts
  • src/main/ipc/wsl-move-release-here.ts
  • src/main/persistence/moving-wsl-projects/wsl-move-running-moves.ts
  • src/main/updater.wsl-move-install-hold-driver.test.ts
  • src/main/updater/updater-install-execution.ts
  • src/main/updater/updater-wsl-move-install-hold.test.ts
  • src/main/updater/updater-wsl-move-install-hold.ts
  • src/renderer/src/i18n/locales/de.json
  • src/renderer/src/i18n/locales/en.json
  • src/renderer/src/i18n/locales/es.json
  • src/renderer/src/i18n/locales/fr.json
  • src/renderer/src/i18n/locales/ja.json
  • src/renderer/src/i18n/locales/ko.json
  • src/renderer/src/i18n/locales/pt.json
  • src/renderer/src/i18n/locales/ru.json
  • src/renderer/src/i18n/locales/tr.json
  • src/renderer/src/i18n/locales/zh-Hant.json
  • src/renderer/src/i18n/locales/zh.json

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

Comment on lines +62 to +63
const wslHold = wslMoveInstallHold()
if (wslHold) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Major · Stability & Availability · Heavy lift

Supported by static analysis

Script executed:

#!/bin/bash
# Inspect release entrypoints and install-state guards at the reviewed revision.
rg -n -C 5 'releaseWslMovedProjectHere\s*\(|releaseWslMoveChainHere\s*\(|quittingForUpdate|runBeforeUpdateQuitCleanup' src/main

Repository: stablyai/orca

Length of output: 30419


Script executed:

# Locate install-state guards, the hold definition, and preflight/quit call sites.
rg -n -F -e 'isQuittingForUpdate' -e 'wslMoveInstallHold' -e 'preflightMacInstall' -e 'quitAndInstall(' -e 'quittingForUpdate' src/main || test "$?" -eq 1

# Read the release handler around the call site, including its guards.
sed -n '130,200p' src/main/ipc/wsl-move-handlers.ts

# Read the install execution flow from entry through the awaited cleanup.
sed -n '40,130p' src/main/updater/updater-install-execution.ts

Repository: stablyai/orca

Length of output: 21087


Re-check the WSL move hold after the install awaits.

performQuitAndInstall reads wslMoveInstallHold() once, at updater-install-execution.ts:62. It then awaits preflightMacInstall (line 86) and runBeforeUpdateQuitCleanup (line 102) before it calls the native quitAndInstall (line 143). An undo that starts during either await is not seen by the hold.

The release entrypoint does not check install state. registerWslMoveUseHereHandler in src/main/ipc/wsl-move-handlers.ts checks the gate, the sender, and the request shape. It then calls releaseWslMovedProjectHere without reading quittingForUpdate. If the update proceeds during an undo, the quit can interrupt the undo after it has stopped host terminals.

Make undo admission and install quit share one state until native invocation. Refuse new WSL release claims once quittingForUpdate is set. Keep the install refusal path valid until quitAndInstall runs. A second hold check placed just before the native call does not close the window, because an undo can still start after that check.

@nwparker
nwparker force-pushed the nwparker/wsl-move-undo-blocks-claims branch from 9e77093 to 8932eae Compare October 28, 2026 21:41
@nwparker

Copy link
Copy Markdown
Contributor Author

STATUS: rebased --onto main 2be70e5 (#35357 merged as 89a7c68). Head 8932eae; range-diff identical to abe2f5f; the diff stat shows only the 5 race-fix files. Approved by e9-reviewer and fix-reviewer. Checks: pnpm tc, typecheck:ci, code-quality, 9 test files (127 tests passing). Not merged.

@nwparker
nwparker merged commit 04f015c into main Oct 28, 2026
65 of 68 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant