Stop Docker Desktop and Update WSL During a Docker Install or Upgrade - #705
Conversation
#701 checked the WSL platform floor before a docker install or upgrade and only named the remedy, upgrade-host.ps1 -Wsl. That left docker to skip outright whenever WSL was behind, since that script also refuses while Docker Desktop is running. Verifying #701 on a real Windows host surfaced two more gaps. Docker Desktop's own WSL integration goes stale across most engine bumps, surfacing as "WSL integration with distro '<name>' unexpectedly stopped". The in-app "Restart the WSL integration" button does not clear it. And wsl --update raises its own UAC prompt, which a headless run cannot answer. - `install-tools.ps1`: a docker install, upgrade or reinstall now drives the whole maintenance window itself. It stops Docker Desktop through its own CLI (`docker desktop stop`/`start`), runs `wsl --update` where the floor is not met, shuts every WSL distro down with `wsl --shutdown` so each one's integration remounts fresh, and restarts Docker Desktop once the package itself is settled. Confirmed first, unless `-Yes` was given. A non-elevated, non-interactive run refuses to start `wsl --update` rather than hang on its UAC prompt, and restores Docker Desktop to how it found it. The WSL-readiness check moved from an unconditional early gate to just before real work happens, so an already-current docker no longer fails a run over an unrelated stale WSL platform. - `README.md`: documents the moved boundary. WSL used to be strictly read-only here, naming `upgrade-host.ps1 -Wsl` as a person's own step. A docker version bump already needing the same stop-then- restart window for its own integration is what moved it. ## Verification Exercised live on a real Windows host, not simulated: a real docker upgrade (4.85.0 -> 4.86.0) through winget; the WSL-below-floor path through a real `wsl --update` past an actual UAC prompt (WSL 2.7.10.0 -> 2.7.11.0), re-verifying the floor and restarting Docker Desktop afterward; the plain engine-bump stop, shutdown, restart cycle; the declined-prompt and neither-condition-true branches, through mocked extraction of the real functions; and the headless-UAC-refusal guard, both tripping unelevated and bypassing elevated correctly. `Invoke-ScriptAnalyzer` against the repo's own settings, a parse check, `pytest scripts/test_bootstrap.py scripts/test_host_gate.py` (82 passed), and `host_gate.py` all pass.
There was a problem hiding this comment.
Pull request overview
This PR updates the Windows host setup flow so Docker install/upgrade operations can safely manage the required WSL maintenance window (including stopping/restarting Docker Desktop and performing WSL updates) instead of only reporting WSL as a prerequisite.
Changes:
- Extend
install-tools.ps1to stop Docker Desktop viadocker desktop stop/start, optionally runwsl --updatewhen below Docker’s WSL floor, and force a clean remount viawsl --shutdownaround Docker package changes. - Add safeguards for headless/unelevated runs so
wsl --updatedoesn’t hang on an unanswerable UAC prompt, and ensure Docker Desktop is restored to its prior running state. - Update
host-setup/windows/README.mdto document the moved “WSL is read-only” boundary and the new Docker/WSL maintenance behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| host-setup/windows/install-tools.ps1 | Adds Docker Desktop stop/start + WSL update/shutdown orchestration during docker install/upgrade/reinstall, moving WSL readiness from an early gate to just-in-time before work. |
| host-setup/windows/README.md | Documents the new Docker maintenance window behavior and why WSL updates are now driven during docker changes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Copilot review on #705 found two real gaps in Enter-DockerMaintenance. - A `docker desktop stop` failure, or Docker Desktop still answering as running right after a reported success, went unchecked, so the window could still run `wsl --update` against a host that never actually let go of the WSL service, the exact failure mode the window exists to avoid. - The confirm prompt for the WSL-update path always mentioned stopping and restarting Docker Desktop, even when it was never running to begin with, which reads as a promise the run does not keep. `install-tools.ps1`: Enter-DockerMaintenance now refuses and returns `Proceed = $false` where a stop fails or Docker Desktop is still detected running afterward, instead of proceeding regardless. The WSL-update prompt now names only WSL when Docker Desktop was not running to stop. Verified with mocked extraction of the real functions: a failed stop never reaches `wsl --update`, a "successful" stop that leaves Docker Desktop still running is treated the same way, and the prompt text carries no stop/restart mention when nothing will be stopped. Parse check, `Invoke-ScriptAnalyzer` against the repo's own settings, and `pytest scripts/test_bootstrap.py scripts/test_host_gate.py` (82 passed) all pass.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (2)
host-setup/windows/install-tools.ps1:626
Exit-DockerMaintenancerunswsl.exe --shutdownviarun, which pipes native output through PowerShell (| Out-Host). This bypasses theInvoke-WslUTF-16/WSL_UTF8handling described earlier in the file and can produce NUL-separated output. SetWSL_UTF8=1around this call (or route it throughInvoke-Wsl) to keep output readable and consistent with the file’s own convention forwsl.execalls.
info 'Shutting down WSL'
$shutdownCode = run 'wsl.exe' @('--shutdown')
if ($shutdownCode -ne 0) { warn "wsl --shutdown exited $shutdownCode" }
host-setup/windows/install-tools.ps1:607
Enter-DockerMaintenancerunswsl.exe --updateviarun, which pipes native output through PowerShell (| Out-Host). Earlier in this file,Invoke-Wslnotes thatwsl.exeemits UTF-16 by default and needsWSL_UTF8=1to avoid NUL-separated output when captured/piped. This new call path bypasses that guard, sowsl --updateoutput can become unreadable and it breaks the stated invariant that everywsl.execall goes throughInvoke-Wsl. SetWSL_UTF8=1around therun wsl.exe --updatecall (or route it throughInvoke-Wsl) so output stays readable and consistent.
This issue also appears on line 624 of the same file.
info 'Updating the WSL platform'
info 'This restarts every distribution, so anything running inside one is stopped'
$updateCode = run 'wsl.exe' @('--update')
if ($updateCode -ne 0) { warn "wsl --update exited $updateCode" }
Copilot's suppressed findings on #705 round 2 caught that Enter-DockerMaintenance and Exit-DockerMaintenance ran wsl --update and wsl --shutdown through the plain `run` wrapper, bypassing the WSL_UTF8 guard Invoke-Wsl applies to every other wsl.exe call in this file. Confirmed real: a side-by-side comparison shows the old path prints "W S L v e r s i o n :..." (UTF-16 read as single-byte characters), which is exactly what an earlier live test of this same code already printed without it being caught as a bug at the time. `install-tools.ps1`: adds Invoke-WslRun, the mutation-side counterpart to Invoke-Wsl, which applies the same WSL_UTF8 guard around a live streamed `run` call instead of a captured read. Both new call sites route through it instead of calling `run 'wsl.exe'` directly. Verified live: the same wsl --version call through the old path prints garbled spaced-out text, through the new path prints clean readable lines. Parse check and `Invoke-ScriptAnalyzer` against the repo's own settings both pass.
|
Answering the 2 suppressed findings from round 2 (#705 (review), commit 1. Fixed in 2. Fixed in |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (4)
host-setup/windows/install-tools.ps1:614
- The headless/UAC refusal message suggests running
upgrade-host.ps1 -Wsl, but that script refuses while Docker Desktop is running (as documented in this README). In this branch you may also have just restarted Docker Desktop to restore state, so the suggested remedy can immediately fail again. Update the warning to explicitly tell the user to quit/stop Docker Desktop before running the WSL upgrade.
if (-not $script:ELEVATED -and (-not [Environment]::UserInteractive -or [Console]::IsInputRedirected)) {
warn "wsl --update needs administrator and raises its own prompt, which nothing can answer unattended; it would hang rather than fail, so this refuses to start it. Run interactively once so the prompt has someone to answer, or update WSL first with: host-setup\windows\upgrade-host.ps1 -Wsl"
if ($result.Stopped) { Start-DockerDesktop | Out-Null; $result.Stopped = $false }
host-setup/windows/install-tools.ps1:764
- When
Enter-DockerMaintenancereturnsProceed = $falsedue to a Docker Desktop stop failure (no WSL problem),$wslProblemis$nulland this warning becomesdocker skipped,with no useful reason. Include a fallback reason so the log stays actionable.
if (-not $dockerMaintenance.Proceed) {
warn "docker skipped, $wslProblem"
$script:FAILED += $ToolName
return
}
host-setup/windows/README.md:86
- This paragraph implies the Docker upgrade flow always stops/starts Docker Desktop and always runs
wsl --shutdown. The implementation only restarts Docker Desktop (and runswsl --shutdown) when it actually stopped Docker Desktop (i.e., when it was running). Please align the README wording with the script’s restore-to-previous-state behavior.
Where WSL is present but behind the floor, or `docker` itself is about to change version, `install-tools.ps1` drives the fix itself rather than only naming it, asking first unless `-Yes` was given: it stops Docker Desktop through its own CLI (`docker desktop stop`, not the tray icon), runs `wsl --update` where the floor is not met, shuts every WSL distribution down with `wsl --shutdown`, and starts Docker Desktop again (`docker desktop start`) once the `docker` package itself is also settled.
host-setup/windows/install-tools.ps1:579
- These comments describe WSL shutdown and Docker Desktop restart as unconditional, but
Exit-DockerMaintenanceonly runs when Docker Desktop was actually stopped (Stopped = $true). Update the comment block to reflect the conditional behavior so it matches what the code does.
This issue also appears on line 612 of the same file.
# Everything a docker install, upgrade or reinstall needs from Docker Desktop and WSL before winget touches the package is folded into one window rather than run twice: Docker Desktop holds the WSL service open, so a platform update fails part way while it is running, the same reason upgrade-host.ps1 -Wsl refuses outright.
# Docker Desktop's own per distro WSL integration also goes stale across an engine bump often enough that it has a name on Docker's own tracker, surfacing as "WSL integration with distro '<name>' unexpectedly stopped" the next time anything in that distro touches docker, and the in app "Restart the WSL integration" button on that dialog does not clear it, since it retries the proxy inside the distro that is already running against the same stale state.
# Both are fixed the same way: Docker Desktop stopped, WSL brought current, every distro shut down with it so each one's integration remounts fresh rather than being patched in place, and Docker Desktop started again once the docker package itself is also settled, by Exit-DockerMaintenance below.
# Boundary note: this script used to be read-only on WSL by design, naming upgrade-host.ps1 -Wsl as a person's own step rather than running it.
# The maintainer moved that boundary once a docker version bump on its own already needed this same stop, then restart, window for its own integration to recover, since a platform update asks for nothing more than that same window with Docker Desktop already stopped inside it.
Copilot's round-3 suppressed findings on #705 caught four more real gaps. - The headless-UAC refusal named upgrade-host.ps1 -Wsl as the remedy without saying Docker Desktop has to be quit first, and on the path that had just restarted Docker Desktop to restore state, that named remedy would fail again immediately. - A docker skip over a failed Docker Desktop stop, rather than a WSL gap, logged "docker skipped, " with nothing after the comma, since the WSL-problem string that message expected was empty on that path. - Both README.md and the Enter-DockerMaintenance comment block described the stop, WSL update, shutdown, restart cycle as unconditional, when it only runs where Docker Desktop was actually running to begin with. install-tools.ps1: the UAC-refusal warning now says to quit Docker Desktop before running the named remedy. The skip warning falls back to a real reason when no WSL problem string is set. The comment block now says the cycle runs "where Docker Desktop is running to begin with." README.md: reworded to match, a run that finds Docker Desktop already stopped touches neither the shutdown nor the restart. Parse check and Invoke-ScriptAnalyzer against the repo's own settings both pass. pytest scripts/test_bootstrap.py scripts/test_host_gate.py: 82 passed.
|
Answering the 4 suppressed findings from round 3 (#705 (review), commit 1. Fixed in 2. Fixed in 3. Fixed in 4. Fixed in |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (2)
host-setup/windows/install-tools.ps1:593
- The confirmation prompt embeds
$WslProblemverbatim, butTest-WslReadyForDockerincludes a full remedy sentence (e.g., “Update it with: host-setup\windows\upgrade-host.ps1 -Wsl”). Now that this script can runwsl --updateitself, the prompt becomes misleading/duplicative. Consider stripping the “Update it with:” clause (and similar) before showing it in the interactive question so the prompt reflects what the script will actually do.
$question = if ($WslProblem -and $WasRunning) {
"docker needs WSL updated first ($WslProblem). Stop Docker Desktop, update WSL, and restart Docker Desktop to continue?"
} elseif ($WslProblem) {
"docker needs WSL updated first ($WslProblem). Update WSL to continue?"
} else {
host-setup/windows/install-tools.ps1:614
- This warning says “including one this line just restarted…”, but the restart happens after the warning (and only when
$result.Stoppedis true). Reword the message so it doesn’t claim the restart already occurred.
if (-not $script:ELEVATED -and (-not [Environment]::UserInteractive -or [Console]::IsInputRedirected)) {
warn "wsl --update needs administrator and raises its own prompt, which nothing can answer unattended; it would hang rather than fail, so this refuses to start it. Run interactively once so the prompt has someone to answer, or quit Docker Desktop yourself and run host-setup\windows\upgrade-host.ps1 -Wsl, which refuses on its own while Docker Desktop is running, including one this line just restarted to leave the host as it found it"
if ($result.Stopped) { Start-DockerDesktop | Out-Null; $result.Stopped = $false }
Copilot's round-4 suppressed findings on #705 caught two more gaps, one of them my own previous fix's wording. - The confirm prompt embedded Test-WslReadyForDocker's string verbatim, which carries its own "Update it with: host-setup\windows\upgrade-host.ps1 -Wsl" remedy clause, meant for a caller that will not fix WSL itself. Shown inside a prompt for a run about to do exactly that, it read as misleading and duplicative. - The UAC-refusal warning I reworded last round claimed "this line just restarted" Docker Desktop, but the restart is the statement right after the warning, not the warning itself, so the claim was temporally wrong at the moment it printed. install-tools.ps1: the prompt now strips the remedy clause before embedding the WSL-problem string. The UAC-refusal warning now says the refusal "also restarts Docker Desktop... so it may already be running again by the time this is read," instead of claiming it already had. Parse check and Invoke-ScriptAnalyzer against the repo's own settings both pass. pytest scripts/test_bootstrap.py scripts/test_host_gate.py: 82 passed.
|
Answering the 2 suppressed findings from round 4 (#705 (review), commit 1. Fixed in 2. Fixed in |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (2)
host-setup/windows/install-tools.ps1:630
- In
-DryRun,Invoke-WslRun '--update'is a no-op that returns 0, but the script still re-checks WSL readiness and will mark docker as skipped/failed because WSL is (correctly) still behind. Dry run should print what it would do without requiring the host state to have changed.
$updateCode = Invoke-WslRun '--update'
if ($updateCode -ne 0) { warn "wsl --update exited $updateCode" }
$stillBroken = Test-WslReadyForDocker
if ($stillBroken) {
warn "docker skipped, still $stillBroken after the update"
if ($result.Stopped) { Start-DockerDesktop | Out-Null; $result.Stopped = $false }
$result.Proceed = $false
return $result
}
host-setup/windows/install-tools.ps1:610
- In
-DryRun,runreturns success without actually stopping Docker Desktop, but this block still re-checksTest-DockerDesktopRunningand will always treat Docker Desktop as “did not stop” when it’s running, causing a dry run to incorrectly skip/fail docker maintenance.
This issue also appears on line 622 of the same file.
if ($WasRunning) {
if ((Stop-DockerDesktop) -ne 0 -or (Test-DockerDesktopRunning)) {
warn 'docker skipped, Docker Desktop did not stop, and a WSL platform update fails part way while it is running'
$result.Proceed = $false
return $result
}
$result.Stopped = $true
}
Copilot's round-5 suppressed findings on #705 caught a real regression: Enter-DockerMaintenance's stop-failure and WSL-still-broken guards check real, live host state, but under -DryRun nothing actually ran, so Docker Desktop is still genuinely running and WSL is still genuinely behind. Both guards fired as failures on every dry run against a live host, rather than showing a preview. install-tools.ps1: both re-checks (Test-DockerDesktopRunning after the stop, Test-WslReadyForDocker after the update) are skipped under -DryRun, and the run proceeds to preview the rest of the window as if the action had succeeded. Verified live: `-Reinstall docker -DryRun` on this host, with Docker Desktop genuinely running, now previews the full stop, install, shutdown, start sequence instead of failing on "Docker Desktop did not stop." Parse check and Invoke-ScriptAnalyzer against the repo's own settings both pass. pytest scripts/test_bootstrap.py scripts/test_host_gate.py: 82 passed.
|
Answering the 2 suppressed findings from round 5 (#705 (review), commit 1. Fixed in 2. Fixed in Re-verified live after both fixes: |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (1)
host-setup/windows/install-tools.ps1:617
- This warning says “this refusal also restarts Docker Desktop…”, but in the code path where
$WasRunningis$false(so$result.Stoppedstays$false) nothing is stopped or restarted. That makes the message misleading for headless runs where Docker Desktop was already not running.
if (-not $script:ELEVATED -and (-not [Environment]::UserInteractive -or [Console]::IsInputRedirected)) {
warn "wsl --update needs administrator and raises its own prompt, which nothing can answer unattended; it would hang rather than fail, so this refuses to start it. Run interactively once so the prompt has someone to answer, or quit Docker Desktop yourself and run host-setup\windows\upgrade-host.ps1 -Wsl, which refuses on its own while Docker Desktop is running: this refusal also restarts Docker Desktop, to leave the host as it found it, so it may already be running again by the time this is read"
Copilot's round-6 suppressed finding on #705 caught that the UAC-refusal warning's restart mention was still unconditional text, even after the round-4 fix: on the path where $WasRunning is $false, nothing was ever stopped, so nothing restarts, and the message claimed otherwise. install-tools.ps1: the restart clause is now built from $result.Stopped at the point the warning fires, rather than a fixed string, so it only appears when this refusal is the thing about to restart Docker Desktop. Parse check and Invoke-ScriptAnalyzer against the repo's own settings both pass. pytest scripts/test_bootstrap.py scripts/test_host_gate.py: 82 passed.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (1)
host-setup/windows/install-tools.ps1:558
Test-DockerDesktopRunningdetects the Docker Desktop state by regex-matching the JSON output. Since the command already emits JSON (--format json), parsing it withConvertFrom-Jsonis more robust (and matches the JSON-handling pattern used elsewhere, e.g.setup-wsl.ps1:191-196). Keeping a regex fallback is fine if the CLI ever prints non-JSON on some versions.
$text = (& docker desktop status --format json 2>&1 | Out-String)
if ($LASTEXITCODE -ne 0) { return $false }
return ($text -match '"Status"\s*:\s*"running"')
Copilot's round-7 suppressed finding on #705: Test-DockerDesktopRunning regex-matched the JSON `docker desktop status --format json` already emits, rather than parsing it, unlike the ConvertFrom-Json pattern setup-wsl.ps1 already uses for Docker's own settings file. install-tools.ps1: parses the status with ConvertFrom-Json, keeping the regex as a fallback for a CLI version that ever answers non-JSON, rather than reading a parse failure as "not running" outright. Verified live: against this host's actually-running Docker Desktop, the new path returns True. Parse check and Invoke-ScriptAnalyzer against the repo's own settings both pass. pytest scripts/test_bootstrap.py scripts/test_host_gate.py: 82 passed.
|
Answering the 1 suppressed finding from round 7 (#705 (review), commit
Fixed in |
Copilot review on #705 found that Test-DockerDesktopRunning returned $false whenever the docker CLI was off PATH or docker desktop status failed, even where Docker Desktop was genuinely running. That let Enter-DockerMaintenance skip the stop step and proceed straight into wsl --update against a live Docker Desktop, the exact failure mode this whole maintenance window exists to prevent. install-tools.ps1: falls back to the same process probe upgrade-host.ps1's Get-DockerProcess already uses (Docker Desktop, com.docker.backend, com.docker.build) whenever the CLI is unavailable or its status call fails, rather than reading either as "not running." Verified live: with the CLI path mocked unavailable, the process-probe fallback correctly reports this host's genuinely-running Docker Desktop as running. Parse check and Invoke-ScriptAnalyzer against the repo's own settings both pass. pytest scripts/test_bootstrap.py scripts/test_host_gate.py: 82 passed.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (1)
host-setup/windows/install-tools.ps1:623
Enter-DockerMaintenancecurrently treats any non-zero exit code fromdocker desktop stopas a hard failure ($stopCode -ne 0 -or Test-DockerDesktopRunning). That means the docker operation can be skipped even when Docker Desktop actually stopped successfully, and in that case the function returns withStopped = $false, so the caller may not restart Docker Desktop (leaving the host in a different state than it started). Consider basing the refusal solely on whether Docker Desktop is still running after the stop attempt (the condition that actually matters forwsl --update), and setStopped = $truewhenever Docker Desktop is observed stopped.
$stopCode = Stop-DockerDesktop
if (-not $script:DRY_RUN -and ($stopCode -ne 0 -or (Test-DockerDesktopRunning))) {
warn 'docker skipped, Docker Desktop did not stop, and a WSL platform update fails part way while it is running'
$result.Proceed = $false
return $result
Copilot's round-9 suppressed finding on #705: Enter-DockerMaintenance refused the run on any non-zero exit from docker desktop stop, even where Docker Desktop had actually stopped. Worse, that path also left Stopped unset, so Exit-DockerMaintenance below had no way to know a restart was owed, stranding the host with Docker Desktop off when it started running. install-tools.ps1: the refusal now checks only whether Docker Desktop is still observed running after the attempt, the condition that actually matters for wsl --update, and sets Stopped whenever it is observed stopped, regardless of the exit code. Verified live: -Reinstall docker -DryRun still previews the full stop, install, shutdown, start sequence correctly. Parse check and Invoke-ScriptAnalyzer against the repo's own settings both pass. pytest scripts/test_bootstrap.py scripts/test_host_gate.py: 82 passed.
|
Answering the 1 suppressed finding from round 9 (#705 (review), commit
Fixed in |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (2)
host-setup/windows/install-tools.ps1:116
- The header comment says this behavior applies only to a docker install/upgrade, but the script also supports
-Reinstalland will run the same Docker Desktop/WSL maintenance window in that mode. Updating the comment avoids misleading readers about when WSL/Docker Desktop will be stopped or restarted.
An install or upgrade of docker also brings the WSL platform itself up to Docker Desktop's own
floor where it is behind, and stops Docker Desktop first either way, since Docker Desktop holds
the WSL service open and both a platform update and its own WSL integration otherwise need a
restart to recover from an engine bump. wsl --update raises its own administrator prompt, which
this refuses to start unattended where nothing could answer it, rather than hang.
host-setup/windows/install-tools.ps1:606
- The confirmation prompt claims "docker is about to change version", but this path can also run for
-Reinstall(or a first-time install) where the version might not be changing. Consider wording the prompt in terms of an install/upgrade/reinstall to keep it accurate across modes.
} else {
"docker is about to change version, and Docker Desktop's own WSL integration commonly goes stale across an engine bump. Stop Docker Desktop first, and restart it after, to avoid that?"
}
Copilot's round-10 suppressed findings on #705: the usage text and the plain engine-bump confirm prompt both described the maintenance window as an install-or-upgrade behavior, but -Reinstall drives the same window (already verified live earlier in this PR) and the prompt's "about to change version" framing does not fit a reinstall to the same version. install-tools.ps1: usage text now says "install, upgrade or reinstall." The confirm prompt now says "about to be upgraded or reinstalled" rather than "about to change version" -- this branch only reaches a docker that is already installed and running, so a fresh install was never a case it needed to cover. Parse check and Invoke-ScriptAnalyzer against the repo's own settings both pass. pytest scripts/test_bootstrap.py scripts/test_host_gate.py: 82 passed.
|
Answering the 2 suppressed findings from round 10 (#705 (review), commit 1. Fixed in 2. Fixed in |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (3)
host-setup/windows/install-tools.ps1:584
Start-DockerDesktopalso assumes thedockerCLI is available. If Docker Desktop was stopped via process-based detection (or PATH is broken), the restart step can throw a command-not-found error instead of warning and continuing. Add the sameGet-Command dockerguard used for stopping.
function Start-DockerDesktop {
info 'Starting Docker Desktop'
$code = run 'docker' @('desktop', 'start', '--timeout', '180')
if ($code -ne 0) { warn 'docker desktop start did not exit cleanly, start Docker Desktop by hand and check WSL integration per distro' }
return $code
}
host-setup/windows/install-tools.ps1:577
Stop-DockerDesktopassumes thedockerCLI is onPATH. ButTest-DockerDesktopRunningexplicitly falls back to a process probe when the CLI is missing, meaning the script can detect Docker Desktop running yet crash here with “docker not recognized” when trying to stop it. Guard for a missingdockercommand and fail the maintenance step cleanly (soEnter-DockerMaintenancecan refuse and the run can restore state).
This issue also appears on line 579 of the same file.
function Stop-DockerDesktop {
info 'Stopping Docker Desktop'
$code = run 'docker' @('desktop', 'stop', '--timeout', '90')
if ($code -ne 0) { warn "docker desktop stop exited $code" }
return $code
}
host-setup/windows/README.md:7
- The README now says only installing/upgrading
dockerupdates the WSL platform, butinstall-tools.ps1’s header comment states that reinstall of docker also triggers the same maintenance window. Update the summary line to match the script’s behavior.
- [`install-tools.ps1`][install-tools] installs and upgrades the host tools, and reports what each one is installed at, where it came from, and which scope it sits in. Installing or upgrading `docker` also brings the WSL platform up to Docker Desktop's own floor where it is behind.
Copilot's round-11 suppressed findings on #705 caught a real crash risk introduced by the round-8 process-probe fallback: Test-DockerDesktopRunning can now answer true through the process probe alone, with the docker CLI missing or off PATH, but Stop-DockerDesktop and Start-DockerDesktop both still called `docker desktop stop`/`start` unconditionally, which is a PowerShell command-not-found throw rather than a native exit code either function could warn on and recover from. install-tools.ps1: both functions now check for the docker CLI first, warning and returning a non-zero code (rather than throwing) when it is missing. The existing Test-DockerDesktopRunning gate in Enter-DockerMaintenance already refuses the run correctly in that case, since nothing was actually stopped. README.md: the What Is Here summary line now says "installing, upgrading or reinstalling docker," matching the header comment fixed last round. Verified live: both functions warn and return 1 instead of throwing when the CLI is mocked missing. -Reinstall docker -DryRun still previews correctly. Parse check and Invoke-ScriptAnalyzer against the repo's own settings both pass. pytest scripts/test_bootstrap.py scripts/test_host_gate.py: 82 passed.
|
Answering the 3 suppressed findings from round 11 (#705 (review), commit 1. Fixed in 2. Fixed in 3. Fixed in |
Merge origin/develop (#704, #705, #709 landed since this branch forked) to clear the DIRTY merge state. Only .claude-plugin's .source-digest conflicted; resolved by regenerating via scripts/build_dist.py, same as every prior round on this branch. host-setup/linux/README.md still pointed at scripts/test_bootstrap.py, which #704 moved to scripts/tests/test_bootstrap.py on develop without updating this reference. Pre-existing drift, not caused by this branch, but merging develop in surfaces it as a live dead-path finding in prose_lint's CI-gated check, so fixed the reference (link target and both visible mentions) to keep the gate green. Ran the full local verification set post-merge: test_prose_lint.py, test_repo_gate.py, test_pr_review.py, spec/audit.py --selftest, gh-write-guard.py --selftest, repo_gate.py, and the full-tree prose_lint CI invocation. All green.
…Python CI Gates (#718) Thirty-one squashes, `56f4d7d..d54862a`. 115 files, +20436/-5298. **Merge with a merge commit, never a squash, and never with `--delete-branch`.** This pull request's head is `develop` itself. ## What lands **Fleet Skills.** The `.agents/skills/` source tree, the generated `.claude-plugin/` distribution, `scripts/build_dist.py` with its `--check` gate, and `scripts/skills_install.py` with its host stamp (#676). Packaged as skills on top of the scaffold: PR review conduct and Copilot instructions upkeep (#677), comment and doc style (#678), resync-a-repo and fleet-conformance-check (#679), the per-language codestyles (#680), git commit conventions and operational vs release workflow (#681), stand up a repo (#683), and repo-worktree (#717). Coverage gaps closed in three passes (#690, #691, #692) plus the P4 sentence-length opt-in (#697). **Host setup.** The Windows host-setup tooling and its PowerShell gate (#674), the Windows bootstrap loader (#682), Docker install and upgrade on Linux and Windows with a version floor (#701, #705), a `uv` floor in `spec/host-tools.json` (#698), self-healing of a shadowing `uv`, `jq`, or `git-restore-mtime` copy (#689), node's real winget package id (#696), and a README for the Linux host-setup nuances (#710). **Python and CI.** Python tooling in CI with the script tests moved to `scripts/tests` (#704), `ruff format` adopted and gated (#709), and the PSScriptAnalyzer claim conditioned on repos that carry `.ps1` files (#686). **Conduct rules.** Triage-order and scope guardrails in pr-review-conduct (#684), `pr_review.py wait` requesting a review rather than only polling for one (#685), a tech-agnostic signed-commit verification (#708), execution rather than analogy to verify platform-specific code (#715), and a unique worktree for every task (#717). **Docs.** The fleet map and gap register with peer messaging declared (#687), mermaid flow diagrams in the kept-authority docs (#702), and the map pointed at the shipped diagrams and current tooling (#703). ## Issues this promotion closes Each landed on `develop` on its own pull request. The keyword fires only on a merge into `main`, so it sits here rather than on the feature pull requests. Closes #700 Closes #707 Closes #711 Closes #712 Closes #714 Closes #688 #699 stays open on purpose: #717 shipped the layout convention and the skill, and the physical migration of existing checkouts is still tracked there. ## Review record Every squash closed its own Copilot loop on its own pull request before merging to `develop`. This promotion carries no new content of its own, so its review is the merged tree as a whole. ## Consequence worth stating The `GOVERNANCE.md` and `AGENTS.md` sections these squashes changed become the canonical the moment this reaches `main`, and every carrying repository reads as drifted from that point until it resyncs. That is the ordinary consequence of a canonical moving rather than a defect. The Skills installer added here is also how a machine picks the new skills up, so a session that keeps restating a rule already packaged as a skill is the signal to run it.
Stop Docker Desktop and Update WSL During a Docker Install or Upgrade
#701 checked the WSL platform floor before a docker install or upgrade
and only named the remedy, upgrade-host.ps1 -Wsl. That left docker to
skip outright whenever WSL was behind, since that script also refuses
while Docker Desktop is running. Verifying #701 on a real Windows host
surfaced two more gaps. Docker Desktop's own WSL integration goes
stale across most engine bumps, surfacing as "WSL integration with
distro '' unexpectedly stopped". The in-app "Restart the WSL
integration" button does not clear it. And wsl --update raises its own
UAC prompt, which a headless run cannot answer.
install-tools.ps1: a docker install, upgrade or reinstall nowdrives the whole maintenance window itself. It stops Docker Desktop
through its own CLI (
docker desktop stop/start), runswsl --updatewhere the floor is not met, shuts every WSL distro downwith
wsl --shutdownso each one's integration remounts fresh, andrestarts Docker Desktop once the package itself is settled.
Confirmed first, unless
-Yeswas given. A non-elevated,non-interactive run refuses to start
wsl --updaterather than hangon its UAC prompt, and restores Docker Desktop to how it found it.
The WSL-readiness check moved from an unconditional early gate to
just before real work happens, so an already-current docker no
longer fails a run over an unrelated stale WSL platform.
README.md: documents the moved boundary. WSL used to be strictlyread-only here, naming
upgrade-host.ps1 -Wslas a person's ownstep. A docker version bump already needing the same stop-then-
restart window for its own integration is what moved it.
Verification
Exercised live on a real Windows host, not simulated: a real docker
upgrade (4.85.0 -> 4.86.0) through winget; the WSL-below-floor path
through a real
wsl --updatepast an actual UAC prompt (WSL 2.7.10.0-> 2.7.11.0), re-verifying the floor and restarting Docker Desktop
afterward; the plain engine-bump stop, shutdown, restart cycle; the
declined-prompt and neither-condition-true branches, through mocked
extraction of the real functions; and the headless-UAC-refusal guard,
both tripping unelevated and bypassing elevated correctly.
Invoke-ScriptAnalyzeragainst the repo's own settings, a parsecheck,
pytest scripts/test_bootstrap.py scripts/test_host_gate.py(82 passed), and
host_gate.pyall pass.