Skip to content

Add formal MXC E2E validation path#866

Merged
shanselman merged 1 commit into
mainfrom
formal-mxc-validation
Jun 26, 2026
Merged

Add formal MXC E2E validation path#866
shanselman merged 1 commit into
mainfrom
formal-mxc-validation

Conversation

@shanselman

@shanselman shanselman commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add scripts\validate-mxc-e2e.ps1 as the formal local MXC E2E validation path
  • Make the script set the MXC/E2E gates itself and fail if the Gateway MXC proof skips unless -AllowSkip is explicitly requested
  • Document when agents/developers must run the path in AGENTS.md, docs\TEST_COVERAGE.md, and docs\WINDOWS_NODE_TESTING.md

Validation

  • .\scripts\validate-mxc-e2e.ps1 -NoBuild (2 Gateway MXC proof tests passed)
  • .\build.ps1
  • dotnet test .\tests\OpenClaw.Shared.Tests\OpenClaw.Shared.Tests.csproj --no-restore (2514 passed, 31 skipped)
  • dotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore (1215 passed)
  • python .agents\skills\autoreview\scripts\autoreview --mode local --engine copilot --model gpt-5.5 returned clean

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@clawsweeper

clawsweeper Bot commented Jun 26, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed June 26, 2026, 3:44 PM ET / 19:44 UTC.

Summary
Adds scripts/validate-mxc-e2e.ps1 and updates AGENTS/testing docs to make the two Gateway MXC E2E proofs a strict local validation path.

Reproducibility: not applicable. This PR adds a validation path rather than reporting a broken existing behavior. The after-fix behavior is supported by PR-body validation output and source inspection, but I did not execute it because this review is read-only.

Review metrics: 2 noteworthy metrics.

  • Changed surface: 4 files: 1 script added, 3 docs/policy files changed. The PR is focused on validation tooling and repository guidance rather than product runtime behavior.
  • MXC proof assertions: 2 named Gateway MXC proofs asserted. The wrapper hard-codes the success and denied-write proofs that must pass for local MXC merge validation.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
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:

  • [P2] Confirm maintainer acceptance of the new AGENTS.md MXC validation requirement before merge.

Risk before merge

  • [P1] The PR changes AGENTS.md validation policy, so maintainers should explicitly accept making this wrapper the canonical local MXC merge-validation lane.
  • [P1] GitHub reported mergeStateStatus UNSTABLE with several Build and Test jobs still in progress during this read-only review; normal required checks should finish before merge.

Maintainer options:

  1. Accept the formal MXC lane
    Merge once maintainers agree AGENTS.md should require this wrapper for MXC, system.run, Windows node command execution, and related gateway setup work.
  2. Narrow the policy wording
    If maintainers want the script but not a new required lane, revise AGENTS.md and docs to present it as recommended targeted proof instead of required merge validation.
  3. Pause the process change
    Pause or close if the formal validation requirement should wait for a broader repository validation-policy decision.

Next step before merge

  • No automated repair is indicated; the remaining action is maintainer acceptance of the AGENTS.md validation-policy change and normal required-check completion.

Security
Cleared: Cleared: the diff adds a local PowerShell validation wrapper and docs, invokes existing build/test paths, and does not add dependencies, secret access, or runtime security-boundary changes.

Review details

Best possible solution:

Land the wrapper and docs if maintainers want a formal local MXC lane, while keeping hosted CI skip behavior unchanged.

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

Not applicable; this PR adds a validation path rather than reporting a broken existing behavior. The after-fix behavior is supported by PR-body validation output and source inspection, but I did not execute it because this review is read-only.

Is this the best way to solve the issue?

Yes for implementation shape; wrapping the existing Gateway MXC E2E proofs and failing TRX skips is the narrow maintainable path. Maintainers still need to decide whether AGENTS.md should make it required for MXC-adjacent work.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add merge-risk: 🚨 automation: The diff adds a new local validation script and AGENTS.md requirement that future agent/proof workflows may treat as canonical.

Label justifications:

  • P3: This is low-risk validation tooling and documentation/policy work, not an urgent user-facing runtime fix.
  • merge-risk: 🚨 automation: The diff adds a new local validation script and AGENTS.md requirement that future agent/proof workflows may treat as canonical.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit 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 running the new wrapper after the change with 2 Gateway MXC proof tests passed, plus the required build, Shared tests, and Tray tests.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body reports running the new wrapper after the change with 2 Gateway MXC proof tests passed, plus the required build, Shared tests, and Tray tests.
Evidence reviewed

What I checked:

  • PR diff scope: The PR changes 4 files: one new PowerShell validation script and three docs/policy files, with 247 additions and 3 deletions. (ef2c195bfa40)
  • Wrapper drives the intended gates: The added script sets OPENCLAW_REPO_ROOT, OPENCLAW_RUN_E2E, and OPENCLAW_RUN_MXC_E2E, then runs the focused MxcSetupAndConnectTests E2E filter. (scripts/validate-mxc-e2e.ps1:161, ef2c195bfa40)
  • Wrapper fails skipped MXC proofs by default: The script asserts the two named Gateway MXC proofs were reported and passed; NotExecuted or Skipped becomes an error unless -AllowSkip is supplied. (scripts/validate-mxc-e2e.ps1:99, ef2c195bfa40)
  • Policy text added to AGENTS.md: The PR head adds a targeted MXC/system.run validation section that requires running the wrapper for MXC-adjacent work. (AGENTS.md:32, ef2c195bfa40)
  • Underlying MXC proofs exist on current main: Current main defines the success and denied-write Gateway system.run MXC proof methods that the new script asserts by name. (tests/OpenClaw.E2ETests/Setup/MxcSetupAndConnectTests.cs:26, 1c377cb64b65)
  • Current CI keeps hosted-runner skip behavior: Current main already reports the same MXC proof names in the setup-connect shard, but allows NotExecuted/Skipped on hosted runners instead of failing them. (.github/workflows/ci.yml:372, 1c377cb64b65)

Likely related people:

  • shanselman: Current-main history ties this person to the MXC GitHub Actions gating change that this formal local validation path complements. (role: recent adjacent contributor; confidence: high; commits: 1c377cb64b65; files: tests/OpenClaw.E2ETests/E2EFactAttribute.cs, .github/workflows/ci.yml, docs/WINDOWS_NODE_TESTING.md)
  • TheAngryPit: Merged PR history shows this person introduced the Gateway system.run MXC proof methods that the new wrapper asserts. (role: MXC E2E proof introducer; confidence: high; commits: e70868d82bf0; files: tests/OpenClaw.E2ETests/Setup/MxcSetupAndConnectTests.cs, tests/OpenClaw.E2ETests/E2EFactAttribute.cs, docs/WINDOWS_NODE_TESTING.md)
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. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. labels Jun 26, 2026
@shanselman shanselman merged commit 37bdd38 into main Jun 26, 2026
15 checks passed
@shanselman shanselman deleted the formal-mxc-validation branch June 26, 2026 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 automation 🚨 Merging this PR could break CI, automerge, proof capture, label sync, or automation. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. 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.

1 participant