Skip to content

fix(installer): stop PS 5.1 stderr wrapping from aborting version probes - #1255

Open
MasterFede5 wants to merge 1 commit into
DeusData:mainfrom
MasterFede5:fix/installer-ps51-stderr-probe
Open

fix(installer): stop PS 5.1 stderr wrapping from aborting version probes#1255
MasterFede5 wants to merge 1 commit into
DeusData:mainfrom
MasterFede5:fix/installer-ps51-stderr-probe

Conversation

@MasterFede5

Copy link
Copy Markdown

What does this PR do?

Fixes a Windows PowerShell 5.1 failure mode in install.ps1: the script sets $ErrorActionPreference = "Stop" globally, and PS 5.1 (the default powershell.exe the README instructs users to run) wraps redirected native stderr in ErrorRecords. The two --version 2>&1 probes (candidate at ~L265, installed binary at ~L288) therefore abort the whole install for any healthy binary that prints a single warning line to stderr while exiting 0 — reporting error: downloaded binary failed to run.

The fix relaxes EAP to Continue only around the two native probe calls, restoring it in a finally block. Failure detection is unchanged: it stays on $LASTEXITCODE.

Reproduction (PS 5.1.26100.8875, powershell.exe -File): stub binary printing a version to stdout + one warning to stderr, exit 0 → previous pattern aborts with the stderr text as the exception; new pattern captures the version output and still detects a nonzero exit (tested with exit 3). EAP is restored to Stop afterwards.

The literal invariants checked by tests/test_windows_bundle_contract.sh (& $DownloadedLauncher --version, launcher-only invocation) are preserved — that contract test passes locally.

Checklist

  • Every commit is signed off (git commit -s) — required, CI rejects
    unsigned commits (DCO, see CONTRIBUTING.md)
  • Tests pass locally (make -f Makefile.cbm test) — not run: no C toolchain on this machine; the change touches only install.ps1. tests/test_windows_bundle_contract.sh (the suite that asserts installer invariants) passes locally.
  • Lint passes (make -f Makefile.cbm lint-ci) — C linters not applicable to this script-only change; install.ps1 parses clean (PSParser, 0 errors).
  • New behavior is covered by a test (reproduce-first for bug fixes) — reproduced first with a stderr-writing stub as described above; the existing bundle contract test still guards the probe invariants. Happy to add a harness test if you point me at the preferred place for installer regression tests.

🤖 Generated with Claude Code

install.ps1 sets $ErrorActionPreference = "Stop" globally, but Windows
PowerShell 5.1 (the default powershell.exe the README instructs users to
run) wraps redirected native stderr lines in ErrorRecords. The two
'--version 2>&1' probes therefore treat any healthy binary that prints a
warning to stderr (exit code 0) as a fatal error and abort the install.

Relax EAP to Continue only around the two native probe calls, restoring
it in a finally block; failure detection remains on $LASTEXITCODE.

Reproduced on PS 5.1.26100.8875 with a stub that prints a version to
stdout, one warning line to stderr, and exits 0: the previous pattern
aborts, the new pattern captures the version and still detects nonzero
exits. tests/test_windows_bundle_contract.sh passes (the literal
'& $DownloadedLauncher --version' invariant is preserved).

Signed-off-by: MasterFede5 <friedrickms@gmail.com>

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: MasterFede5 <friedrickms@gmail.com>
@MasterFede5
MasterFede5 requested a review from DeusData as a code owner July 24, 2026 18:13
@DeusData

Copy link
Copy Markdown
Owner

Thanks for this, and sorry for the delayed acknowledgement. Queued for review.

A +19/-2 single-file change is easy to review — nothing blocking from a size standpoint. CI shows 1-2 failing checks worth checking while it waits.

PowerShell 5.1 wrapping stderr and thereby aborting verification is a nasty class of bug: it makes a working install look broken for reasons that have nothing to do with the install. Good catch.

@DeusData DeusData added this to the 0.9.1-rc milestone Jul 28, 2026
@DeusData DeusData added bug Something isn't working editor/integration Editor compatibility and CLI integration ux/behavior Display bugs, docs, adoption UX windows Windows-specific issues priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker. labels Jul 28, 2026
@DeusData

Copy link
Copy Markdown
Owner

Great catch, and a textbook writeup. The hazard is real and still live on current main — I verified it rather than taking your word for it: install.ps1:8 sets $ErrorActionPreference = "Stop" globally, and both probes run bare underneath it (:259 and :327). PowerShell 5.1 wraps redirected native stderr in ErrorRecords, so under Stop the first stderr line becomes terminating — meaning a healthy binary that prints any warning to stderr while exiting 0 aborts the install with "downloaded binary failed to run".

That is exactly the kind of latent installer bomb that only detonates the day the binary prints its first warning, and with the daemon work adding warning paths it is a plausible one. Your scoped try/finally with detection kept on $LASTEXITCODE is the right conservative shape — the failure path is untouched, and the restore is exception-safe.

Thank you for the pure-ASCII discipline. I byte-scanned the patch and it is clean, comments deliberately avoiding em-dashes. We have been burned by a single non-ASCII character in this exact file before — PowerShell 5.1 decodes a BOM-less .ps1 as ANSI and one em-dash injected a stray quote that made install.ps1 unparseable on main. Not everyone knows that; you clearly did.

Three things needed, all small:

1. A mechanical rebase. Main removed the launcher stub (a54ea95), so $DownloadedLauncher is now $DownloadedBinary and the probes sit at install.ps1:259 and :327. Your fix survives the rename intact. Worth knowing: the literal invariant pinned at tests/test_windows_bundle_contract.sh:206 requires the substring & $DownloadedBinary --version, and the rebased line still contains it since 2>&1 comes after — so that contract stays green.

2. One hardening ask — a narrow false-pass window. With EAP=Continue, a binary that fails to launch (corrupt or wrong-architecture image, as opposed to exiting nonzero) may surface as a non-terminating error, in which case $LASTEXITCODE is not updated. For probe 1 that fails safe: no native command has run yet, so $LASTEXITCODE is $null and the -ne 0 check throws. But probe 2 has prior native calls before it (probe 1 exiting 0, and & $Dest daemon stop), so a stale 0 could false-pass a corrupt install and print "Installed: …" alongside an error record. Pre-fix, EAP=Stop promoted that to terminating and aborted correctly.

Pre-seeding $global:LASTEXITCODE = 1 inside each relaxed block before the call closes it, and costs nothing. If you get the chance on your 5.1 box, it would also be genuinely useful to confirm how a start-failure actually surfaces there — I could not settle that from source alone.

3. Yes please to the test you offered. You mentioned you would add a harness test if pointed at the right place: a stderr-writing stub under the Windows VM win.sh smoke-install/guards family is the right home. Worth saying why it matters — our real-Windows smoke leg runs install.ps1 but with a stderr-quiet binary, so it would not have caught the original hazard either. At minimum, a contract pin on the EAP save/restore block would stop the fix being silently removed later.

With the rebase and the pre-seed this merges. Thanks for a precise, well-scoped fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working editor/integration Editor compatibility and CLI integration priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker. ux/behavior Display bugs, docs, adoption UX windows Windows-specific issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants