Add Gateway system.run MXC runtime E2E proof - #787
Conversation
|
Codex review: found issues before merge. Reviewed June 26, 2026, 7:30 AM ET / 11:30 UTC. Summary Reproducibility: yes. for the docs mismatch by source inspection: the documented command filters only the success test while the same section expects denied-write proof markers. I did not run Windows E2E from this read-only 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:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Land this after a maintainer explicitly accepts the MXC security model and CI proof contract, with the docs filter corrected to run both proof methods. Do we have a high-confidence way to reproduce the issue? Yes for the docs mismatch by source inspection: the documented command filters only the success test while the same section expects denied-write proof markers. I did not run Windows E2E from this read-only Linux checkout. Is this the best way to solve the issue? Yes for the gated E2E proof shape; it is a focused way to preserve the real Gateway path. The runtime Full review comments:
Overall correctness: patch is correct AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 6c56a83f43a6. Label changesLabel changes:
Label justifications:
Evidence reviewedSecurity concerns:
What 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
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f55454a5e
ℹ️ 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".
|
@clawsweeper re-review Addressed the review stack in 1392d01.
Autoreview note: scoped autoreview was rerun on a native WSL clean copy of only this PR diff. It reports one remaining P1 preferring fail-closed-by-default for MXC unavailable. I am intentionally not taking that change because it conflicts with the ClawSweeper request to preserve the fallback path; the strict setting above is the bounded opt-in fail-closed path. Proof run:
|
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Status update after latest push (1763f84):
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review Status update after latest push
Final-head validation on Windows build
The PR body was updated with this final-head proof and current stack contract. |
|
🦞🧹 I asked ClawSweeper to review this item again. |
Give only the setup-connect E2E shard extra time for the combined setup and MXC runtime proof fixtures while leaving the other E2E shards on the existing budget. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Windows wxc-exec rejects filesystem.deniedPaths before launching the sandboxed command. Keep denied paths as the local allow-list filter, but omit the unsupported backend field so AppContainer default-deny can run the command while still stripping parent/exact grants over sensitive paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
b27369a to
5d695ab
Compare
|
@shanselman thanks for the follow-up on the MXC deniedPaths behavior. I pulled the current PR head and validated the exact head after your change. Current PR3 head validated: Local host:
Validation run on that exact head:
Installed-app proof on the same head:
Runtime proof through the real path:
Denied-write proof after the
Additional operational smokes after install:
Safe claim: the current @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Follow-up installed-app runtime proof on the same current PR head. No code changes were made after the previous validation comment. I kept the installed build from the exact PR head and ran a broader live Companion/Windows Node feature matrix to verify the app is operational beyond the PR3-specific Validated head:
Full installed-app live matrix:
Commands/capabilities covered:
MXC-specific runtime proof from the same matrix:
Browser/canvas/screen notes:
Safe claim:
@clawsweeper re-review |
Tracking issue: #784
Stacked after: #786
Summary
This PR adds focused runtime proof for the real Gateway path:
Gateway -> node.invoke -> Windows node -> system.run -> MXC process containment.
It is intentionally stacked after #786. A focused compare for the PR3 delta is:
TheAngryPit/openclaw-windows-node@feature/mxc-26200-sdk-0-7-validation...feature/gateway-system-run-mxc-e2e
What Changed
wxc-exec.exepath before deciding whether to skip, avoiding false skips when the test output itself lacks the MXC helper but the built app/SDK path has it.system.runthroughopenclaw gateway call node.invoke, and verifies MXC diagnostics in the tray log.gateway.nodes.allowCommandsincludessystem.run,system.run.prepare, andsystem.whichbefore invoking the proof path.docs/WINDOWS_NODE_TESTING.md.Validation
Final local validation for PR3 head
020d861e, stacked on PR2 head442de02a, on Windows build26200.8737:.\build.ps1passed without setting$env:OS.141passed,0failed,0skipped.28passed,0failed,0skipped.dotnet build tests\OpenClaw.E2ETests\OpenClaw.E2ETests.csproj -c Debug -r win-x64 --no-restore -p:UseSharedCompilation=falsepassed with0warnings,0errors.OPENCLAW_RUN_E2E=1 dotnet test tests\OpenClaw.E2ETests\OpenClaw.E2ETests.csproj --no-build -c Debug -r win-x64 --filter "FullyQualifiedName~OpenClaw.E2ETests.Setup.MxcSetupAndConnectTests" --logger "trx;LogFileName=OpenClaw.E2ETests.pr3-current-26200-8737.trx"passed:2passed,0failed,0skipped.Installed-App Runtime Proof
The top-of-stack app was built, installed, launched, and tested through the real installed Gateway/Windows Node path on the same Windows build:
%LOCALAPPDATA%\OpenClawTray\OpenClaw.Tray.WinUI.exe.Sha.020d861eb50d605afa400aa16d89e240c9c85c5d.OpenClawGatewayWSL distro,OpenClaw 2026.6.10.0.6.4-tap-pr3-gateway-mxc-e2e-on-pr2-770522b.1.19.system.run: available.Success proof:
OPENCLAW_REAL_APP_MXC_26200_8737_OK_f636bbaf.exitCode=0,timedOut=false, marker present in stdout.executor=mxc-direct-appc,contained=True,wxcExec=<set>,version=0.7.0-alpha,envKeys=[],network={defaultPolicy=block,enforcementMode=capabilities}, finalcontainment=mxc.Denied-write proof:
OPENCLAW_REAL_APP_MXC_26200_8737_DENIED_d1d8822f.%LOCALAPPDATA%\OpenClawTray.exitCode=1,timedOut=false, marker absent from stdout,stderrLen=42.executor=mxc-direct-appc,contained=True, finalcontainment=mxc,stdoutChars=0,stderrChars=42.Scope Limits
Refs #784