Skip to content

Fix locale-sensitive Windows tests and technical token formatting - #785

Merged
shanselman merged 4 commits into
openclaw:mainfrom
TheAngryPit:feature/locale-safe-windows-tests
Jun 21, 2026
Merged

Fix locale-sensitive Windows tests and technical token formatting#785
shanselman merged 4 commits into
openclaw:mainfrom
TheAngryPit:feature/locale-safe-windows-tests

Conversation

@TheAngryPit

@TheAngryPit TheAngryPit commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Tracking issue: #784

Summary

This PR makes a small set of Windows test/formatting paths safe on localized Windows installations:

  • Compare sandbox Documents, Desktop, and Downloads grants against OS-provided known-folder paths instead of English folder suffixes.
  • Format compact technical token counts with InvariantCulture, so regional decimal separators do not change test-facing output.
  • Serialize tests that mutate AppVersionInfo.TestOverride, including WindowsNodeClientTests, to avoid parallel shared-state races.
  • Mark shared test projects explicitly so local/CI validation does not silently no-op.

This is PR1 in the Windows 25H2 / MXC validation tracking issue. It is independent from the MXC runtime enablement PR and can be reviewed separately.

Why

On a Portuguese Windows setup, known folders can resolve to localized or redirected locations such as Documentos and Ambiente de Trabalho. The previous tests assumed English path suffixes like Documents, Desktop, or Downloads, which makes otherwise correct behavior look broken on localized Windows.

The compact token count formatter also used current-culture numeric formatting. That can produce comma decimals such as 15,3K in locales where the technical/test contract expects stable dot-decimal output.

Changed Files

  • src/OpenClaw.Tray.WinUI/Chat/ChatUsageFormatter.cs
  • tests/Directory.Build.props
  • tests/OpenClaw.Shared.Tests/AppVersionInfoTestCollection.cs
  • tests/OpenClaw.Shared.Tests/AppVersionInfoTests.cs
  • tests/OpenClaw.Shared.Tests/Mxc/MxcPolicyBuilderTests.cs
  • tests/OpenClaw.Shared.Tests/WindowsNodeClientTests.cs

Validation

Current-head validation was repeated for commit 0555bfa52c0de1d6b88055260d24995d2c2b7782 on Windows Release Preview 25H2 build 26200.8728, Portuguese Windows locale:

  • ./build.ps1: passed; all builds succeeded.
  • dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore: passed; 2262 passed, 29 skipped, 0 failed.
  • dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore: passed; 1088 passed, 0 skipped, 0 failed.
  • openclaw-autoreview --mode branch --base upstream/main --engine codex --model gpt-5.5 --thinking high: clean; no accepted/actionable findings; overall patch is correct (0.86).

Validation note: this was run in a fresh worktree at the exact PR head. The first --no-restore test command hit the repository-documented first-run project.assets.json gotcha, so the test projects were restored and the full required validation sequence was repeated.

Notes

This PR intentionally does not change MXC availability, system.run, Gateway pairing, WSL Gateway setup, release versioning, build automation, validation helper scripts, or installed Companion behavior.

The MXC runtime and Gateway proof are split into #786 and #787. This PR remains only the locale/test baseline and intentionally does not claim MXC runtime behavior.

@clawsweeper

clawsweeper Bot commented Jun 18, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed June 20, 2026, 4:08 PM ET / 20:08 UTC.

Summary
The PR makes compact chat token counts culture-invariant, updates localized Windows known-folder tests to compare OS-resolved paths, serializes AppVersionInfo-mutating tests, and marks test projects explicitly.

Reproducibility: yes. Source inspection shows current main uses culture-sensitive count formatting and English suffix assertions for localized Windows folders, and the contributor supplied current-head Portuguese Windows validation.

Review metrics: 2 noteworthy metrics.

  • Changed surface: 6 files, +56/-8. The actual merge result is small and limited to one formatter file plus tests/test metadata.
  • Required proof commands: 3 reported passed. The contributor reports the AGENTS-required build, Shared.Tests, and Tray.Tests all passing at the current PR head on the affected locale setup.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #784
