Skip to content

[Repo Assist] fix(setup): ARM64-appropriate virtualization error message#609

Draft
github-actions[bot] wants to merge 1 commit into
masterfrom
repo-assist/fix-issue-593-arm64-wsl-virt-message-6fdf945d21a80f14
Draft

[Repo Assist] fix(setup): ARM64-appropriate virtualization error message#609
github-actions[bot] wants to merge 1 commit into
masterfrom
repo-assist/fix-issue-593-arm64-wsl-virt-message-6fdf945d21a80f14

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

🤖 This PR was created by Repo Assist, an automated AI assistant.

Summary

Closes #593

Fixes the ARM64 UX bug where a preflight-wsl virtualization failure showed x86-centric remediation instructions (VT-x/AMD-V, Intel VT or AMD SVM) on ARM64 devices like the Microsoft Cadmus, where those settings don't exist.

Root Cause

The TryGetEnvironmentIssue helper in SetupSteps.cs used a single hardcoded x86 remediation string for the "virtualization is not enabled" WSL error. ARM64 devices use ARMv8 EL2 virtualization extensions, which are exposed in UEFI as a generic "Virtualization Support" toggle or controlled via Intune (Memory Integrity / HVCI / Pluton) — none of which match the VT-x/AMD-V terminology.

Fix

Branch the remediation message on RuntimeInformation.OSArchitecture:

  • Architecture.Arm64 → generic UEFI / Intune / HVCI wording (new)
  • All other architectures → existing VT-x/AMD-V (Intel VT or AMD SVM) text (unchanged)

The detection logic itself (Contains(text, "virtualization is not enabled")) is unchanged — the WSL error text is the same on both architectures.

Test Status

  • Build: Infrastructure failure — GitVersion MSBuild task fails to write GitHub Actions env-file (set_env_*) in this runner environment. This is a pre-existing infrastructure issue unrelated to this change (no C# compiler errors).
  • SetupEngine tests: pre-existing test DetectsFirmwareVirtualizationOff checks Assert.Contains("virtualization", message), which passes on both branches of the new conditional.

Generated by 🌈 Repo Assist, see workflow run. Learn more.

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repo-assist.md@97143ac59cb3a13ef2a77581f929f06719c7402a

When preflight-wsl fails with 'virtualization is not enabled' on an
ARM64 device, the previous message told users to enable 'VT-x/AMD-V
(Intel VT or AMD SVM)' — terminology that doesn't exist on ARM64.

Branch the remediation text on RuntimeInformation.OSArchitecture:
- Arm64 → generic UEFI / Intune / HVCI wording
- x64/x86 → existing VT-x/AMD-V text (unchanged)

Closes #593

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

clawsweeper Bot commented May 31, 2026

Codex review: found issues before merge. Reviewed May 30, 2026, 9:43 PM ET / 01:43 UTC.

Summary
The PR updates the WSL virtualization-disabled setup message so ARM64 devices get UEFI and management-policy wording while non-ARM64 devices keep the existing VT-x/AMD-V text.

Reproducibility: yes. from source inspection: current master maps the observed WSL virtualization-off text to the x86 BIOS/VT-x/AMD-V remediation string. The linked report supplies the ARM64 setup path that reaches that branch, but I did not run a real ARM64 setup reproduction in this read-only review.

Review metrics: 1 noteworthy metric.

  • Changed surface: 1 source file changed, 0 test files changed. The PR adds host-dependent behavior without adding deterministic coverage for both architecture branches.

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

  • [P1] Add redacted after-fix ARM64 setup evidence showing the new message, such as a screenshot, terminal output, setup log, copied live output, or recording; redact private device, organization, endpoint, account, IP, policy, and credential details.
  • [P1] Add deterministic tests for both x64 and ARM64 wording so host architecture does not make the suite fail.
  • After adding proof, update the PR body to trigger a fresh ClawSweeper review automatically, or ask a maintainer to comment @clawsweeper re-review if it does not rerun.

Proof guidance:

  • [P1] Needs real behavior proof before merge: No after-fix real ARM64 setup proof is attached; add redacted evidence showing the new message in the affected setup path before merge. 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] No after-fix real ARM64 setup proof is attached, so reviewers cannot confirm the changed message appears in the affected setup path.
  • [P1] The PR leaves existing BIOS assertions unchanged, which can fail on ARM64 hosts once the new branch is active.
  • [P1] There is a competing open contributor PR for the same bug that also updates tests, so maintainers should choose one landing branch rather than merge competing implementations.

