Skip to content

Fix Windows build detection without OS env var - #822

Merged
shanselman merged 1 commit into
openclaw:mainfrom
TheAngryPit:tap/build-windows-detection-envless
Jun 25, 2026
Merged

Fix Windows build detection without OS env var#822
shanselman merged 1 commit into
openclaw:mainfrom
TheAngryPit:tap/build-windows-detection-envless

Conversation

@TheAngryPit

Copy link
Copy Markdown
Contributor

Summary

Fixes #821.

build.ps1 no longer relies on the mutable OS=Windows_NT environment variable to decide whether it is running on Windows.

The script now uses:

  • PowerShell's $IsWindows automatic variable when available.
  • [System.Environment]::OSVersion.Platform as a Windows PowerShell-compatible fallback.

Why

On a Windows Insider/preview host (25H2, build 26200.8737), a PowerShell 7 process can run without $env:OS populated. In that case the previous check rejected a real Windows host with:

This project requires Windows

Validation

On Windows 25H2 build 26200.8737:

  • ./build.ps1 -CheckOnly with $env:OS = $null passed and reported Windows detected.
  • ./build.ps1 with $env:OS = $null passed.
  • dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore passed: 2417 passed, 29 skipped.
  • dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore passed: 1154 passed.
  • openclaw-autoreview on the one-file diff reported no accepted/actionable findings.

Notes:

  • In this fresh linked worktree, the first --no-restore test attempt hit the repository-documented first-run asset gotcha. I ran the same test projects once with restore to materialize assets, then reran the canonical --no-restore commands above.

@clawsweeper

clawsweeper Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed June 25, 2026, 10:05 AM ET / 14:05 UTC.

Summary
The PR adds a Test-WindowsHost helper to build.ps1 using $IsWindows or .NET OSVersion.Platform, then uses it for the prerequisite OS check.

Reproducibility: yes. from source inspection: current main checks only $env:OS, so a Windows PowerShell session with $env:OS = $null reaches the unsupported-host branch before other prerequisites run. I did not execute the Windows reproduction in this read-only Linux checkout.

Review metrics: 1 noteworthy metric.

  • Build script surface: 1 file changed, 10 added, 1 removed. The patch is confined to prerequisite host detection, so maintainers can review compatibility rather than broad product behavior.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #821
Summary: This PR is the candidate fix for the paired build-script OS detection issue; no separate canonical implementation was found.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

  • none.

Risk before merge

  • [P2] The GitHub setup-connect E2E shard is currently failing, though the contributor's local matching run at the exact head points to a WSL startup/probe timeout rather than this one-file build-script change.
  • [P1] This read-only review did not run the Windows build/test commands because doing so would create artifacts; it relies on the PR body, follow-up comment, source inspection, and GitHub check metadata.

Maintainer options:

  1. Decide the mitigation before merge
    Land this focused build.ps1 host-detection fix after normal PR checks are green or the setup-connect runner timeout is accepted as unrelated; the linked issue should close through the merge.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P2] No repair lane is needed; maintainers should review the focused PR and resolve or rerun the setup-connect E2E check before merge.

Security
Cleared: The diff only changes local build host detection and adds no dependencies, permissions, downloads, secrets, or external code execution paths.

Review details

Best possible solution:

Land this focused build.ps1 host-detection fix after normal PR checks are green or the setup-connect runner timeout is accepted as unrelated; the linked issue should close through the merge.

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

Yes, from source inspection: current main checks only $env:OS, so a Windows PowerShell session with $env:OS = $null reaches the unsupported-host branch before other prerequisites run. I did not execute the Windows reproduction in this read-only Linux checkout.

Is this the best way to solve the issue?

Yes; using PowerShell's $IsWindows when available and .NET OSVersion.Platform as a fallback is narrow, Windows PowerShell-compatible, and preserves the non-Windows fail-fast behavior.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 4e7982bafb86.

Label changes

Label justifications:

  • P2: This fixes a real local build failure path with limited blast radius and no runtime, data, auth, or message-delivery impact.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body reports real Windows 25H2 build.ps1 -CheckOnly and full build runs with $env:OS unset plus shared/tray tests, and the follow-up comment adds local E2E evidence at the exact head.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body reports real Windows 25H2 build.ps1 -CheckOnly and full build runs with $env:OS unset plus shared/tray tests, and the follow-up comment adds local E2E evidence at the exact head.
