fix: keep winget output out of Inno path

[steipete]

Summary

• keep winget stdout/stderr visible without returning it from Resolve-InnoCompiler
• capture the native exit code before PowerShell runs another command
• pin the first-run install contract with installer build assertions

Validation

• Windows 11 ARM64 VM: InstallerIssAssertionTests 12/12 passed
• AutoReview: clean; no accepted/actionable findings

Found while building a fresh side-by-side dev installer for the onboarding VM pass.

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed July 6, 2026, 5:55 AM ET / 09:55 UTC.

Summary
The PR pipes winget install stdout/stderr through Out-Host, captures $LASTEXITCODE immediately, and adds installer-build source assertions for that contract.

Reproducibility: yes. from source inspection and a PowerShell semantics check: native command output inside the current function can be captured as return output when assigned to $iscc. I did not run the exact Windows winget installer flow in this read-only review.

Review metrics: none identified.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

• [P1] Add redacted live output, a terminal screenshot, or a log showing scripts/build-inno-local.ps1 -InstallInno keeps winget output visible while Using ISCC: contains only the compiler path.
• [P1] Report the required ./build.ps1, shared test, and tray test results, or state the exact Windows prerequisite blocker.

Proof guidance:

• [P1] Needs real behavior proof before merge: The PR body reports tests but no after-fix real run; add a redacted terminal screenshot, copied live output, or log from current head, and updating the PR body should trigger a fresh review or a maintainer can comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

• [P1] External-PR real behavior proof is missing: the body does not show a current-head -InstallInno run where winget output remains visible while Resolve-InnoCompiler returns only the Inno compiler path.
• [P1] Repository-required local validation is not fully reported in the PR body; it lists focused installer assertions and AutoReview, but not ./build.ps1, shared tests, and tray tests required by AGENTS.md.

Maintainer options:

1. Decide the mitigation before merge
   Merge the narrow installer-script fix after the contributor adds redacted current-head terminal output or logs for the -InstallInno path and reports the required repository validation.
2. Pause or close
   Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

• [P1] No automated repair lane is appropriate because the remaining blocker is contributor-provided real behavior proof and validation reporting, not a concrete code defect.

Security
Cleared: The diff changes only output handling for an existing local winget installer helper and a source assertion test, with no new dependency, secret, permission, workflow, or package-resolution surface.

Review details

Best possible solution:

Merge the narrow installer-script fix after the contributor adds redacted current-head terminal output or logs for the -InstallInno path and reports the required repository validation.

Do we have a high-confidence way to reproduce the issue?

Yes from source inspection and a PowerShell semantics check: native command output inside the current function can be captured as return output when assigned to $iscc. I did not run the exact Windows winget installer flow in this read-only review.

Is this the best way to solve the issue?

Yes, the proposed implementation is the narrowest maintainable fix for the bug: host the native output and capture the native exit code before recursion returns the compiler path. Merge readiness still depends on contributor proof and required validation reporting.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 2c720251b293.

Label changes

Label changes:

• add P2: The PR addresses a focused installer-build failure mode with limited blast radius and no evidence of a broad runtime outage.
• add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
• add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body reports tests but no after-fix real run; add a redacted terminal screenshot, copied live output, or log from current head, and updating the PR body should trigger a fresh review or a maintainer can comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

• P2: The PR addresses a focused installer-build failure mode with limited blast radius and no evidence of a broad runtime outage.
• rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body reports tests but no after-fix real run; add a redacted terminal screenshot, copied live output, or log from current head, and updating the PR body should trigger a fresh review or a maintainer can comment @clawsweeper re-review. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence reviewed

What I checked:

• Repository policy read: AGENTS.md was read fully and its validation/proof guidance applies because this PR touches installer/setup validation; it requires full build, shared tests, tray tests, and real PR proof for setup-adjacent work. (AGENTS.md:1, 2c720251b293)
• Current main still has the bug-prone path: Resolve-InnoCompiler on current main directly invokes winget install inside a function whose return value is assigned to $iscc, so native success output can become part of the function output alongside the compiler path. (scripts/build-inno-local.ps1:69, 2c720251b293)
• PR diff is narrow: The PR changes only the winget install invocation to 2>&1 | Out-Host, stores $wingetExitCode = $LASTEXITCODE, and checks that stored value before resolving the compiler again. (scripts/build-inno-local.ps1:69, b4863a58bfff)
• Assertion coverage added: The PR adds source assertions that pin both 2>&1 | Out-Host and immediate $LASTEXITCODE capture in the installer assertion test. (tests/OpenClaw.Tray.Tests/InstallerIssAssertionTests.cs:166, b4863a58bfff)
• PowerShell behavior sanity check: A read-only PowerShell simulation showed native output becomes function output in the current shape, while piping through Out-Host keeps the native output visible and preserves the captured native exit code.
• Feature history: git blame shows the current Resolve-InnoCompiler implementation, including the unpiped winget install, on commit 4166e0f; git log -S finds the same commit for that winget command. (scripts/build-inno-local.ps1:69, 4166e0fd63f8)

Likely related people:

• shanselman: Git blame and git log -S tie the current Resolve-InnoCompiler and winget install implementation to commit 4166e0f, and GitHub metadata maps the related PR author/merger to shanselman. (role: introduced behavior; confidence: high; commits: 4166e0fd63f8; files: scripts/build-inno-local.ps1)
• karkarl: The side-by-side installer assertion test and related installer script changes on current main are attributed to commit dae64b6, whose related PR metadata maps to karkarl. (role: recent area contributor; confidence: high; commits: dae64b6c49e9; files: scripts/build-inno-local.ps1, tests/OpenClaw.Tray.Tests/InstallerIssAssertionTests.cs)
• steipete: GitHub metadata for feat: add dev/release side-by-side app identity #922 shows steipete authored follow-up commits and merged the side-by-side installer identity work that this PR builds on, so this is not based only on authorship of the current PR. (role: recent adjacent contributor and merger; confidence: medium; commits: 6c922fca0637, ce5983ea4512, dae64b6c49e9; files: scripts/build-inno-local.ps1, tests/OpenClaw.Tray.Tests/InstallerIssAssertionTests.cs)

What the crustacean ranks mean

• 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
• 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
• 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
• 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
• 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
• 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
• 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b4863a58bf

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Avoid redirecting winget stderr under Windows PowerShell

When this helper is run with -InstallInno from the default Windows PowerShell 5.1 host, merging native stderr with 2>&1 is still affected by the script's $ErrorActionPreference = "Stop"; Microsoft's native-command docs note that this was changed only beginning in PowerShell 7.2 (https://learn.microsoft.com/powershell/scripting/learn/shell/running-commands#handling-output-and-errors). If winget emits any non-fatal diagnostic on stderr before exiting 0, the script can terminate here before $wingetExitCode is captured or ISCC is resolved, breaking the first-run installer path this change is meant to fix. Keep stderr on the native error stream, or temporarily relax the preference while forwarding it.

Useful? React with 👍 / 👎.