Repository navigation
Name Why a Windows Tool Install Failed and Print Docker's Status in UTF-8 - #2811
Merged
Merged
Conversation
…TF-8 A failed winget install printed only winget's exit code, while the reason sat in the installer log winget names and nobody reads. A silent Inno Setup installer such as Git for Windows answers its own files-in-use dialog with Cancel and logs the dialog text, so the run ended in a bare failure with the cause one file away. On a failed install, `Invoke-ToolApply` now prints winget's code in hex as well, reads the matching installer log newer than the attempt through `Get-InstallerFailureReason`, and prints its reason entries with their continuation lines joined. `Get-ToolHolder` now names every process running from the tool's install tree rather than only one named after the tool, which misses a background helper a tool ships, such as the GnuPG key daemon inside Git for Windows. The tree is the child of Program Files, Programs, or WinGet's package directory holding the probe, so it does not depend on which of a tool's copies PATH resolved, and a probe in no such container is matched by name alone. `docker desktop stop` and `start` print a UTF-8 check mark, which a console on its OEM code page renders as three unrelated characters. `Invoke-Utf8Run` switches the console to UTF-8 for those two calls only and restores it after.
… Use The menu runs every task in a child pwsh piped through itself, so a switch inside the child alone could not fix the docker check mark: the child re-encodes its pipe in the code page it started with and the menu decodes it the same way. The menu now switches the console to UTF-8 for its lifetime and restores it on exit, which the child inherits at startup, and `Invoke-Utf8Run` switches only where its own output is not redirected, so a run captured to a file keeps the bytes intact. The failure output moves into `Write-InstallFailure`. It names running processes as the cause only where the installer log says files were in use, and as a possibility only where the log gave no reason, since a Git upgrade failing on a download otherwise blamed every open Git Bash. Log entries are masked for the home folder like the log path, the `Stop-Process` command sits on a line of its own, and the install root now prefers the nested WinGet machine package directory and the per-user Python directory over the container holding them.
…Its Interpreter A download failure runs no installer and leaves no log, so the "no reason" branch blamed every open Git Bash for it, and the in-use match caught any log line naming a process, a burn bundle's "Process returned error" among them. `Write-InstallFailure` now names running processes only where a log entry uses the in-use wording itself or winget's code is a sharing violation or an access denial, which is how a portable package reports a locked file without writing an installer log. Python's probe is the py launcher, which installs in a directory of its own, so the tree search never reached the interpreter that blocks an upgrade. `Get-ToolHolder` takes the managed line's interpreter for python instead. `Invoke-Utf8Run` reads the console through `Test-OutputToConsole`, so the test drives its switching branch again from a harness whose output is always captured.
ptr727
added
the
comments
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
`Get-ToolHolder` read `.Source` off the result of a `Get-Command` that found nothing, which StrictMode turns into a terminating error, so a failed install with in-use evidence for a tool not on this session's PATH aborted the run, and for docker left Docker Desktop stopped. The lookup now reads the source only from a command it found. winget's own INSTALL_PACKAGE_IN_USE and INSTALL_FILE_IN_USE codes now count as in-use evidence beside the sharing violation and access denial codes, and the comment no longer claims an unverified exit code for a portable package.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2811 +/- ##
==========================================
Coverage ? 60.30%
==========================================
Files ? 16
Lines ? 8479
Branches ? 0
==========================================
Hits ? 5113
Misses ? 3366
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The failure-reporting path can still abort or omit holder guidance, and it can recommend stopping unrelated processes.
5 open findings
What changed in this PR
This PR improves Windows host-tool upgrades by explaining failed winget installs and displaying Docker status in UTF-8.
Changes:
- Add installer-log reasons, hexadecimal exit codes, and process-holder guidance to failure reports.
- Switch console encoding for Docker commands and the interactive menu, then restore it.
- Add focused PowerShell-driven tests for the new diagnostics and encoding behavior.
| File | Description |
|---|---|
tests/test_install_tools.py |
Tests failure reporting, log parsing, install roots, and encoding. |
host-setup/windows/install-tools.ps1 |
Adds failure diagnostics and UTF-8 Docker command handling. |
host-setup/menu.ps1 |
Sets UTF-8 console output during the menu session. |
🧠 Review effort: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
… Tree `Get-InstallerFailureReason` capped its entries at the last four before `Write-InstallFailure` looked for in-use wording, so an in-use entry followed by several error or abort lines lost both the reason and the holder report. It now reads whether files were in use from every entry, returns that as `InUse`, and keeps the first in-use entry among those shown. The in-use wording also counts as a reason entry on its own, where before only an entry the error pattern matched could carry it. `Get-ToolHolder` added every process named after the probe even once an install tree was resolved, so a Python upgrade read from the line's interpreter still named py.exe running from the launcher's directory. The name match is now the fallback for a probe with no readable tree.
Owner
Author
|
A recorded local strict-review pass covers head |
A hosted Windows runner's temp directory is an 8.3 short path, and Get-Item expands it, so the container the harness set from that path never matched the file it read back, no tree resolved, and the name fallback found the harness's own pwsh. The harness now reads the directory back through Get-Item before building the container from it. Reproduced locally with an 8.3 temp directory, which fails the old harness and passes this one.
Owner
Author
|
A recorded local strict-review pass covers head |
ptr727
added a commit
that referenced
this pull request
Oct 11, 2026
…ilure Reporting Fixes to Main (#2816) ## Summary Promotes these pull requests from develop to main: - #2832 Require Every CLI Script to Answer Help and Refuse Unknown Arguments - #2833 Exempt nopep8 as a Leading Directive in the Prose Gate - #2815 Bind the Workspace-Edit Rule Only to an Agent Running Inside VS Code - #2813 Record HomeAssistant-Config's Incidental Python as a driftNote - #2827 Read nopep8 in Any Letter Case in the Prose Gate - #2825 Spend a Head Refusal a Later Genuine Round on That Head Answers - #2819 Read noqa and CSpell Directives in Any Letter Case - #2814 Judge the Prose in Front of a Trailing noqa or pylint Directive - #2811 Name Why a Windows Tool Install Failed and Print Docker's Status in UTF-8 - #2808 Read a Tool Name as a Prose Gate Directive Only Where It Opens the Comment - #2802 Name a File the Prose Gate Cannot Decode in Its Scope Line - #2800 Read a First-Line Comment After a Byte Order Mark as Leading in Every Language - #2797 Name a Python File the Prose Gate Cannot Parse in Its Scope Line - #2794 Let the Unattended Orchestrator Retry a Transient Dispatch Failure - #2792 Close the Remaining Blocked-Promotion Gaps in the Unattended Worker - #2790 Read a Carried Change Set Past the Compare Endpoint's 300-File Ceiling - #2784 Let a Round on the Head Outrank pr_review.py wait's Repo-Wide Quota Reading - #2782 Say What pr_review.py wait --min-rounds Counts and Name It at the Timeout - #2778 Share the Bounded Step Reader Across the Test Harnesses - #2776 Pin the Prose Gate's Override Wiring by Structure - #2773 Exempt a List Item's Label Separator from the Dash Rule for Every Marker and Bold Spelling - #2771 Strip a List Item's Opening Label for Every Marker and Bold Spelling in the Prose Gate - #2768 Page pr_review.py's Changed-File List Past 100 So a Large Promotion Can State Coverage - #2765 Let an Attested Local Pass Cover a Head Beside a Table-Less Copilot Round Closes #2285 Closes #2820 Closes #2818 Closes #2809 Closes #2807 Closes #1659 Closes #2796 Closes #2798 Closes #2163 Closes #1984 Closes #2614 Closes #2767 Closes #2781 Closes #2741 Closes #2775 Closes #1665 Closes #2770 Closes #2738 Closes #2758 Closes #2739 🤖 Generated with [Claude Code](https://claude.com/claude-code)
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Two problems surfaced in one run of the menu's "Upgrade the host tools" task on Windows.
A failed install said nothing about why. A Git for Windows upgrade failed with
winget exited -1978335226and no reason. The reason was in the installer log winget names: the silent Inno Setup installer answered its own files-in-use dialog with Cancel because a background GnuPG key daemon, which Git for Windows ships and gpg starts for commit signing, was still running from Git's tree. The old holder check matched only a process named after the tool, so it found nothing.On a failed install,
install-tools.ps1now:0x8A150006), the form winget documents its codes under,Get-InstallerFailureReason) and prints its reason entries, continuation lines joined and the home folder masked,Get-ToolHolder,Get-InstallRoot), with aStop-Processcommand on its own line, but only where a log entry uses the in-use wording or winget's code is a sharing violation or access denial, so an unrelated failure does not blame every open Git Bash. Python's tree is read from the managed line's interpreter rather than from thepylauncher.Docker's status printed as mojibake.
docker desktop stopandstartprint a UTF-8 check mark, which rendered as three code page 437 characters. The menu runs each task in a child pwsh piped through itself, so the fix is inmenu.ps1: the console is switched to UTF-8 for the menu's lifetime and restored on exit, which a child pwsh inherits at startup.Invoke-Utf8Runcovers a direct console run ofinstall-tools.ps1, and leaves redirected output alone so a captured log keeps its bytes intact. A menu-shaped parent and child chain decodedU+0393 U+00A3 U+00F4before the change andU+2713after it.Open findings from the local strict review
The pre-push edit budget was spent with these two still open. Both are
introduced, and both are fixed in a879bd3, which a further local strict review pass found clean.introduced, fixed in a879bd3:Get-ToolHolderreads.Sourceoff$nullwhen the probe is not on PATH, a terminating error under StrictMode that aborts the run and, for docker, skipsExit-DockerMaintenance.introduced, fixed in a879bd3: the in-use code list omits winget's own0x8A150101(package in use) and0x8A150103(file in use).The remaining decimal-only
winget exitedsites in other files are filed as #2810.Test plan
python3 -m unittest tests.test_install_toolson Windows: newTestWindowsInstallFailureReasonandTestWindowsInstallFailureReportcover the log parsing, the log age and id filters, install-root resolution, both branches ofInvoke-Utf8Run, and eachWrite-InstallFailurebranch.🤖 Generated with Claude Code