Skip to content

fix: ask Windows users to restart instead of failing WSL setup - #56

Merged
gitcommit90 merged 2 commits into
mainfrom
fix/windows-wsl-restart-ux
Aug 2, 2026
Merged

fix: ask Windows users to restart instead of failing WSL setup#56
gitcommit90 merged 2 commits into
mainfrom
fix/windows-wsl-restart-ux

Conversation

@gitcommit90

@gitcommit90 gitcommit90 commented Aug 2, 2026

Copy link
Copy Markdown
Owner

Problem

On a genuinely fresh Windows 11 machine (WSL and VirtualMachinePlatform disabled, as they are by default), first-run setup showed "Shared runtime setup failed — Microsoft WSL 2.7.10.0 is not ready in the signed-in user's session" while the elevated PowerShell window was visibly still enabling VirtualMachinePlatform.

Root cause: Start-Process -Verb RunAs -Wait is not dependable for an elevated ShellExecute launch — it can return while the child is still working. The signed-in pass then read a non-terminal status, saw exit code 0, probed for a WSL runtime the child had not finished installing, and reported a failure. Seconds later the child wrote the correct restart_required status that nothing read.

Confirmed on the acceptance VM: the status file ended up holding restart_required, with VirtualMachinePlatform=EnablePending, vmcompute absent, and CBS RebootPending true — while the Captain had been shown a failed installation.

Fix

  • Block on the real elevated process handle (WaitForExit()), then wait a bounded 30s for a terminal child status rather than racing its final write.
  • New Test-PendingWslRestart: treat a reboot Windows has not taken yet as restart_required, covering EnablePending, DISM's ambiguous Possible flag, an absent vmcompute service, and pending Component Based Servicing.
  • Both failure exits in the signed-in pass now ask for a restart first when one is pending.
  • UI: a pending restart is no longer painted in the danger colour or under a "setup failed" heading, and gives explicit numbered steps (restart → sign back in as the same user → reopen 1Helm, setup resumes).

The restart itself is unavoidable — Windows cannot activate WSL 2 without one — so the goal is to state that plainly rather than imply breakage.

Verification

  • npm run ci: 118 tests, 0 failures.
  • New regression assertions cover the process-handle wait, the terminal-status wait, the pending-restart detection, the restart-before-failure ordering, and the three UI changes.

Note: main is now ahead of the staged 0.0.37 acceptance artifacts, so the next candidate must be cut as 0.0.38.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Improved Windows setup handling when WSL or system components require a restart.
    • Setup now reports restart-required states instead of incorrectly showing failures.
    • Improved handling of elevated setup completion and pending Windows restart conditions.
  • New Features

    • Added clear, numbered instructions for restarting, signing in, and resuming setup.
    • Updated onboarding status styling to distinguish restart-required states from actual failures.
  • Tests

    • Added regression coverage for Windows setup recovery and onboarding guidance.

Start-Process -Verb RunAs -Wait can return while the elevated child is
still enabling Windows optional features. The signed-in pass then read a
non-terminal status, saw exit code 0, probed for a WSL runtime the child
had not finished installing, and reported "Microsoft WSL is not ready in
the signed-in user's session" as a failed installation - while the
PowerShell window was visibly still enabling VirtualMachinePlatform.
Seconds later the child wrote the correct restart_required status that
nothing read.

The signed-in pass now blocks on the real process handle and waits a
bounded period for a terminal child status. A reboot Windows has not
taken yet is recognised as restart_required rather than failure, via
EnablePending, DISM's ambiguous "Possible" flag, an absent vmcompute
service, or a pending Component Based Servicing restart.

The restart is genuinely unavoidable: Windows cannot activate WSL 2 or
VirtualMachinePlatform without one. So the UI now states that plainly
instead of implying breakage - no danger colouring, no "setup failed"
heading, and explicit numbered steps telling the Captain to restart,
sign back in, and reopen 1Helm to resume.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 2, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Windows WSL setup now detects pending restart states, waits for elevated completion, and reports restart_required. Onboarding displays numbered restart and resume steps with distinct styling. Desktop tests cover the new setup and UI states. Asset cache versions were updated.

Changes

Windows WSL restart handling

Layer / File(s) Summary
Pending restart detection
scripts/install-wsl-runtime.ps1
The installer detects pending WSL features, missing vmcompute, registry reboot markers, and component-servicing restarts.
Elevated setup completion
scripts/install-wsl-runtime.ps1
The signed-in process waits for the elevated child and terminal status record. Pending Windows restarts now produce restart_required.
Restart-required onboarding and validation
src/client/onboarding.ts, test/desktop.mjs, CHANGELOG.md
The setup card displays numbered restart, sign-in, and resume steps. Restart states use muted styling, while failures retain danger styling. Regression assertions cover the new behavior.

Asset cache version update

