fix: ask Windows users to restart instead of failing WSL setup - #56
Conversation
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>
📝 WalkthroughWalkthroughWindows WSL setup now detects pending restart states, waits for elevated completion, and reports ChangesWindows WSL restart handling
Asset cache version update
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
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
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
📒 Files selected for processing (5)
CHANGELOG.mdpublic/index.htmlscripts/install-wsl-runtime.ps1src/client/onboarding.tstest/desktop.mjs
| 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') |
There was a problem hiding this comment.
🎯 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 testRepository: 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'}")
PYRepository: 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:
- 1: https://learn.microsoft.com/en-us/windows-hardware/manufacture/desktop/dism/dismrestarttype-enumeration?view=windows-11
- 2: https://stackoverflow.com/questions/67533186/what-is-the-difference-between-restartrequired-and-restartneeded-in-get-windowso
- 3: https://learn.microsoft.com/en-us/windows/client-management/client-tools/add-remove-hide-features
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.
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 -Waitis 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 correctrestart_requiredstatus that nothing read.Confirmed on the acceptance VM: the status file ended up holding
restart_required, withVirtualMachinePlatform=EnablePending,vmcomputeabsent, and CBS RebootPending true — while the Captain had been shown a failed installation.Fix
WaitForExit()), then wait a bounded 30s for a terminal child status rather than racing its final write.Test-PendingWslRestart: treat a reboot Windows has not taken yet asrestart_required, coveringEnablePending, DISM's ambiguousPossibleflag, an absentvmcomputeservice, and pending Component Based Servicing.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.Note:
mainis 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
New Features
Tests