Uh oh!
There was an error while loading. Please reload this page.
fix(installer): re-land #412 + #413 onto develop (follow-up to #446) - #447
Merged
Conversation
LukasWodka
commented
Jul 27, 2026
ContributorAuthor
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit b5b5db9. Configure here.
…#439) * fix(installer): fail fast on k3d spawn failure; bound the create wait (#412, #426 Windows half) Two fixes to New-K3dCluster's create path, both observed live: 1. Start-Process failure ('%1 is not a valid Win32 application' from a broken k3d.exe) is non-terminating by default, leaving $k3dProc null — and 'while (-not $null.HasExited)' is always true, so the installer spun 'Creating compute environment...' forever over a dead install. The spawn now runs under -ErrorAction Stop in a try/catch that cleans up the temp logs + proxy config and fails with the real exception, the log path, and a remedy. 2. k3d cluster create --wait has no timeout of its own, so a stalled image pull spun the spinner indefinitely. The wait is now bounded (15 min default, TB_CREATE_TIMEOUT_MIN override): on expiry the process is killed, the last stderr lines and the install-log path are printed, and the install fails loudly. Extracted as Wait-ProcessWithDeadline so the deadline/kill path is unit-tested. Closes#412. Windows half of #426 (the bash half — k3d create + helm timeouts in cluster.sh / install-client-helm.sh — follows separately). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(installer): remove the partial cluster when the create wait times out (Bugbot #439) Killing k3d mid --wait skips its own rollback, so the timeout path left a half-created cluster behind — and the next run's reuse path would see serversRunning > 0 and print 'Compute environment already running' over a broken environment. The timeout path now deletes the partial cluster (bounded at 2 min via Wait-ProcessWithDeadline) before failing, and tells the operator the exact manual command if the delete itself fails. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…med failure state (#440) * fix(installer): Docker engine wait — 10 minutes, elapsed progress, named failure state (#413) A first-ever Docker Desktop start on AV-heavy corporate machines routinely needs 5-10 minutes (WSL bootstrap, image unpack). The old 3-minute cap turned a normal cold start into a failed install plus a manual re-paste of the one-liner — observed as a recurring wait-then- re-run loop on hospital installs. - Wait bound: 3 min -> 10 min default, TB_DOCKER_WAIT_MIN override. - After the first minute the spinner shows elapsed minutes and the expected worst case, so the wait doesn't read as a hang. - On expiry the failure names the observed state: Docker Desktop process gone (start it / fix its error window) vs. running with the engine still down (tray-icon guidance) - instead of one generic line. Closes#413 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(installer): exited-Docker path drops the slow-start advice (Bugbot #440) When Docker Desktop's process has exited, 'a first start can be slow' and the TB_DOCKER_WAIT_MIN hint contradict the diagnosis and steer operators toward raising the wait instead of restarting/fixing the crash. The slow-start reassurance + override hint now print only on the engine-still-starting path; the exited path fails with its own start-and-fix remedy. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
LukasWodkaforce-pushed
the
fix/re-land-412-413-develop
branch
from
July 27, 2026 15:24
b5b5db9 to
b219ebfCompareLukasWodka
marked this pull request as ready for review
July 27, 2026 15:24
LukasWodka
commented
Jul 27, 2026
ContributorAuthor
bugbot run |
shujaatTracebloc
approved these changes
Jul 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Re-lands the two stack members whose squash-merges went into already-merged feature branches instead of
develop(the stacked-PR merge-button trap — bases were unprotected):-ErrorAction Stop(no infinite spinner on a null process), create wait bounded at 15 min (TB_CREATE_TIMEOUT_MIN), partial cluster deleted on timeout, credential-bearing proxy config cleaned up on failure.TB_DOCKER_WAIT_MIN), honest elapsed progress, named failure states — with the exited-process path keeping its own remedy (no contradictory slow-start advice).Both are the exact Bugbot-clean squashes cherry-picked from the orphaned branches, on top of #446 (the #410 re-land).
Merge order — read first
Draft on purpose. Merge #446 first (base: develop). I'll then retarget this PR to
developand mark it ready — that also makes thecloses #412 / closes #413keywords effective. Do not merge while the base is the feature branch.Type
Bug fix (re-land)
Test plan
Full Pester suite at this tip: 175 passed, 0 failed, 8 skipped.
gen-manifest.sh --checkclean. Content identical to the already-Bugbot-verified #439/#440 squashes.🤖 Generated with Claude Code
Note
Low Risk
Installer-only PowerShell changes with new env overrides and Pester tests; no auth, data, or production runtime paths affected.
Overview
Hardens the Windows
install-k8s.ps1installer around Docker Desktop and k3d cluster create so slow or broken runs fail with actionable messages instead of hanging or leaving a bad state.Docker engine wait (#413): Default wait rises from ~3 to 10 minutes (
TB_DOCKER_WAIT_MIN). The spinner shows elapsed time after the first minute. On timeout, errors distinguish Docker Desktop exited vs still running but engine not ready, with matching hints (no slow-start advice when the process is dead).k3d create (#412 / #426): New
Wait-ProcessWithDeadlinewraps the create spinner with a 15-minute bound (TB_CREATE_TIMEOUT_MIN), kills the process on expiry, tails k3d logs, and deletes a partial cluster before failing so the next run does not reuse a half-created environment.Start-Processuses-ErrorAction Stopso a failed spawn errors immediately instead of spinning on a null process.Pester coverage and
scripts/manifest.sha256are updated for the changed script.Reviewed by Cursor Bugbot for commit b219ebf. Bugbot is set up for automated code reviews on this repo. Configure here.