Evidence reviewed

What I checked:

  • Repository policy read: Full AGENTS.md was read; its validation expectations inform the proof assessment, but this ClawSweeper review is read-only and did not run artifact-producing build/test commands. (AGENTS.md:1, 4e7982bafb86)
  • Current main still has reported behavior: Current main checks $env:OS -ne "Windows_NT" and exits with This project requires Windows, matching the source-reproducible failure when that environment variable is absent. (build.ps1:146, 4e7982bafb86)
  • Latest release still has reported behavior: v0.6.3 points at 85445c78066b2c4b5927e1e72a9a376f443ae8f4, and its build.ps1 contains the same $env:OS prerequisite check. (build.ps1:146, 85445c78066b)
  • PR diff is narrow: The PR changes only build.ps1, adding Test-WindowsHost and replacing the single $env:OS prerequisite branch with that helper. (build.ps1:54, 4337fe742586)
  • Proof and checks context: GitHub metadata shows the PR has sufficient proof labels, reports real Windows 25H2 build/test validation in the body, and includes a contributor follow-up explaining the one failing setup-connect CI shard as a WSL runner startup timeout after local matching E2E passed. (4337fe742586)
  • Linked issue is paired with this PR: The related issue reports the same $env:OS = $null failure and GitHub lists this PR as the closing pull request reference.

Likely related people:

  • Scott Hanselman: git log --all -S '$env:OS' identifies the original build.ps1 prerequisite detection commit, and git log --follow shows later build-script hardening by the same author. (role: introduced behavior and build-script area contributor; confidence: high; commits: 5f18e7b530a6, 782153f0313a; files: build.ps1)
  • Vincent Koc: The most recent focused build.ps1 commit changed prerequisite/build output handling in the same script. (role: recent adjacent contributor; confidence: medium; commits: 0e61fa287afb; files: build.ps1)
  • Régis Brid: Current-line blame in this grafted checkout attributes the visible prerequisite block to the broad current-main copy commit, though the more specific build-script history points earlier. (role: current-main provenance signal; confidence: low; commits: 6623cb8410a4; files: build.ps1)
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.

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal priority bug or improvement with limited blast radius. labels Jun 25, 2026
@TheAngryPit

TheAngryPit commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Local follow-up on the single failing CI shard at the exact PR head 4337fe7.

I reproduced the same setup-connect E2E lane locally, matching the workflow shape:

  • dotnet restore
  • dotnet build src/OpenClaw.Shared -c Debug --no-restore
  • dotnet build src/OpenClaw.SetupEngine -c Debug --no-restore
  • dotnet build src/OpenClaw.Tray.WinUI -c Debug -r win-x64
  • dotnet build tests/OpenClaw.E2ETests -c Debug -r win-x64
  • OPENCLAW_RUN_E2E=1 dotnet test tests/OpenClaw.E2ETests --no-build -c Debug -r win-x64 --filter "FullyQualifiedName~OpenClaw.E2ETests.Setup.SetupAndConnectTests"

Result: passed locally, 15/15 tests.

Environment used for the local run:

  • Windows 25H2 / build 26200.8737
  • WSL 2.7.3.0
  • .NET SDK 10.0.301

The failing GitHub shard appears to fail before any individual setup/connect assertion runs. The downloaded CI artifact shows wsl-create timing out while probing a freshly installed WSL distro:

ext Fresh WSL distro OpenClawE2E-1f2d6885 could not run a root verification command: no output

The equivalent local step completed successfully:

ext wsl-create -> Success Created clean WSL2 distro OpenClawE2E-886e6718

Local teardown also completed successfully and left no OpenClawE2E-* distro registered.

So the current evidence suggests the red setup-connect shard is a GitHub runner WSL startup/probe timeout rather than a regression from this one-file �uild.ps1 change.

@clawsweeper

clawsweeper Bot commented Jun 25, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@shanselman
shanselman merged commit 7a7f39d into openclaw:main Jun 25, 2026
18 of 19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

build.ps1 should detect Windows without requiring OS=Windows_NT

2 participants