Repository navigation
fix(wsl-mover): hold an app update install while a moved project comes back - #35357
Conversation
|
STATUS: ready. Head fb1e131, stacked on #35345 (8a57edc); I'll rebase onto main when #35345 merges. Install is held during 'Use on this computer again' / Restore through a separate undo mark, so the move registry's readers (quit prompt, driver, Cancel, undo's own guard) are unchanged. The held messages are localized in 11 locales. Checks: pnpm tc, typecheck:ci, 57 test files (652 tests passing), localization catalogs + extraction, code-quality, anti-slop, oxlint, ast-grep scan, flaky filter and gates. Reviewers: fix-reviewer, e9-reviewer. |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 30 seconds. View limit details
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 |
[Medium impact] The change affects update installs during WSL project moves and returns on this desktop. The PR still needs a fix for an undo that starts while update shutdown work is waiting. Findings
SummaryHolds a user-requested update install while a moved WSL project comes back.
Reviews (2) · Last reviewed commit: "fix(wsl-mover): hold an app update insta..." · Reviewed by Greptile |
| const wslHold = wslMoveInstallHold() | ||
| if (wslHold) { |
There was a problem hiding this comment.
wslMoveInstallHold() is checked only before the updater awaits its shutdown work. On Windows, that work includes store.flushPendingAsync(). A user can start “Use on this computer again” while that wait is pending: the button remains enabled, and its handler does not check whether installation has started. The updater then quits without checking the new undo run, interrupting the host-stop work this change aims to protect.
Recheck the hold before invoking the installer, or prevent new undos once installation starts. Add a test that pauses shutdown work, starts an undo, and then resumes installation.
fb1e131 to
144ba18
Compare
ELI5
Installing an app update quits Orca. #34813 already refuses to install while a project is moving into a WSL host. Bringing a moved project back ("Use on this computer again", or Restore) was not covered, so an Install pressed mid-way would quit Orca while it was stopping the host's copies of the project's terminals. Install now waits for that too, and says why.
What Changed
Before: while "Use on this computer again" or Restore was stopping the host's terminals and dropping the move, pressing Install quit the app straight away.
After: Install is refused until the undo returns. The update card shows "A project is coming back from its WSL host. Install the update once that finishes." with Install to retry, and the staged download is kept. Once the undo finishes, Install works as before. Both held messages (move and undo) are now localized in all 11 languages.
Mechanism:
releaseWslMovesHere, the shared path of "Use on this computer again" and Restore, now marks an undo as running for its whole duration, using a newbeginWslMoveUndoRun()inwsl-move-running-moves.ts.wslMoveInstallHold()(src/main/updater/updater-wsl-move-install-hold.ts): is a move running (the existing message), or an undo running (the new message)?updater-install-execution.tsuses that, recordsquit_and_install_held_for_wsl_move_undofor an undo, and otherwise behaves as before.Why
Why a separate undo mark rather than counting undo as a move. Before choosing, I checked every reader of the move registry. Counting undo as a move would change what four of them do:
wslMove:isRunning→use-wsl-move-quit-guard): it would ask "Quit while a project is moving? The move will be undone at next launch", which is wrong for an undo.claimWslMoveRun): a move or preview of the same distro would be refused asalready_runningduring an undo.wsl-move-use-locally'sisWslMoveRunning): the undo would refuse itself asmove-running.cancelWslMove): it would report cancelling an undo that ignores the signal.isWslMoveUnderwayOnHost(the host server update hold) is unaffected either way. So undo gets its own mark and only the install path reads it.Scope: only the user's Install (
performQuitAndInstall) is held. An update that installs on its own when the app quits still installs if the user quits mid-undo. Quitting interrupts an undo either way, and undo stays out of the quit prompt for the reasons above. The hold is checked once, when Install is pressed.The hold lasts until the undo returns. The window reload it then starts runs with window writes held, and a quit then loses nothing the undo needs.
Flag-off: undo is only reachable while WSL hosts are serving (
wslHostsServing(): gate open, or saved WSL work behind a closed gate). WithORCA_INTERNAL_WSL_HOSTSunset and no saved WSL work, nothing ever begins an undo, so the install path behaves as before. The move message's text is unchanged in English.User-risk: low. An Install pressed during an undo is refused with an explanation and works once the undo ends, typically seconds. The undo mark is released in a
finally, so a failed or thrown undo can't hold updates.Linked Issue
Follow-up to #34813, found reviewing #35345 (merged).
Visual Proof
N/A. The held message appears in the existing update card's error state. Only the text differs, and it is covered by tests.
Testing
I manually tested these changes locally
Automated tests added/updated, or explained why not below
updater.wsl-move-install-hold-driver.test.ts(from test(wsl-mover): prove an update install is held during a real move #35345) has a new undo case:releaseWslMovedProjectHerethen runs, paused while it stops the host's copiesNew
updater-wsl-move-install-hold.test.ts:isAnyWslMoveRunningorclaimWslMoveRunChecks run:
pnpm tc,pnpm run typecheck:ci, 57 test files (652 tests passing: every updater test, the mover IPC tests, use-locally, orchestrator, main i18n and the locale policy tests),verify-localization-catalogs,verify-localization-extraction,pnpm run check:code-quality:changed, anti-slop oxlint, oxlint, ast-grep scan, the flaky-test filter and gates.Review
Agent skill upstream boundary
Notes
Status
Ready for review. Two reviewers (update/quit path).
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)