Skip to content

test(wsl-mover): prove an update install is held during a real move - #35345

Merged
nwparker merged 1 commit into
mainfrom
nwparker/wsl-move-update-hold-proof
Oct 28, 2026
Merged

nwparker merged 1 commit into
mainfrom
nwparker/wsl-move-update-hold-proof

Conversation

@nwparker

@nwparker nwparker commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​272 0 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​272
Prod 0 0 0 0

ELI5

#34813 makes Orca refuse to install an app update while a project is moving into a WSL host, because installing quits the app mid-move. We can't prove that on the shared Windows test box, where real update prompts are never accepted. This adds an automated test that runs the real update-install code against a real project move, and checks both the hold and that the update installs once the move is over.

What Changed

Test-only. A new file, src/main/updater.wsl-move-install-hold-driver.test.ts, joins two existing harnesses:

  • The updater harness (createUpdaterMocks, used by updater.wsl-move-install-hold.test.ts): a fake auto-updater with a downloaded, staged update and an interactive install.
  • The move kill rig (createWslMoveKillRig, used by the fault-injection tests): the real move driver moveProjectsToWslHost, run against a real orcad profile store standing in for the host, with the real cutover journal and a kill switch.

The move is paused where it reaches for the host, so it is genuinely in flight when Install is pressed. Each test then checks:

  • While the move runs: the install is held. The updater never calls quitAndInstall, no terminals are killed, the window gets updater:quitAndInstallAborted, the status card shows "A project is moving to its WSL host. Install the update once the move finishes." with an Install retry, and the quit_and_install_held_for_wsl_move lifecycle record is written.
  • After the move settles, pressing Install again installs the same staged update (quitAndInstall once; only the earlier attempt was aborted). That is checked for four endings:
    1. the move finishes (moved)
    2. the move fails because its host stops answering (refused)
    3. the user cancels the move (wsl_move_cancelled)
    4. the desktop is killed mid-move (the kill switch fires at the pause step), then the next launch's recovery settles the move (not stuck) with no move running

Why

The existing test from #34813 claims the running-moves registry directly. It proves the install path reads the registry, but not that a real move run claims it while in flight and releases it on every way a run can end. This test drives the real driver, so a run that forgot to release, or released too early, fails it. With the hold disabled, all four tests fail.

Why not e2e:

  • The e2e move specs replace main's wslMove:* handlers with a scripted host, so no real driver runs.
  • The e2e update specs only set the renderer's update status.
  • Neither path runs the main-process install code against a real move.

The integration level is the highest that runs both for real. The test loads the move modules after the updater harness resets modules, so both read one running-moves registry.

Flag-off: test-only. The test turns the WSL hosts gate on itself; nothing ships.

User-risk: none (no production code changes).

Linked Issue

Follow-up to #34813 (CI proof for the update-during-move hold, which awin can't exercise).

Visual Proof

N/A. Test-only.

Testing

  • I manually tested these changes locally

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

  • The new file: 4 tests. It passes 3 runs in a row, together with updater.wsl-move-install-hold.test.ts and wsl-move-driver-fault-injection.test.ts (9 tests per run).

  • Mutation check: with the hold condition disabled in updater-install-execution.ts, all 4 fail.

Checks run: pnpm tc, pnpm run typecheck:ci, pnpm run check:code-quality:changed, anti-slop oxlint, oxlint, ast-grep scan, the flaky-test filter and gates. The test never sleeps in real time: the updater runs on fake timers, and the move is paused on a promise.

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

  • Platform: the updater harness handles macOS's native-updater step, as the existing hold test does.
  • SSH, folder workspaces and wire compatibility: not affected; test-only.

Status

Ready for review.

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 8a57edc, from origin/main 7b3fb54. Test-only: the real updater install path against a real move-driver run over the kill rig. The install is held with its message while the move runs, and the same staged update installs after the move is moved, refused (host gone), cancelled, or killed mid-run and recovered on relaunch. All 4 tests fail with the hold disabled. Checks: pnpm tc, typecheck:ci, code-quality, anti-slop, oxlint, ast-grep scan, flaky-test filter and gates; 3 repeat runs green. Review requested from fix-reviewer.

@coderabbitai

coderabbitai Bot commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You'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 20 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

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: 1cb24412-48c0-48df-afae-301df49e4cad
Commits

Reviewing files that changed from the base of the PR and between dd0c4cf and 8a57edc.

Files selected for processing: 1
  • src/main/updater.wsl-move-install-hold-driver.test.ts
  • 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.

@greptile-apps

greptile-apps Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 4/5

[Low impact] The PR's effects stay inside automated tests.

This test-only change is safe to merge, with a non-blocking gap in the crash recovery test.

Findings

  1. P2 Recovery is never exercised ▶

Summary

Adds four tests that join the updater harness to the real WSL move driver.

  • The updater test proves installs wait until a real WSL move settles.

Reviews (1) · Last reviewed commit: "test(wsl-mover): prove an update install..." · Reviewed by Greptile

Comment on lines +252 to +253
// Killed at the pause, after the move has journaled its cutover.
staged.mover.rig.kill.killAt = 'sleep'

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Recovery is never exercised

The sleep kill point fires before the move writes its journal. The driver turns that error into a refusal, then releases the running move in finally. Recovery has nothing to settle, so this test passes even if journal recovery does nothing.

Use a kill point after journaling, assert that an unfinished journal exists before restart, and check its expected result afterward.

Knowledge Base Used: Desktop persistence and lifecycle

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