Layer / File(s) Summary
Asset cache-busting
public/index.html
The stylesheet and module script use updated cache-busting query versions.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant SignedInProcess
  participant ElevatedSetup
  participant StatusFile
  participant WindowsRestartCheck
  participant OnboardingCard
  SignedInProcess->>ElevatedSetup: start elevated WSL setup
  SignedInProcess->>ElevatedSetup: wait for child process
  ElevatedSetup->>StatusFile: write terminal status
  SignedInProcess->>StatusFile: wait for terminal record
  SignedInProcess->>WindowsRestartCheck: check pending restart
  WindowsRestartCheck-->>SignedInProcess: return restart_required
  SignedInProcess->>OnboardingCard: render restart instructions
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains the problem, fix, and verification, but it omits the required change type, numbered acceptance ledger, checklist details, and post-merge sections. Add the missing template sections, including the selected change type, numbered acceptance outcomes, required verification checkboxes, and post-merge checklist.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: Windows users are asked to restart instead of seeing a WSL setup failure.
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.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/windows-wsl-restart-ux

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

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/install-wsl-runtime.ps1`:
- Around line 85-93: Update Test-RestartRequired to treat RestartRequired equal
to "Possible" as requiring a restart, alongside its existing restart-required
values. Add a regression test covering the "Possible" value and verify
Test-PendingWslRestart reports a pending restart when both features are enabled,
vmcompute exists, and no CBS reboot key is present.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 57ef8eda-a08d-4556-86b2-36e0944f3b13

📥 Commits

Reviewing files that changed from the base of the PR and between 83f2a2c and 275959c.

📒 Files selected for processing (5)
  • CHANGELOG.md
  • public/index.html
  • scripts/install-wsl-runtime.ps1
  • src/client/onboarding.ts
  • test/desktop.mjs

Comment on lines +85 to +93
function Test-PendingWslRestart {
if ($null -eq (Get-Service -Name vmcompute -ErrorAction SilentlyContinue)) { return $true }
foreach ($name in @("Microsoft-Windows-Subsystem-Linux", "VirtualMachinePlatform")) {
$feature = Get-WindowsOptionalFeature -Online -FeatureName $name -ErrorAction SilentlyContinue
if ($null -eq $feature) { continue }
if ([string]$feature.State -ne "Enabled") { return $true }
if (Test-RestartRequired $feature) { return $true }
}
return (Test-Path -LiteralPath 'HKLM:\SOFTWARE\Microsoft\Windows\CurrentVersion\Component Based Servicing\RebootPending')

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 4 'function Test-RestartRequired|RestartRequired|Test-PendingWslRestart' scripts/install-wsl-runtime.ps1 test

Repository: gitcommit90/1Helm

Length of output: 6830


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- helper and pending-restart logic ---'
sed -n '55,96p' scripts/install-wsl-runtime.ps1
printf '%s\n' '--- relevant regression assertions ---'
sed -n '175,198p' test/desktop.mjs
printf '%s\n' '--- available PowerShell runtimes ---'
command -v pwsh || true
command -v powershell || true

printf '%s\n' '--- static behavioral probe of the helper conditions ---'
python3 - <<'PY'
from pathlib import Path
import re

text = Path("scripts/install-wsl-runtime.ps1").read_text()
match = re.search(
    r'function Test-RestartRequired\s*\{(?P<body>.*?)\n\}',
    text,
    re.S,
)
if not match:
    raise SystemExit("Test-RestartRequired not found")
body = match.group("body")
print(body.strip())
for value in ("Required", "Possible", "1", "True", "False", "None"):
    accepted = any(f'"{value}"' in body for _ in [0])
    print(f"{value}: {'accepted by source conditions' if accepted else 'not accepted by source conditions'}")
PY

Repository: gitcommit90/1Helm

Length of output: 5300


🌐 Web query:

Microsoft Get-WindowsOptionalFeature RestartRequired Possible enum official documentation

💡 Result:

The RestartRequired property returned by the Get-WindowsOptionalFeature PowerShell cmdlet corresponds to the DismRestartType enumeration defined in the Microsoft.Dism.Commands namespace [1][2]. According to official Microsoft documentation for the DismRestartType enumeration, the possible values are [1]: - No (0): No restart is required [1]. - Possible (1): This package or feature might require a restart [1]. - Required (2): This package or feature always requires a restart [1]. In the context of the Get-WindowsOptionalFeature output, these values indicate whether a system restart is necessitated after enabling or disabling a specific optional feature [1][3]. Note that this property is distinct from the RestartNeeded property sometimes seen on objects returned by DISM-related cmdlets; RestartRequired describes the requirement inherent to the feature, while RestartNeeded typically indicates whether a restart is currently pending on the system [2].

Citations:


Treat RestartRequired = "Possible" as a pending restart.

Test-RestartRequired does not accept "Possible". If both features are Enabled, vmcompute exists, and CBS has no pending-restart key, Test-PendingWslRestart can report failure instead of restart_required.

Update the helper and add a regression case for "Possible".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/install-wsl-runtime.ps1` around lines 85 - 93, Update
Test-RestartRequired to treat RestartRequired equal to "Possible" as requiring a
restart, alongside its existing restart-required values. Add a regression test
covering the "Possible" value and verify Test-PendingWslRestart reports a
pending restart when both features are enabled, vmcompute exists, and no CBS
reboot key is present.

@gitcommit90
gitcommit90 merged commit 24db600 into main Aug 2, 2026
6 checks passed
@gitcommit90
gitcommit90 deleted the fix/windows-wsl-restart-ux branch August 2, 2026 05:54
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