Summary: This PR is the candidate fix for the locale/test baseline slice of the Windows 25H2/MXC validation tracker; the sibling PRs cover adjacent runtime-policy and Gateway-proof slices.

Members:

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

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

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

Risk before merge

Maintainer options:

  1. Decide the mitigation before merge
    Land this narrowed locale/test baseline if maintainer review agrees, then rebase and review the MXC runtime-policy and Gateway-proof follow-up PRs separately.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • No automated repair is needed; the remaining action is maintainer review and merge-order coordination with the linked follow-up PRs.

Security
Cleared: The diff touches formatter logic, tests, and test project metadata only; I found no concrete security or supply-chain concern.

Review details

Best possible solution:

Land this narrowed locale/test baseline if maintainer review agrees, then rebase and review the MXC runtime-policy and Gateway-proof follow-up PRs separately.

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

Yes. Source inspection shows current main uses culture-sensitive count formatting and English suffix assertions for localized Windows folders, and the contributor supplied current-head Portuguese Windows validation.

Is this the best way to solve the issue?

Yes. The narrowed PR fixes the behavior at the right boundary by using invariant formatting, OS-resolved folder expectations, and test serialization without changing MXC runtime policy.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦞 diamond lobster, so this older rating label is no longer current.

Label justifications:

  • P2: This is a normal-priority Windows locale/test reliability and technical formatting fix with limited production blast radius.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body and latest comment provide current-head Windows 25H2 Portuguese-locale build/test/autoreview output with pass counts for the changed behavior.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body and latest comment provide current-head Windows 25H2 Portuguese-locale build/test/autoreview output with pass counts for the changed behavior.
Evidence reviewed

What I checked:

Likely related people:

  • the99missedcalls: git blame points the compact token formatter changed by this PR to the chat usage footer feature commit. (role: introduced formatter behavior; confidence: high; commits: ff38d8499c48; files: src/OpenClaw.Tray.WinUI/Chat/ChatUsageFormatter.cs, tests/OpenClaw.Tray.Tests/ChatUsageFormatterTests.cs)
  • AlexAlves87: git blame/log point the current MXC policy builder/tests, AppVersionInfo tests, and shared test props to this area history. (role: shared-test and MXC policy test area contributor; confidence: high; commits: b175439fbfd4; files: src/OpenClaw.Shared/Mxc/MxcPolicyBuilder.cs, tests/OpenClaw.Shared.Tests/Mxc/MxcPolicyBuilderTests.cs, tests/OpenClaw.Shared.Tests/AppVersionInfoTests.cs)
  • Ranjesh: git log shows recent gateway connection and pairing hardening in WindowsNodeClientTests, adjacent to the unsafe request-id assertion adjusted here. (role: recent WindowsNodeClient test contributor; confidence: medium; commits: ea36b12f9e4c; files: tests/OpenClaw.Shared.Tests/WindowsNodeClientTests.cs, src/OpenClaw.Shared/WindowsNodeClient.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.

@TheAngryPit
TheAngryPit marked this pull request as ready for review June 18, 2026 18:57
@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 18, 2026
@TheAngryPit

Copy link
Copy Markdown
Contributor Author

Status update after latest push (debafff):

  • Fixed the origin/main zero-test/no-op risk by marking shared test projects explicitly with IsTestProject=true.
  • Fixed the brittle unsafe request-id assertion by checking the full unsafe id (
    eq-1 && bad) instead of the incidental substring �ad.
  • Local CI-equivalent proof: Shared.Tests executed 2262/passed 2262; Tray.Tests executed 1088/passed 1088.
  • Autoreview rerun: clean, no accepted/actionable findings.
  • Review threads: none unresolved.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 19, 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.

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed 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. labels Jun 20, 2026
@TheAngryPit

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Current-head validation was repeated for 0555bfa52c0de1d6b88055260d24995d2c2b7782.

Scope:

  • Fresh detached worktree at the exact PR head.
  • Windows Release Preview 25H2 build 26200.8728.
  • Portuguese Windows locale.
  • No code changes in this pass; this is proof refresh for the current head.

Validation:

  • ./build.ps1 -> passed; all builds succeeded.
  • dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore -> passed; 2262 passed, 29 skipped, 0 failed.
  • dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore -> passed; 1088 passed, 0 skipped, 0 failed.

Note:

  • The first Shared.Tests --no-restore run in the fresh worktree hit the repository-documented first-run project.assets.json gotcha.
  • I restored the test projects and then repeated the full AGENTS-required sequence from the beginning: ./build.ps1, Shared.Tests, Tray.Tests.
  • No private local paths, tokens, or endpoints are included in this proof summary.

@clawsweeper

clawsweeper Bot commented Jun 20, 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.

@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. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jun 20, 2026
@clawsweeper clawsweeper Bot added status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. and removed status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jun 20, 2026
@TheAngryPit

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Correction and current-head proof for PR1.

I previously posted a re-review request for the stale PR1 head 0555bfa52c0de1d6b88055260d24995d2c2b7782. That was wrong because the real local PR1 base for the stacked PR2/PR3 branches also included the later validation-tooling commits. PR1 has now been pushed to the correct head:

a81719ba52d5f1b1cfce39f8fd64a446b202c230

What changed since 0555bfa:

  • Hardened build.ps1 Windows/architecture/SDK detection for PowerShell hosts where $env:OS or $env:PROCESSOR_ARCHITECTURE is absent.
  • Added scripts/Test-WindowsLocal.ps1, a local Windows validation lane that can run from a short subst path for deep checkouts.
  • Kept GitVersion repository trust process-local during validation instead of mutating global Git config.
  • Documented the local Windows validation lane in docs/TEST_COVERAGE.md.

Current-head validation on Windows Release Preview 25H2 build 26200.8728:

  • ./build.ps1 -> passed; all builds succeeded.
  • dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore -> passed; 2262 passed, 29 skipped, 0 failed.
  • dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore -> passed; 1088 passed, 0 skipped, 0 failed.
  • openclaw-autoreview --mode branch --base upstream/main --engine codex --model gpt-5.5 --thinking high -> clean; no accepted/actionable findings; overall patch is correct (0.82).

Safe claim:

@clawsweeper

clawsweeper Bot commented Jun 20, 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.

@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. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 20, 2026
@TheAngryPit
TheAngryPit force-pushed the feature/locale-safe-windows-tests branch from a81719b to 0555bfa Compare June 20, 2026 19:51
@TheAngryPit

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

PR1 was narrowed again in response to the previous review's merge-risk note.

What changed:

  • Removed the local validation-helper/build-automation scope from PR1.
  • PR1 now contains only the locale/test/formatter baseline:
    • invariant compact token formatting
    • OS-resolved known-folder test assertions
    • AppVersionInfo shared-state test serialization
    • explicit shared test-project marking
  • The dropped validation-helper work is preserved only locally for now and is not part of this PR.

Current PR1 head:

0555bfa52c0de1d6b88055260d24995d2c2b7782

Current-head validation on Windows Release Preview 25H2 build 26200.8728:

  • ./build.ps1 -> passed; all builds succeeded.
  • dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore -> passed; 2262 passed, 29 skipped, 0 failed.
  • dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore -> passed; 1088 passed, 0 skipped, 0 failed.
  • openclaw-autoreview --mode branch --base upstream/main --engine codex --model gpt-5.5 --thinking high -> clean; no accepted/actionable findings; overall patch is correct (0.86).

Safe claim:

@clawsweeper clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jun 20, 2026
@shanselman
shanselman merged commit 0faa821 into openclaw:main Jun 21, 2026
29 of 32 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: 🦞 diamond lobster Very strong PR readiness with only minor 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.

2 participants