Skip to content

[WIP][Don't merge] Plan for End to End testing using crabbox - #1291

Open
larroy wants to merge 4 commits into
openclaw:mainfrom
larroy:dev/plarroy/gui-e2e-plan
Open

larroy wants to merge 4 commits into
openclaw:mainfrom
larroy:dev/plarroy/gui-e2e-plan

Conversation

@larroy

@larroy larroy commented Sep 2, 2026

Copy link
Copy Markdown
Contributor
Additional instructions

MUST: Keep Allow edits from maintainers enabled for this PR so maintainers
can help update the branch when needed.

What Problem This Solves

Initial design for E2E testing using crabbox.

Why This Change Was Made

User Impact

Evidence

Change Type

  • Bug fix
  • Feature
  • Refactor
  • Docs or instructions
  • Tests or validation
  • Security hardening
  • Chore or infrastructure

Scope

  • Tray or WinUI UX
  • Windows node capability
  • Local MCP or winnode
  • Gateway, connection, or pairing
  • Setup or onboarding
  • Permissions, privacy, or security
  • Tests, CI, or docs

Required proof pools

Validation

Real Behavior Proof

  • Environment tested:
  • PR head or commit tested:
  • Exact steps or command run:
  • Evidence after fix:
  • Observed result:
  • Screenshot or artifact links verified? (Yes/No/N/A)
  • Not verified or blocked:

Security Impact

  • New permissions or capabilities? (Yes/No)
  • Secrets or tokens handling changed? (Yes/No)
  • New or changed network calls? (Yes/No)
  • Command or tool execution surface changed? (Yes/No)
  • Data access scope changed? (Yes/No)
  • If any answer is Yes, explain the risk and mitigation:

Compatibility and Migration

  • Backward compatible? (Yes/No)
  • Config or environment changes? (Yes/No)
  • Migration needed? (Yes/No)
  • If yes, list the exact upgrade steps:

Review Conversations

  • I replied to or resolved every bot review conversation addressed by this PR.
  • I left unresolved only conversations that still need maintainer judgment.

joelagnel and others added 4 commits September 1, 2026 15:24
…ith measured context profiles

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Proposes tests/OpenClaw.GuiE2ETests built on FlaUI with a fake gateway tier
and a real WSL gateway tier, a scenario catalog with parity tests, Crabbox
orchestration into the interactive desktop session, CI wiring, proof-pool
integration and a phased rollout.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…kill stub

Verified reuse points, app env hooks, fake gateway wire contract, repo build
conventions and AutomationId inventory for implementers; a scoped Phase 0
spike prompt; and a gui-e2e skill stub to be completed in Phase 3.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@clawsweeper clawsweeper Bot added P1 Urgent regression or broken agent/channel workflow affecting real users now. merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. 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. labels Sep 2, 2026
@clawsweeper

clawsweeper Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed September 2, 2026, 12:34 PM ET / 16:34 UTC.

ClawSweeper review

What this changes

The branch adjusts Windows Local AI to select GPU-dependent context and cache profiles, changes WSL networking onboarding, and adds a proposed Crabbox/FlaUI GUI E2E plan.

Regression provenance

Possible regression — suspected (reviewed change). No predecessor PR is attributed.

Merge readiness

Blocked until real behavior proof is added - 9 items remain

This Codex review found a merge-blocking Local AI setup-consent regression and a scope mismatch in this WIP PR.

Priority: P1
Reviewed head: fd0c59ecf2f252718c98278442f88c9dbd1f696b
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🧂 unranked krab (1/6) The patch has meaningful design and test signal, but it contains a blocking setup-consent regression and no real behavior proof.
Proof confidence 🧂 unranked krab (1/6) Needs real behavior proof before merge: The setup capabilities page now feeds the consent flag to the existing WSL configuration and shutdown step, but the PR body contains no filled current-head run, UI capture, or redacted runtime evidence showing this behavior. 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.
Patch quality 🦪 silver shellfish (2/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Needs proof Needs real behavior proof before merge: The setup capabilities page now feeds the consent flag to the existing WSL configuration and shutdown step, but the PR body contains no filled current-head run, UI capture, or redacted runtime evidence showing this behavior. 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 5 items Introduced consent bypass: The introduced assignment records WSL networking consent whenever Local AI is enabled, without a separate acknowledgement.
Existing global side effect: The setup step consumes that flag before applying mirrored networking and issuing a WSL shutdown.
Removed explicit acknowledgement: The introduced diff removes the checkbox whose text acknowledged the global WSL change and one-time shutdown, leaving only an informational warning.
Findings 1 actionable finding [P1] Restore explicit confirmation before changing global WSL networking
Security None None.

How this fits together

The Windows setup wizard checks GPU and WSL readiness, selects a local inference profile, then records and starts a managed local model runtime. When mirrored WSL networking is absent, this flow can modify user-wide WSL configuration and stop active WSL distributions.

flowchart LR
A[Setup wizard] --> B[Hardware and WSL checks]
B --> C[Local AI profile selection]
C --> D[WSL change confirmation]
D --> E[Global WSL configuration]
C --> F[Local model manifest]
F --> G[Local AI runtime]
Loading

Decision needed

Question Recommendation
Should the Local AI implementation be split from this GUI E2E planning PR before further review? Split the Local AI patch: Keep this PR focused on the GUI E2E proposal and review the Local AI profile and setup change independently after restoring confirmation.

Why: The PR body describes an initial GUI E2E design, while most of the branch is a separate Local AI behavior change with a merge-blocking consent regression.

Before merge

  • Add real behavior proof - Needs real behavior proof before merge: The setup capabilities page now feeds the consent flag to the existing WSL configuration and shutdown step, but the PR body contains no filled current-head run, UI capture, or redacted runtime evidence showing this behavior. 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.
  • Restore explicit confirmation before changing global WSL networking (P1) - Enabling Local AI now writes WslMirroredNetworkingConsent as true, so Continue reaches the existing step that edits the user-wide .wslconfig and runs a global WSL shutdown. The removed checkbox was the only separate acknowledgement of that disruptive side effect; keep an explicit confirmation gate rather than treating the feature toggle as consent.
  • Resolve merge risk (P1) - Enabling Local AI with mirrored networking disabled now automatically authorizes a user-wide .wslconfig change and stops every running WSL distribution, interrupting existing workloads.
  • Resolve merge risk (P1) - The stated GUI E2E planning scope is coupled to a 35-file Local AI implementation change, preventing independent review of either concern.
  • Complete next step (P2) - Restore explicit confirmation before the global WSL change and shutdown, add current-head real behavior proof, and resolve the mixed Local AI versus GUI E2E scope.
  • Improve patch quality - Restore a separate confirmation gate for the global WSL change and shutdown.
  • Improve patch quality - Provide redacted current-head proof of the corrected setup path, then update the PR body or ask a maintainer for @clawsweeper re-review.
  • Improve patch quality - Resolve whether the Local AI implementation belongs in this GUI E2E planning PR.

Findings

  • [P1] Restore explicit confirmation before changing global WSL networking — src/OpenClaw.SetupEngine.UI/Pages/CapabilitiesPage.xaml.cs:248
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Patch scope 35 files, +1,119/-194 lines The branch combines a GUI E2E proposal with a substantial Local AI implementation change.

Merge-risk options

Maintainer options:

  1. Restore explicit WSL confirmation (recommended)
    Require a separate acknowledgement before persisting consent and running the global WSL networking change and shutdown.
  2. Separate the unrelated scopes
    Move the Local AI implementation to its own review so the GUI E2E plan can be evaluated without a bundled behavior change.

Technical review

Best possible solution:

Split the GUI E2E proposal from the Local AI work, and retain an explicit acknowledgement that gates the global WSL change and shutdown.

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

Yes, source-reproducible: enable Local AI where mirrored WSL networking is disabled, then continue setup. The changed assignment makes the existing setup step pass its consent guard before it edits global WSL configuration and shuts down WSL.

Is this the best way to solve the issue?

No. The current change removes the prior explicit acknowledgement for a global side effect; restore that gate and review the unrelated Local AI implementation separately from the GUI E2E design.

Full review comments:

  • [P1] Restore explicit confirmation before changing global WSL networking — src/OpenClaw.SetupEngine.UI/Pages/CapabilitiesPage.xaml.cs:248
    Enabling Local AI now writes WslMirroredNetworkingConsent as true, so Continue reaches the existing step that edits the user-wide .wslconfig and runs a global WSL shutdown. The removed checkbox was the only separate acknowledgement of that disruptive side effect; keep an explicit confirmation gate rather than treating the feature toggle as consent.
    Confidence: 0.98

Overall correctness: patch is incorrect
Overall confidence: 0.98

AGENTS.md: found and applied where relevant.

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

Labels

Label changes:

  • add P1: The introduced setup path can stop active WSL distributions without retaining the prior explicit acknowledgement.
  • add merge-risk: 🚨 compatibility: The patch changes established setup behavior around a user-wide WSL configuration setting.
  • add merge-risk: 🚨 availability: The changed path performs a global WSL shutdown that can interrupt running workloads.
  • add merge-risk: 🚨 other: The branch’s Local AI scope materially exceeds the stated GUI E2E planning purpose.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦪 silver shellfish.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The setup capabilities page now feeds the consent flag to the existing WSL configuration and shutdown step, but the PR body contains no filled current-head run, UI capture, or redacted runtime evidence showing this behavior. 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.

Label justifications:

  • P1: The introduced setup path can stop active WSL distributions without retaining the prior explicit acknowledgement.
  • merge-risk: 🚨 compatibility: The patch changes established setup behavior around a user-wide WSL configuration setting.
  • merge-risk: 🚨 availability: The changed path performs a global WSL shutdown that can interrupt running workloads.
  • merge-risk: 🚨 other: The branch’s Local AI scope materially exceeds the stated GUI E2E planning purpose.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦪 silver shellfish.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The setup capabilities page now feeds the consent flag to the existing WSL configuration and shutdown step, but the PR body contains no filled current-head run, UI capture, or redacted runtime evidence showing this behavior. 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

What I checked:

Likely related people:

  • joelagnel: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

  • 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.

@github-actions

Copy link
Copy Markdown
Contributor

This pull request has been automatically marked as stale due to inactivity.
Please update it or it will be closed.

@github-actions github-actions Bot added the stale label Sep 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. P1 Urgent regression or broken agent/channel workflow affecting real users now. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. stale 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.

3 participants