Skip to content

fix(mxc): align build and UBR checks with mxc-sdk platform detection - #810

Closed
QQSHI13 wants to merge 2 commits into
openclaw:mainfrom
QQSHI13:fix/mxc-build-numbers
Closed

fix(mxc): align build and UBR checks with mxc-sdk platform detection#810
QQSHI13 wants to merge 2 commits into
openclaw:mainfrom
QQSHI13:fix/mxc-build-numbers

Conversation

@QQSHI13

@QQSHI13 QQSHI13 commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Problem

MxcAvailability.Probe() was using stale build constants that didn't match the @microsoft/mxc-sdk getPlatformSupport() source of truth:

  • Minimum build: C# had 26300 (exact match), SDK uses >= 26100
  • Minimum UBR: C# had 8289, SDK uses 7965
  • Builds >= 26500: C# blocked them, SDK has no UBR minimum for these

This caused false-negative sandbox unavailability on newer Windows Insider builds that the SDK considers fully supported.

Root Cause

The C# code had to reimplement the SDK's getPlatformSupport() because the tray app is a native WinUI 3 application with no bundled Node.js runtime, so it cannot call the TypeScript SDK at runtime. The reimplementation was not kept in sync when the SDK updated its requirements.

Changes

src/OpenClaw.Shared/Mxc/MxcAvailability.cs:

  • Replace exact build match (build != SupportedBuild) with minimum-build check (build < MinBuild)
  • MinBuild = 26100 (was: SupportedBuild = 26300)
  • MinSupportedUbr = 7965 (was: 8289)
  • New UbrRelaxedBuild = 26500 - builds >= this have no UBR minimum
  • Updated doc comment to reflect actual requirements

tests/OpenClaw.Shared.Tests/Mxc/MxcAvailabilityTests.cs:

  • Added test cases for: 26099 (rejected), 26100/7964 (rejected), 26499/7964 (rejected), 26500/0 (accepted), 27000/0 (accepted)
  • Handles null expectedReason for builds that pass
  • Added allowed-build test cases for 26100/7965, 26300/7965, 26500/0, 27000/0

Validation

All MxcAvailability tests pass with the new constants (14 passed, 0 failed).

QQ and others added 2 commits June 23, 2026 15:53
Previously, clicking a session in the Sessions tab or a chat toast
notification always opened the default/main session instead of the
specific session.

Changes:
- Add App.PendingChatSessionKey as a fallback when HubWindow does not
  exist (background notifications, closed HubWindow).
- Thread sessionKey through ToastActivationRouter (Action<string?>).
- Embed sessionKey in chat toast arguments and route it on activation.
- ChatPage (functional): consume pending key from both HubWindow and
  App fallbacks; use !string.IsNullOrEmpty guard to treat empty string
  as null.
- ChatPage (WebView): consume pending key and append &session={key} to
  the navigation URL, or pass it through GatewayChatHelper on initial
  WebView init.
- SessionsPage: write key to CurrentApp.PendingChatSessionKey in
  addition to hub.PendingChatSessionKey.
- Update tests for the new OpenChat Action<string?> contract.

Fixes session routing for both native FunctionalUI and legacy WebView2
chat surfaces.
The MxcAvailability probe used stale build constants (26300 / UBR 8289)
instead of matching the @microsoft/mxc-sdk getPlatformSupport() values
(build >= 26100, UBR >= 7965 for builds 26100-26500, no UBR minimum
for builds >= 26500).

This caused false-negative sandbox unavailability on newer Windows builds
that the SDK considers supported.

Changes:
- Replace exact-equality build check with minimum-build (>= 26100) check
- Align UBR minimum from 8289 to 7965
- Relax UBR requirement for builds >= 26500 (SDK policy)
- Update tests to cover the expanded build range
@clawsweeper

clawsweeper Bot commented Jun 24, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed June 24, 2026, 8:56 AM ET / 12:56 UTC.

Summary
The PR relaxes MxcAvailability Windows build/UBR checks and tests, and also changes ChatPage WebView URL initialization to include a pending session key.

