Use wxc-exec --probe for MXC host support; bump mxc-sdk to 0.7.0 - #776
Merged
shanselman merged 6 commits intoJun 21, 2026
Merged
Conversation
Replace the hardcoded Windows build/UBR support gate (build 26300,
UBR 8289) in MxcAvailability with the native `wxc-exec --probe`
result, so newer Windows builds light up without a code change. The
old table was already producing false negatives on current builds
(e.g. 26632).
- MxcAvailability.Probe resolves wxc-exec, runs `wxc-exec --probe`,
and parses the JSON {tier, needsDaclAugmentation, warnings} to
decide support. Adds a test seam (Probe(logger, probeRunner)), a
pure ParseProbeOutput, and a bounded RunWxcExecProbe.
- RunWxcExecProbe bounds the stdout/stderr drain with the remaining
timeout budget (not just process exit) so a handle-inheriting
descendant cannot hang Probe(); abandoned reads are observed on kill.
- SandboxPage probes off the UI thread and shows a neutral "checking"
state until it resolves; the Windows-vs-setup branch no longer
string-matches reason text.
- Bump @microsoft/mxc-sdk ^0.6.1 -> ^0.7.0 and refresh the lockfile.
- Rename the config wire key appContainer -> processContainer to match
the 0.7.0 schema (old name is a deprecated alias); update goldens.
Validated: build.ps1, Shared tests (2052), Tray tests (958).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Follow-ups to the probe-based availability change, addressing two review concerns. #2 Probe error vs. unsupported host: - ParseProbeOutput now classifies into MxcProbeOutcome {Supported, UnsupportedHost, ProbeError}. A negative exit (our timeout/launch sentinel) or exit 0 with no usable output is a transient ProbeError; a positive non-zero exit is a definitive UnsupportedHost. - MxcAvailability exposes ProbeErrored. NodeService caches definitive verdicts for the process lifetime but re-probes (throttled, 5s) after a transient error, so a momentary glitch self-heals instead of pinning the whole process to uncontained execution. Keeps the issue openclaw#494 fall-back-to-host policy. SandboxPage likewise re-probes an errored cache on next init. #3 Degraded isolation tier: - MxcAvailability exposes IsolationTier, NeedsDaclAugmentation, and a derived IsDegradedContainment (true for appcontainer-dacl, DACL augmentation, or an unrecognized tier). Any non-empty tier is still accepted as contained — refusing would drop the host to fully uncontained, which is strictly worse — but SandboxPage now surfaces a "limited containment" caution. Adds unit coverage for outcome classification, ProbeErrored propagation, and degraded-tier detection. Validated: build.ps1, Shared (2062), Tray (958). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Addresses a GPT-5.5 rubber-duck pass on the probe-error/tier changes. - Fix self-heal bug (#1): DirectAppContainerExecutor took a frozen MxcAvailability snapshot, so a transient startup probe error pinned system.run to uncontained for the executor's lifetime even after the host recovered (the re-probe updated the runner's gate but not the executor, which then threw SandboxUnavailableException on every call). The executor now resolves availability lazily via Func<MxcAvailability>. - Explicit probe status (#3): WxcProbeInvocation carries a WxcProbeStatus {Completed, TimedOut, LaunchFailed} instead of overloading exit code -1 as a sentinel. ParseProbeOutput only inspects the exit code for a Completed run, so a real native (possibly negative) exit code can't be misread as our timeout/launch sentinel. - Serialize probing (#2): NodeService.GetOrProbeMxcAvailability is now guarded by a lock and sets the retry timestamp before probing, so concurrent system.run calls can't spawn a storm of wxc-exec --probe processes during a slow/timeout probe. - SandboxPage (openclaw#7): a transient probe error is no longer mislabeled as "your Windows version doesn't support sandboxing" — it shows "Couldn't verify sandbox availability" with a Retry that re-probes. Added an in-flight guard so refresh can't start overlapping probes. Tests: lazy-resolution recovery (errored -> recovered picks up), status classification (timeout/launch ignore exit code; completed-nonzero = unsupported), updated injected-probe and golden tests. Validated: build.ps1, Shared (2064), Tray (958). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Codex review: needs real behavior proof before merge. Reviewed June 21, 2026, 2:57 AM ET / 06:57 UTC. Summary Reproducibility: yes. from source inspection: current main rejects every Windows build other than 26300 before consulting the native SDK probe. I did not reproduce the Windows host behavior in this Linux checkout. Review metrics: 3 noteworthy metrics.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Land one maintainer-chosen MXC 0.7 availability path with version-aware SDK restore and redacted Windows proof for Do we have a high-confidence way to reproduce the issue? Yes from source inspection: current main rejects every Windows build other than 26300 before consulting the native SDK probe. I did not reproduce the Windows host behavior in this Linux checkout. Is this the best way to solve the issue? No as submitted. Probe-based availability is plausible, but the restore path needs version-awareness, live Windows proof is still missing, and maintainers need to reconcile this with the overlapping explicit validation stack. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 2153c7e7d3a1. Label changesLabel justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
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
Bot
added
rating: 🧂 unranked krab
…page Addresses a dual-model (Opus + Codex) adversarial review of the probe-based availability work. - Never run the blocking wxc-exec --probe (~15s) while holding a lock. GetOrProbeMxcAvailability now starts/joins a single shared in-flight probe Task and waits on it OUTSIDE _mxcAvailabilityLock, so concurrent system.run calls no longer serialize behind the lock and can't spawn a probe storm. The retry window opens only AFTER a probe completes, so a 15s timeout can't immediately permit a back-to-back re-probe (fixes the throttle-collapse where timeout 15s > retry 5s). - Don't probe inside _capabilitiesLock. BuildSystemRunRunner ran the blocking probe while RegisterCapabilities held _capabilitiesLock (the "held briefly" comment was no longer true), stalling capability registration / reconnect. It now logs from a non-blocking PeekMxcAvailability and defers the first real probe to the first system.run (off any lock). - Don't leave the Sandbox page stuck in "Checking...". RefreshAvailabilityAsync now catches an unexpected Probe() fault (synthesizing a probe-errored result so Retry shows) and re-renders in a finally on both the happy and failure paths. Not changed (disputed, needs product/contract decision): a completed non-zero probe exit is still treated as definitive UnsupportedHost. Validated: build.ps1, Shared (2064), Tray (958). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Drop now-obsolete archaeology (specific build/UBR numbers, 'old gate' references) and an inaccurate process-lifetime caching note from the availability/probe comments. No behavior change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
bkudiess
marked this pull request as ready for review
June 18, 2026 18:59
This was referenced Jun 18, 2026
Treat indeterminate probe failures as retryable unless structured probe JSON explicitly reports an unsupported host. Skip availability probing entirely when the sandbox toggle is off. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
We were pinned to
@microsoft/mxc-sdk@0.6.1and gating MXC sandbox availability with a hardcoded Windows build/UBR table (build == 26300 && UBR >= 8289) — the one carrying aTODO: this is all temporary; feature gate this correctly ASAP. That table was already producing false negatives on current Windows builds (verified: build 26632 was wrongly rejected, even thoughwxc-exec --probereports it fully supported at the bestbase-containertier).The SDK already ships the right answer: the native
wxc-exec --probecommand returns{"tier", "needsDaclAugmentation", "warnings"}with exit 0 when the host can run the sandbox. This PR switches to that, bumps the SDK, and adopts the renamed config schema field.What changed
MxcAvailability.Probe()resolveswxc-exec.exe, runswxc-exec --probe, and parses the JSON to decide support — replacing the build/UBR table (and removing the staleTODO+ registry read). So newer Windows builds light up with no code change.@microsoft/mxc-sdk^0.6.1->^0.7.0+ refreshed lockfile.appContainer->processContainerto match the 0.7.0 schema (old name is now a deprecated alias). Updated the config model, builder, executor log, the 4 golden fixtures, and tests.SandboxPageprobes on a background thread (renders a neutral "Checking…" state) and the failure-mode branch no longer string-matches reason text.Robustness (incorporates three review passes: code-review, GPT-5.5 rubber-duck, and a dual-model Opus+Codex adversarial review)
WxcProbeStatus {Completed, TimedOut, LaunchFailed}and outcomesMxcProbeOutcome {Supported, UnsupportedHost, ProbeError}. A transient error (timeout / failed launch / garbled output) is distinguished from a definitive "host unsupported" verdict, and surfaced asMxcAvailability.ProbeErrored.DirectAppContainerExecutorresolves availability lazily per call (a frozen snapshot would pin a startup glitch to uncontained for the executor's lifetime even after recovery).NodeService.GetOrProbeMxcAvailabilityruns the ~15s probe via a single-flight shared task and waits on it outside the availability lock; capability registration uses a non-blocking peek so it never stalls on_capabilitiesLock. The retry window opens only after a probe completes (no back-to-back re-probe when timeout > retry interval).RunWxcExecProbebounds the stdout/stderr drain with the remaining timeout budget (not just process exit), so a handle-inheriting descendant (the SDK ships sandbox daemon/guest helpers) can't hang the probe; abandoned reads are observed on the kill path.IsolationTier/NeedsDaclAugmentation/IsDegradedContainmentare exposed. A weaker tier (appcontainer-dacl) is accepted-but-flagged (the Sandbox page shows a "limited containment" caution) rather than blocked — refusing would drop the host to fully uncontained, which is strictly worse.Validation
./build.ps1— all projects incl. WinUI (0.7.0 restored,wxc-execship-validated)ProbeErrored, degraded-tier, lazy-executor recovery, bounded-probe)🤖 Authored with Copilot CLI.