fix(bootstrap-windows): degrade gracefully when winget is missing#3987
fix(bootstrap-windows): degrade gracefully when winget is missing#3987laitingsheng wants to merge 2 commits into
Conversation
) bootstrap-windows.ps1 (from #3744) raised an unhandled PowerShell exception when winget.exe was not on PATH — common on Windows Server images and stripped Windows installs without the App Installer Store package. The user saw a raw stack trace with no clear next step. Replace the `throw` with a friendly Write-Status error message that: * names the cause (winget missing) and where it usually applies (Windows Server / stripped installs), * offers the two viable recovery paths — install App Installer from the Microsoft Store, or download the Docker Desktop installer directly from docker.com, * notes that re-running bootstrap-windows.ps1 will skip the install step once Docker Desktop is present, and * exits the script with code 1 instead of throwing a stack trace. No fallback automatic installer download is added: hardcoding a Docker Desktop installer URL without integrity verification would broaden the bootstrap script's trust surface for marginal gain, and the manual link path is what the bug report's expected behaviour (option b) asks for. Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThe Windows bootstrap now handles a missing ChangesWindows Bootstrap Docker Installation Error Handling
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
E2E Advisor RecommendationRequired E2E: Full advisor summaryE2E Recommendation AdvisorBase: Required E2E
Optional E2E
New E2E recommendations
|
PR Review AdvisorRecommendation: blocked This is an automated advisory review. A human maintainer must make the final merge decision. Limitations: Review used trusted deterministic PR context and the supplied diff; no scripts, tests, package-manager commands, or PR-provided instructions were executed.; CI and E2E results were still pending/in progress for head SHA 9c27405, so final pass/fail status could not be confirmed.; No unresolved review thread state was available beyond GraphQL reviewThreads.nodes=[]; CodeRabbit was still pending.; Linked issue #3972 had no comments in trusted context; acceptance coverage maps the issue body clauses only.; The E2E Advisor comment was present, but the current-head E2E recommendation and wsl-e2e checks were still in progress, so E2E coverage for this SHA is not established. Full advisor summaryPR Review AdvisorBase: Small, targeted Windows bootstrap/docs fix with security-positive installer trust posture, but merge is blocked by pending CI/E2E, GitHub mergeStateStatus=BLOCKED, missing real execution coverage for the missing-winget path, and a same-file docs overlap. Gate status
🔴 Blockers
🟡 Warnings
🔵 Suggestions
Acceptance coverage
Security review
Test / E2E status
✅ What looks good
Review completeness
|
* scripts/bootstrap-windows.ps1: the error message printed when winget is missing told the user to re-run scripts\bootstrap-windows.ps1, but the documented usage (and the bug repro) downloads the script to $env:TEMP\bootstrap-windows.ps1. Print $PSCommandPath so the message names the script the user actually invoked, with a fallback to "this bootstrap script" when $PSCommandPath is unavailable (e.g. when the file is dot-sourced or piped into PowerShell). * docs/get-started/windows-preparation.mdx: the page still described the bootstrap as installing Docker Desktop unconditionally. Add a short paragraph telling the user what to do when the script reports winget is missing (install App Installer from the Store, or download Docker Desktop manually), then rerun. Signed-off-by: Tinson Lai <tinsonl@nvidia.com>
|
🌿 Preview your docs: https://nvidia-preview-pr-3987.docs.buildwithfern.com/nemoclaw |
Summary
scripts/bootstrap-windows.ps1from #3744 raised an unhandled PowerShell exception whenwinget.exewas not on PATH — common on Windows Server images and stripped Windows installs without the App Installer Store package. The user saw a raw stack trace with no clear next step.Replace the
throwat line 362 with a friendlyWrite-Statuserror message that names the cause, offers the two viable recovery paths (install App Installer from the Microsoft Store, or download Docker Desktop manually from docker.com), notes that re-running the script will skip the install step once Docker Desktop is present, and exits with code 1 instead of throwing.No automatic-download fallback is added: hardcoding a Docker Desktop installer URL without integrity verification would broaden the bootstrap script's trust surface for marginal gain, and the manual link path is the expected behaviour (option b) from the bug report.
Related Issue
Fixes #3972
Changes
throw 'winget.exe not found...'withWrite-Status -Level ERROR+ actionable recovery instructions +exit 1.Type of Change
Verification
npx prek run --all-filespassesnpm testpassesmake docsbuilds without warnings (doc changes only)Signed-off-by: Tinson Lai tinsonl@nvidia.com
Summary by CodeRabbit
Bug Fixes
Documentation