Reproducibility: yes. source-reproducible for the blocking patch defect: current Tray tests explicitly forbid the ChatPage call shape introduced by this PR. The MXC runtime claim itself was not reproduced because this read-only review has no Windows/MXC proof from the contributor.

Review metrics: 3 noteworthy metrics.

  • Changed surface: 3 files, +34/-23. The diff is small enough to repair, but one touched file is unrelated to the stated MXC change.
  • Unrelated ChatPage hunk: 1 non-MXC file changed. That hunk revives session-routing behavior already covered by merged main code and tests.
  • Validation reported: 1 focused test result reported; 3 repo commands required. AGENTS.md requires full build, shared tests, and tray tests before completion, while the PR body reports only focused MXC tests.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
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:

  • Remove the stale ChatPage WebView hunk from this MXC PR.
  • [P1] Align the MXC availability change with the canonical maintainer-approved strategy and add focused tests.
  • [P1] Add redacted Windows/MXC runtime proof plus the AGENTS-required build, shared test, and tray test results.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body reports focused tests only; it does not include after-fix Windows/MXC runtime proof such as redacted terminal output, logs, or artifacts, and contributors should redact private details before posting evidence. 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] Broadening the MXC gate can make the UI and runner treat unproven hosts as sandbox-capable; if runtime MXC fails, system.run can still fall back to uncontained host execution.
  • [P1] The stale ChatPage hunk conflicts with the merged session-routing contract and can cache a session-specific WebView URL, risking wrong-session legacy chat navigation.
  • [P1] The PR body lacks real Windows/MXC proof and does not report the AGENTS-required full build, shared tests, and tray tests.

Maintainer options:

  1. Repair before merge (recommended)
    Remove the stale ChatPage change, align the MXC gate with the chosen maintainer policy, and add redacted Windows runtime proof plus the required validation results.
  2. Approve an explicit table
    Maintainers can intentionally accept a build/UBR table, but should record that support decision and require proof for the sandbox and fallback behavior it changes.
  3. Pause for the canonical MXC stack
    If the broader Windows 25H2/MXC validation stack is the intended path, pause or close this PR after preserving any useful notes there.

Next step before merge

  • [P1] Maintainers need to decide the MXC support/security direction and require contributor-side Windows proof; automation cannot safely supply that environment proof or product decision.

Security
Needs attention: The diff broadens the MXC sandbox availability boundary without real Windows/runtime proof, so security review remains needed before merge.

Review findings

  • [P1] Do not broaden the MXC gate without validated policy — src/OpenClaw.Shared/Mxc/MxcAvailability.cs:115-120
  • [P1] Remove the stale WebView session-key call — src/OpenClaw.Tray.WinUI/Pages/ChatPage.xaml.cs:426
Review details

Best possible solution:

Drop the stale ChatPage hunk and land MXC support through a maintainer-approved, proof-backed policy path with required validation and redacted Windows runtime evidence.

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

Yes, source-reproducible for the blocking patch defect: current Tray tests explicitly forbid the ChatPage call shape introduced by this PR. The MXC runtime claim itself was not reproduced because this read-only review has no Windows/MXC proof from the contributor.

Is this the best way to solve the issue?

No. The narrow maintainable path is to remove the stale ChatPage change and resolve MXC support through the canonical proof-backed policy direction rather than merging a standalone broadened gate.

Full review comments:

  • [P1] Do not broaden the MXC gate without validated policy — src/OpenClaw.Shared/Mxc/MxcAvailability.cs:115-120
    This changes the intentionally narrow build-26300/8289 support gate to a broad >=26100 table without proof that OpenClaw's pinned runtime path is safe on those hosts. Because this gate controls whether system.run is treated as contained and can later fall back uncontained, please remove the relaxation or retarget it to the maintainer-approved MXC policy with Windows runtime proof.
    Confidence: 0.88
  • [P1] Remove the stale WebView session-key call — src/OpenClaw.Tray.WinUI/Pages/ChatPage.xaml.cs:426
    Current main keeps _chatUrl as a base URL and appends a pending session only during navigation; the Tray contract test explicitly forbids passing _pendingWebViewSessionKey in InitializeWebViewAsync. This hunk would fail required Tray tests and can leave legacy WebView chat on a stale session-specific URL.
    Confidence: 0.95