Maintainer options:

  1. Decide the mitigation before merge
    Land one ARM64-aware wording fix that includes deterministic x64 and ARM64 wording tests plus redacted proof from the affected setup path.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P2] Maintainers should choose between this draft bot PR and the existing contributor PR, then require proof and the missing test repair on the selected branch.

Security
Cleared: The diff only changes local setup diagnostic text and adds a framework architecture check; no concrete security or supply-chain concern was found.

Review findings

  • [P2] Update tests for the ARM64 message branch — src/OpenClaw.SetupEngine/SetupSteps.cs:103
Review details

Best possible solution:

Land one ARM64-aware wording fix that includes deterministic x64 and ARM64 wording tests plus redacted proof from the affected setup path.

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

Yes from source inspection: current master maps the observed WSL virtualization-off text to the x86 BIOS/VT-x/AMD-V remediation string. The linked report supplies the ARM64 setup path that reaches that branch, but I did not run a real ARM64 setup reproduction in this read-only review.

Is this the best way to solve the issue?

No. The wording direction is right, but this branch should make the architecture branch testable, update host-sensitive assertions, and provide real ARM64 setup proof before it is the best landing path.

Full review comments:

  • [P2] Update tests for the ARM64 message branch — src/OpenClaw.SetupEngine/SetupSteps.cs:103
    This branch makes TryGetEnvironmentIssue return an ARM64-specific message on ARM64 hosts, but the existing setup tests still assert that the returned preflight message contains BIOS. On an ARM64 test host those assertions now exercise the new branch and fail, and there is no injected-architecture test proving the x64 wording stays unchanged while ARM64 omits VT-x/AMD-V.
    Confidence: 0.93

Overall correctness: patch is incorrect
Overall confidence: 0.9

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against eb06fba21c44.

Label changes

Label justifications:

  • P3: This is a low-risk setup UX wording fix for a specific ARM64 preflight failure.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: No after-fix real ARM64 setup proof is attached; add redacted evidence showing the new message in the affected setup path before merge. 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

What I checked:

Likely related people:

  • Keith Mahoney: git blame and git log -S show this person authored the commit that added the WSL environment diagnostic helper and the current BIOS/VT-x wording. (role: introduced behavior; confidence: high; commits: 9106725567af; files: src/OpenClaw.SetupEngine/SetupSteps.cs, tests/OpenClaw.SetupEngine.Tests/SetupStepsTests.cs)
  • Ranjesh Jaganathan: Recent history for SetupSteps.cs shows multiple setup and WSL workflow commits shortly before the diagnostic helper landed, making this person relevant for setup-engine routing. (role: recent area contributor; confidence: medium; commits: ff3a7cc53736, 98d0760cb5b6, 1b1344ddd34b; files: src/OpenClaw.SetupEngine/SetupSteps.cs)
  • Regis Brid: The linked bug report and the related open PR at Use Arm64-aware wording when WSL reports virtualization off #594 provide the ARM64 reproduction context and a competing tested implementation path. (role: reporter and alternate implementation author; confidence: medium; commits: a3b8e7f, 8c8481457685; files: src/OpenClaw.SetupEngine/SetupSteps.cs, tests/OpenClaw.SetupEngine.Tests/SetupStepsTests.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. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. labels May 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automation P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. repo-assist 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.

Setup: 'preflight-wsl' failure message uses x86 firmware terminology on ARM64

0 participants