Overall correctness: patch is incorrect
Overall confidence: 0.9

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This is bounded Windows MXC support and legacy WebView routing work with serious merge blockers but no emergency outage shown.
  • merge-risk: 🚨 compatibility: The PR changes which Windows builds are treated as MXC-capable, affecting existing sandbox availability and fallback behavior.
  • merge-risk: 🚨 session-state: The ChatPage hunk can cache a session-specific WebView URL and mis-associate later legacy chat navigation with the wrong session.
  • merge-risk: 🚨 security-boundary: The MXC change broadens a sandbox containment gate without current runtime/probe proof for the claimed policy.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
  • 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 focused tests only; it does not include after-fix Windows/MXC runtime proof such as redacted terminal output, logs, or artifacts, and contributors should redact private details before posting evidence. 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

Security concerns:

  • [medium] Unvalidated sandbox support gate — src/OpenClaw.Shared/Mxc/MxcAvailability.cs:115
    Relaxing MxcAvailability can mark hosts as sandbox-capable before OpenClaw has validated the SDK/runtime behavior for those builds, affecting whether system.run is presented as contained or falls back uncontained.
    Confidence: 0.86

What I checked:

Likely related people:

  • Scott Hanselman: The behavior appears to date to commit f52b829a6f3c752a3d95b8309b4e0bec350fc741, which deliberately restricted the MXC sandbox support gate. (role: support-gate contributor; confidence: high; commits: f52b829a6f3c; files: src/OpenClaw.Shared/Mxc/MxcAvailability.cs, tests/OpenClaw.Shared.Tests/Mxc/MxcAvailabilityTests.cs, src/OpenClaw.Shared/Mxc/MxcCommandRunner.cs)
  • Barbara Kudiess: Git history ties the original AppContainer sandboxing implementation and the related probe-based MXC direction to this person. (role: MXC feature introducer and adjacent implementation proposer; confidence: high; commits: 62533e2901bd, d86a96c33071; files: src/OpenClaw.Shared/Mxc/MxcAvailability.cs, src/OpenClaw.Shared/Mxc/MxcCommandRunner.cs, src/OpenClaw.Shared/Mxc/MxcConfigBuilder.cs)
  • QQSHI13: Merged session-routing history introduced the current ChatPage session-key contract and tests that this PR's stale hunk contradicts. (role: recent chat routing contributor; confidence: high; commits: 6283fb174ead; files: src/OpenClaw.Tray.WinUI/Pages/ChatPage.xaml.cs, tests/OpenClaw.Tray.Tests/AppRefactorContractTests.cs)
  • AlexAlves87: Current blame on MxcAvailability and its tests points to the later test-isolation/refactor commit in this checkout. (role: recent MXC area contributor; confidence: medium; commits: 8bcd0f399abd; files: src/OpenClaw.Shared/Mxc/MxcAvailability.cs, tests/OpenClaw.Shared.Tests/Mxc/MxcAvailabilityTests.cs)
  • TheAngryPit: The related canonical issue and open PRs identify this person with the active Windows 25H2/MXC validation and runtime proof stack. (role: current validation-stack proposer; confidence: medium; commits: 76f9bf39d5c7, 5ac78183cffe; files: src/OpenClaw.Shared/Mxc/MxcAvailability.cs, tests/OpenClaw.E2ETests/Setup/MxcSetupAndConnectTests.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.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 session-state 🚨 Merging this PR could lose, corrupt, stale, or mis-associate session or agent state. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. labels Jun 24, 2026
@QQSHI13 QQSHI13 closed this Jun 24, 2026
@QQSHI13
QQSHI13 deleted the fix/mxc-build-numbers branch June 24, 2026 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. merge-risk: 🚨 session-state 🚨 Merging this PR could lose, corrupt, stale, or mis-associate session or agent state. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant