Skip to content

Redesign local gateway setup as out-of-process SetupEngine - #529

Merged
shanselman merged 37 commits into
openclaw:masterfrom
ranjeshj:user/ranjeshj/setupenginefortest2
May 26, 2026
Merged

Redesign local gateway setup as out-of-process SetupEngine#529
shanselman merged 37 commits into
openclaw:masterfrom
ranjeshj:user/ranjeshj/setupenginefortest2

Conversation

@ranjeshj

@ranjeshj ranjeshj commented May 23, 2026

Copy link
Copy Markdown
Collaborator

Redesign local gateway setup as out-of-process SetupEngine

Problem

The previous onboarding/setup flow was tightly coupled to the tray app, making it hard to test, debug, and maintain. The in-process LocalGatewaySetup class was ~4000 lines with mixed concerns (WSL management, gateway config, UI state, approval flows).

Solution

Replace the monolithic in-process setup with a standalone SetupEngine — a config-driven, transactional pipeline that runs as a separate process (SetupEngine.UI). The tray app launches it and receives results via deep-link protocol.

Architecture

  • OpenClaw.SetupEngine — CLI/library with 17 pipeline steps, transaction journal for rollback, retry executor, and structured logging
  • OpenClaw.SetupEngine.UI — WinUI3 app with fluent wizard flow (Welcome → Capabilities → Permissions → Progress → Complete)
  • Config-drivendefault-config.json defines capabilities, WSL distro settings, gateway config; no hardcoded defaults
  • Transactional — journal tracks completed steps; rollback undoes in reverse order on failure

Key changes

New projects:

  • src/OpenClaw.SetupEngine/ — pipeline engine (SetupPipeline, SetupSteps, CommandRunner, RetryExecutor, TransactionJournal)
  • src/OpenClaw.SetupEngine.UI/ — WinUI wizard (6 pages, launched by tray)
  • tests/OpenClaw.SetupEngine.Tests/ — 66 unit tests
  • tests/OpenClaw.E2ETests/ — end-to-end setup test via MCP

Removed (~10,400 lines):

  • src/OpenClawTray.OnboardingV2/ — old V2 onboarding app
  • src/OpenClaw.Tray.WinUI/Onboarding/ — old in-process wizard, services, flow controller
  • src/OpenClaw.Tray.WinUI/Services/LocalGatewaySetup/ — monolithic setup (~5,800 lines)
  • tests/OpenClawTray.OnboardingV2.Tests/ — empty placeholder project
  • Related tray tests (~4,600 lines of tests for removed code)

Modified:

  • src/OpenClaw.Tray.WinUI/App.xaml.cs — launch SetupEngine.UI instead of in-process onboarding
  • src/OpenClaw.Tray.WinUI/Services/StartupSetupState.cs — detect setup-state from SetupEngine
  • .github/workflows/ci.yml — add SetupEngine tests, E2E job, replace OnboardingV2

Reliability hardening (Hanselman dual-model review)

Adversarial review with Claude Opus + GPT Codex identified 18 issues; 12 fixed:

  • HIGH consensus: RetryExecutor exception handling, CommandRunner Win32Exception, shell injection in ExtraConfig
  • LOW consensus: CancellationToken in finally, journal IOException, stale state cleanup, null token fallback, event handler leak, VHD delete retry, keepalive redirect, cleanup on cancel

Test coverage

Suite Tests
SetupEngine unit 66
Shared 1,936
Tray 791
E2E (setup+connect) 1
Total 2,794

E2E validated 5 consecutive runs, avg 153s, no flakiness.

@clawsweeper

clawsweeper Bot commented May 23, 2026

Copy link
Copy Markdown

Codex review: needs changes before merge. Reviewed May 25, 2026, 11:12 PM ET / 03:12 UTC.

Summary
This PR replaces the in-process local gateway onboarding/setup flow with new OpenClaw.SetupEngine and OpenClaw.SetupEngine.UI projects, rewires tray setup/uninstall/keepalive behavior, updates CI, and removes the old onboarding/local setup implementation and tests.

Reproducibility: not applicable. as a bug reproduction, but the packaging gap is source-reproducible: the tray launch path returns when SetupEngine.UI is absent, while the MSIX build path does not show an equivalent SetupEngine.UI inclusion step.

Review metrics: 2 noteworthy metrics.

  • Diff Size: 144 files, +12,602/-19,110. This is a broad replacement of the setup/onboarding surface, so green unit tests alone do not settle upgrade and packaging behavior.
  • Setup Proof Reported: ARM64 E2E, x64 builds, CI gates, and multi-gateway smoke reported. The discussion contains meaningful after-fix runtime evidence, which raises confidence in the core setup path while leaving packaging layout as the remaining blocker.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦞 diamond lobster
Patch quality: 🦐 gold shrimp
Result: needs maintainer review before merge.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Package SetupEngine.UI in MSIX/package and ordinary tray launch layouts.
  • Refresh validation after the packaging fix, including the required AGENTS.md build/shared/tray checks and a package-layout check.

Risk before merge

  • Packaged/MSIX tray builds can lose the local setup entry point because the tray now depends on an external SetupEngine.UI executable but only the release zip publish path clearly copies that executable beside the tray.
  • This PR replaces setup, pairing, uninstall, keepalive, and gateway registry behavior in one large change, so upgrade proof matters for existing local gateways, external gateways, and stored credentials even though the reported sanity checks are strong.
  • The setup path owns bootstrap/shared/device token transitions; a regression here would surface as pairing loops or a tray that cannot reconnect after setup.

Maintainer options:

  1. Package SetupEngine Everywhere (recommended)
    Add SetupEngine.UI to every tray distribution layout that can invoke local setup, including MSIX/package output and ordinary dev/debug launch paths, then refresh validation.
  2. Accept Zip-Only Setup For Now
    Maintainers could intentionally ship the new local setup only in the zip/Inno path, but the PR should document that MSIX/local package setup is unsupported or deferred.
  3. Pause The Rewrite
    If packaging parity is not worth solving in this branch, pause this PR and split the SetupEngine packaging work into a smaller prerequisite or follow-up branch before merging the rewrite.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Fix the PR so OpenClaw.SetupEngine.UI is present in every supported tray launch layout, especially MSIX/package output, without reintroducing the old in-process onboarding flow; add a focused packaging/source assertion if practical and keep the existing release-zip copy behavior.

Next step before merge
A focused repair can add SetupEngine.UI to all tray package/layout outputs without deciding the broader SetupEngine architecture.

Security
Cleared: No concrete new security or supply-chain blocker was found; the installer path remains the existing HTTPS OpenClaw CLI installer pattern and this PR adds token redaction, environment-token use, and config key validation around the new setup flow.

Review findings

  • [P2] Bundle SetupEngine.UI in packaged tray builds — src/OpenClaw.Tray.WinUI/App.xaml.cs:2914-2918
Review details

Best possible solution:

Land the SetupEngine replacement only after every supported tray distribution can launch SetupEngine.UI and the setup/connect/uninstall proof remains green for fresh install and upgrade-style layouts.

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

Not applicable as a bug reproduction, but the packaging gap is source-reproducible: the tray launch path returns when SetupEngine.UI is absent, while the MSIX build path does not show an equivalent SetupEngine.UI inclusion step.

Is this the best way to solve the issue?

No for merge as-is: the out-of-process SetupEngine is a maintainable direction, but the PR should close the package/layout gap so local setup remains reachable from every supported tray distribution.

Full review comments:

  • [P2] Bundle SetupEngine.UI in packaged tray builds — src/OpenClaw.Tray.WinUI/App.xaml.cs:2914-2918
    ShowOnboardingAsync now depends on finding OpenClaw.SetupEngine.UI.exe beside the tray and otherwise only logs and returns. The release zip path copies publish-setup into publish\SetupEngine, but the MSIX job still only builds OpenClaw.Tray.WinUI and the tray project has no package content/target that includes SetupEngine.UI. That means packaged installs can lose the local gateway setup entry point; please include SetupEngine.UI in the packaged output or keep a fallback that works there.
    Confidence: 0.83

Overall correctness: patch is incorrect
Overall confidence: 0.82

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
  • remove rating: 🧂 unranked krab: Current PR rating is rating: 🦐 gold shrimp, so this older rating label is no longer current.

Label justifications:

  • P1: The PR replaces the core local gateway setup and pairing workflow, so a merge regression would affect real first-run and reconnect flows.
  • merge-risk: 🚨 compatibility: The branch removes the in-process onboarding implementation and changes distribution layout assumptions for existing tray builds.
  • merge-risk: 🚨 auth-provider: The setup engine rewrites bootstrap, shared-token, device-token, and gateway registry pairing flows.
  • merge-risk: 🚨 availability: The setup, uninstall, and WSL keepalive paths can make the local gateway unavailable if the new process orchestration or packaging is wrong.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): The PR discussion includes after-fix build, CI, ARM64/x64 setup, E2E, and multi-gateway install/uninstall proof for the changed behavior; no contributor action is needed for the proof gate.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR discussion includes after-fix build, CI, ARM64/x64 setup, E2E, and multi-gateway install/uninstall proof for the changed behavior; no contributor action is needed for the proof gate.
Evidence reviewed

Acceptance criteria:

  • ./build.ps1
  • dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore
  • dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore
  • dotnet build src/OpenClaw.Tray.WinUI/OpenClaw.Tray.WinUI.csproj -c Release -r win-x64 -p:PackageMsix=true -p:GenerateAppxPackageOnBuild=true

What I checked:

  • Repository policy read: AGENTS.md was read in full; its connection/onboarding guidance and validation expectations were applied as review context, but this read-only review did not run build/test commands. (AGENTS.md:1, ef6ac8acbab2)
  • Out-of-process launch dependency: The tray resolves only AppContext.BaseDirectory/SetupEngine/OpenClaw.SetupEngine.UI.exe or a flat sibling executable, then returns after logging if neither exists. (src/OpenClaw.Tray.WinUI/App.xaml.cs:2868, 9cadb6b04d58)
  • Release zip copies SetupEngine.UI: The standard release build publishes SetupEngine.UI to publish-setup and copies it into publish\SetupEngine for the tray artifact. (.github/workflows/ci.yml:320, 9cadb6b04d58)
  • MSIX path lacks equivalent SetupEngine packaging: The MSIX job builds only OpenClaw.Tray.WinUI via msbuild, and the tray project content inspected in the PR does not include SetupEngine.UI output as package content. (.github/workflows/ci.yml:421, 9cadb6b04d58)
  • Maintainer-context validation: The GitHub discussion reports ARM64 build/test/E2E validation, then CI, x64 build, and multi-gateway install/uninstall sanity checks passing on later PR heads, with only a legacy GatewayUrl cleanup polish noted. (24a810895061)
  • Merge-result inspected: The local merge commit has parents current main ef6ac8a and PR head 9cadb6b, so the review considered the actual current-main merge result rather than stale base drift. (69c330aeeafa)

Likely related people:

  • shanselman: Current-main blame and history show Scott Hanselman owning large parts of the existing local setup, setup classification, and connection manager behavior that this PR replaces or rewires. (role: recent area contributor; confidence: high; commits: 45dccec60c51, 6eb18a6cdcdd, 8914780f3d2c; files: src/OpenClaw.Tray.WinUI/Services/LocalGatewaySetup/LocalGatewaySetup.cs, src/OpenClaw.Tray.WinUI/Services/SetupExistingGatewayClassifier.cs, src/OpenClaw.Connection/GatewayConnectionManager.cs)
  • Mike Harsh: Recent current-main commits added WSL keepalive and setup diagnostics, both central to the new SetupEngine install/uninstall and keepalive paths. (role: recent setup/keepalive contributor; confidence: high; commits: 4baf01d768c0, 8446ea552779; files: src/OpenClaw.Tray.WinUI/Services/LocalGatewaySetup/LocalGatewaySetup.cs, src/OpenClaw.Tray.WinUI/Services/LocalGatewaySetup/LocalGatewaySetupDiagnostics.cs, src/OpenClaw.Tray.WinUI/App.xaml.cs)
  • ranjeshj: Prior merged history shows Ranjesh worked on node pairing, MCP runtime toggles, capability counts, and connection defaults before authoring this broader SetupEngine replacement. (role: recent connection/pairing contributor; confidence: high; commits: 7747e6075052, d7d7661ac28c, ed126f2aac6d; files: src/OpenClaw.Connection/GatewayConnectionManager.cs, src/OpenClaw.Tray.WinUI/App.xaml.cs, src/OpenClaw.Tray.WinUI/Pages/ConnectionPage.xaml.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.

ranjeshj and others added 14 commits May 23, 2026 10:22
New headless setup engine that installs WSL, configures OpenClaw gateway,
pairs operator/node connections, and verifies end-to-end connectivity.

- Transactional pipeline with retry, rollback, and crash-recovery journal
- Structured JSONL logging with secret redaction
- v2 signature fix for local gateways in GatewayConnectionManager
- All 15 steps working E2E in headless mode

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Configure gateway with bind=lan (0.0.0.0) instead of loopback to avoid
  unreliable WSL2 localhost port forwarding
- Add StartKeepaliveStep (step 16) to keep distro alive after setup
  completes, ensuring tray connects instantly on launch
- Add default-config.json and design doc

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

- Expand SetupConfig with nested WslConfig, GatewayConfig, CapabilitiesConfig,
  TraySettingsConfig, and PairingConfig — zero hardcoded values
- Register node capabilities (stub INodeCapability) before ConnectAsync so
  gateway stores caps/commands from hello message
- Write settings.json after node pairing (EnableNodeMode=true + cap toggles)
  using merge logic that preserves existing user settings
- Make WSL wsl.conf generation config-driven (user, systemd, interop, etc.)
- Make gateway config-driven (bind mode, auth, health timeout, extra config)
- Write keepalive marker file to prevent tray duplicate keepalive
- Add fully commented default-config.json with all configurable properties

Verified: clean build (0 errors/0 warnings), full E2E run in 118s,
tray auto-connects with capabilities registered.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds DrainPendingApprovalsAsync to VerifyEndToEndStep that iteratively
approves any remaining pending device or node pairing requests. This
ensures the tray launches with zero 'Pairing approval pending' badges.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Standalone unpackaged WinUI app (no FunctionalUI dependency) with:
- Welcome page with lobster icon, info card, V2 text strings
- Capabilities page with 2-column grid, icons, descriptions, toggles
- Progress page with step groups, badges (spinner/check/error), log viewer
- Complete page with success/error state, launch tray button

Features:
- Mica backdrop + extended title bar
- DPI-aware window sizing (720x700 logical)
- UAC manifest (asInvoker) to avoid elevation prompt
- --headless bypass for automation
- Config-driven defaults from SetupConfig

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Welcome: flex layout, full V2 text, 'Install new WSL Gateway' button,
  confirmation dialog, 'Advanced setup' link (opens tray connection page)
- Title bar: lobster icon + 'OpenClaw Setup' text, 36px height
- Permissions page: 5 rows (notifications, camera, mic, location, screen),
  live status checks, 'Open Settings' buttons, 'Refresh status'
- Complete page: party popper, amber Node Mode banner, startup toggle,
  'Finish' button with tray launch + optional registry startup entry
- Window height increased to 820px for better step row spacing

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove all hardcoded defaults from SetupConfig (DistroName, GatewayPort, BaseDistro, etc.)
- Both UI and headless exe now require a config file to run
- UI auto-loads bundled default-config.json from AppContext.BaseDirectory
- Headless Program.cs exits with error if no config found
- Added Content Include in csproj to bundle default-config.json with exe

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- CleanupStaleDistroStep: wsl --shutdown + delete orphaned VHD directory
- CompletePage: show error message + 'View full log' link on failure
- CompletePage: kill old tray, launch via openclaw://chat protocol
- Update SETUP_ENGINE_REDESIGN.md to reflect current implementation

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- add interactive gateway wizard rendering for SetupEngine UI
- make wizard messages render links and device codes inline
- refine progress/log layout and setup failure visuals
- fix wizard retry/skip behavior and credential precedence
- harden WSL cleanup, base distro reuse, and missing WSL handling

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add RollbackAsync to 6 SetupEngine steps for --uninstall support
- Add UninstallAsync to SetupPipeline (reverse rollback execution)
- Add --uninstall, --confirm-destructive, --json-output CLI flags
- Replace ShowOnboardingAsync with out-of-process SetupEngine.UI launch
- Rewrite CliUninstallHandler and SettingsPage uninstall to use SetupEngine
- Delete LocalGatewaySetup (4000+ lines), Onboarding, OnboardingV2 projects
- Extract GatewayConnectorInterfaces and WslCommandRunner from deleted code
- Fix StartupSetupState to scan per-gateway dirs for device tokens
- Simplify WSL keepalive to direct wsl process spawn
- Add SetupEngine to build.ps1 with post-build copy to WinUI output
- Set production defaults: DistroName=OpenClawGateway, GatewayPort=18789

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Create tests/OpenClaw.E2ETests/ project with end-to-end tests that exercise
the full setup pipeline headless via SetupEngine CLI, spawn the tray app,
and verify operator+node connectivity through MCP app.status/app.nodes calls.

- E2ESetupFixture: runs Program.Main() headless, patches settings for MCP,
  spawns tray process, polls connection status, cleans up via uninstall
- SetupAndConnectTests: verifies connected state and node capabilities
- McpClient: JSON-RPC client for MCP HTTP server verification
- CI workflow: parallel e2e job with test artifact upload for debugging
- SetupEngine.csproj: add RuntimeIdentifiers for RID-specific test builds

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Hanselman adversarial review (Opus + Codex) identified 18 issues.
Fixed 12 reliability bugs across the setup engine:

HIGH consensus (both models):
- RetryExecutor: wrap action() in try/catch to prevent pipeline crashes
- CommandRunner: catch Win32Exception on process.Start()
- ConfigureGatewayStep: shell-escape ExtraConfig values

Verified LOW consensus:
- TryResetReloadModeAsync: use CancellationToken.None in finally
- TransactionJournal: catch IOException on writes
- CleanupStaleGatewayStep: delete setup-state.json from both AppData and LocalAppData
- AutoApprove: fall back to BootstrapToken when SharedGatewayToken is null
- TrayArtifactCleanup: protect DeleteFileIfExists with try/catch
- StartKeepaliveStep: remove unused stdout/stderr redirect
- PairOperatorStep: unsubscribe DeviceTokenReceived handler
- Program: run TrayArtifactCleanup on Cancelled outcome
- CleanupStaleDistroStep: retry VHD directory deletion with backoff

Added 66 unit tests in new OpenClaw.SetupEngine.Tests project:
- RetryExecutorTests (11), SetupPipelineTests (14),
  TransactionJournalTests (9), SetupLoggerTests (7),
  SetupConfigTests (18), SetupContextTests (7)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@ranjeshj
ranjeshj force-pushed the user/ranjeshj/setupenginefortest2 branch from ceace09 to 328419f Compare May 23, 2026 17:24
@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. P1 Urgent regression or broken agent/channel workflow affecting real users now. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. labels May 23, 2026
@clawsweeper

clawsweeper Bot commented May 23, 2026

Copy link
Copy Markdown

ClawSweeper PR egg

🔥 Warming up: real-behavior proof passed; findings, security review, or rank-up moves are still in progress.

Hatch command

Comment @clawsweeper hatch when this PR is hatchable.

Hatchability rules:

  • Merged PRs are hatchable.
  • Open PRs are hatchable when they are status: 👀 ready for maintainer look, status: 🚀 automerge armed, or labeled clawsweeper:automerge.
  • Closed unmerged PRs are hatchable only when one of those hatchable labels is still present in the durable record.
What is this egg doing here?
  • Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
  • The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
  • Hatchability usually comes from sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness. A merged PR is already final, so merge makes the egg hatchable independently.
  • The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
  • Rarity is just collectible sparkle: 🥚 common, 🌱 uncommon, 💎 rare, ✨ glimmer, and 🌈 legendary.

- Replace OnboardingV2.Tests build/run steps with SetupEngine.Tests (66 unit tests)
- Remove empty OnClawTray.OnboardingV2.Tests project (superseded by SetupEngine)
- Drop --no-restore from E2E build step to fix RID-mismatch NETSDK1004

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@ranjeshj ranjeshj changed the title fix: setup engine reliability fixes and unit tests Redesign local gateway setup as out-of-process SetupEngine May 23, 2026
ranjeshj and others added 5 commits May 23, 2026 12:26
- Add -r win-x64 to SetupEngine.Tests build (allows implicit restore for RID-specific deps)
- Add -r win-x64 to SetupEngine.Tests run step
- Rename e2e job to e2etests for clarity

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…n, release packaging

- CleanupStaleGatewayStep: preserve SSH-tunneled and non-local gateway
  records instead of deleting by URL match alone
- InstallCliStep: validate HTTPS scheme, shell-quote URL, add
  --proto '=https' --tlsv1.2 to curl
- ci.yml: publish SetupEngine.UI into release package
- Add 8 unit tests covering gateway preservation and URL validation

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

- Default gateway bind from 'lan' (0.0.0.0) to 'loopback' (127.0.0.1)
- Add ValidateWslLockdownStep: verify user, dirs, ownership after configure
- Replace --token argv with OPENCLAW_GATEWAY_TOKEN env var (9 call sites)
- Add ExistingConfigDetector for dynamic replacement dialog on WelcomePage
- Remove unimplemented OperatorScopes/NodeScopes/CliScopes from PairingConfig
- Add port conflict detection (ss -tlnp) and improved failure diagnostics
- Add RedactTokens helper for log sanitization
- Default SkipPermissions to false, add fallback tray exe path
- Fix docs drift: step count, default claims
- Add UI step group mappings for validate-wsl-lockdown and run-wizard
- 279 new lines of unit tests (lockdown, bind validation, token redaction, etc.)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add setup run locking, append-only recovery journals, atomic persistence, bounded command output and rollback handling, UI cancellation/error guards, wizard loop bounds, isolated local data support, and expanded token redaction.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Ensure setup runs privileged WSL configuration as root so imported base distros with non-root defaults still configure correctly. Align local AppData override handling across SetupEngine, tray setup detection, keepalive, and e2e isolation, and accept numeric setup-state phases written by the new engine.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@clawsweeper clawsweeper Bot removed the status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. label May 25, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. labels May 25, 2026
@ranjeshj
ranjeshj marked this pull request as ready for review May 25, 2026 14:13
@clawsweeper clawsweeper Bot added status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels May 25, 2026
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. labels May 25, 2026
Use Fluent icon glyphs for completed and failed setup steps, add a subtle pending indicator, and give the active progress ring enough space to render cleanly.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels May 25, 2026
Align the UI test WinAppSDK bootstrap/runtime install with the Microsoft.WindowsAppSDK package version and add fixture startup diagnostics.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels May 25, 2026
@shanselman

Copy link
Copy Markdown
Collaborator

Pre-merge sanity-check ✅

Re-reviewed the latest head (bfbd554) after the hardening pass. Most of the review findings have been addressed and the architecture+test coverage are solid. The remaining open items are all fast-follow material, not blockers. Confidence: ~90%, recommend merge after the three checks below.

Three quick sanity checks before pulling the trigger

  1. CI green on bfbd554test, e2etests, build, build-msix, build-extension, release all pass. (The new needs: [test, e2etests] gates the publish path, which is the right call.)

  2. Manual smoke on x64 — local validation passed on ARM64 (./build.ps1, Shared 1936/29, Tray 796, SetupEngine 170, ARM WSL E2E 5/5 in ~2:45). One fresh x64 install → connect → uninstall cycle as a release rehearsal would catch anything ARM/x64-specific.

  3. Multi-gateway smoke — register a second non-local gateway in the tray, run SetupEngine local-gateway install, then uninstall. Confirm the external gateway record still works after uninstall. The new SetupManagedDistroName discrimination correctly preserves external records, but TrayArtifactCleanup.ResetOnboardingSettings still clears GatewayUrl / EnableNodeMode / AutoStart. Worth validating that the tray then picks up the other registered gateway cleanly on next launch.

Recommended fast-follow PRs (post-merge)

Bundle into 2–3 small follow-ups:

  • Robustness: token rotation atomicity in FinalizeOperatorForTray; tray↔engine file locking on shared settings/registry; write setup-state.json on failure too (so the tray surfaces failure context instead of looping back into setup).
  • Uninstall polish: journal-aware uninstall (skip rollbacks for never-executed steps); multi-gateway-aware settings reset with a test.
  • Cleanup: keepalive marker check Process.StartTime (PID reuse), preflight port → start race, remove dead TryRevokeOperatorTokenAsync legacy path, split SetupSteps.cs (~2,500 lines), actually enforce the documented OS version check.

Things deliberately left alone

  • InstallCliStep still does curl | bash from openclaw.ai/install-cli.sh. After reading the script — it's a 625-line cross-platform installer with its own sha256 verification of upstream Node tarballs, used by the whole OpenClaw project — embedding or pinning it in the Windows engine would create more problems than it solves. Best handled by the OpenClaw web/CLI team independently (versioned URLs + signed artifacts, etc.).
  • SharedGatewayToken persistence in GatewayRecord (the new dashboard-link feature relies on it). Could DPAPI-protect later.

🤖 Posted by Copilot CLI on behalf of the review tooling. Happy to re-validate once these three checks are done.

@shanselman

Copy link
Copy Markdown
Collaborator

Sanity-check results ✅

Bot ran all three pre-merge checks on 24a81089. Summary:

✅ Check #1: CI green

Build and Test workflow on 24a81089 ("Fix tray UI test runtime bootstrap") completed successfully. All gates (repo-hygiene, test, e2etests, build, build-msix, build-extension) green.

✅ Check #2: x64 builds clean

Local validation:

  • OpenClaw.Tray.WinUI builds on win-x64 (0 errors, 679 warnings — all pre-existing CS-warning noise)
  • OpenClaw.SetupEngine.UI builds on win-x64 (0 errors, 1 warning — the existing CA1416 on TrayArtifactCleanup.Run)

✅ Check #3: Multi-gateway scenario (the interesting one)

Pre-seeded gateways.json with an external (non-local, non-tunnel) record external-test-gw-001 pointing at wss://example.invalid:18789, plus a settings.json with the user's GatewayUrl, EnableNodeMode=false, and a custom MyCustomUserKey field. Then ran setup with DistroName=OpenClawMG2-1643, then uninstall.

Install:

  • ✅ External gateway record preserved exactly in gateways.json
  • ✅ Local gateway record added alongside the external (both coexist)
  • ✅ External identity directory gateways/external-test-gw-001/ preserved
  • ✅ Settings merge logic correct: MyCustomUserKey preserved, EnableNodeMode=false (user's choice) preserved, only missing capability defaults appended — this is the hardened TryAdd path working as intended
  • activeId switched to the new local (reasonable: they just installed it)

Uninstall (--uninstall --confirm-destructive):

  • ✅ Local gateway record removed from gateways.json
  • ✅ External gateway record still preserved (the SetupManagedDistroName discrimination works)
  • ✅ Local identity directory removed
  • ✅ External identity directory still present
  • ✅ WSL distro OpenClawMG2-1643 unregistered
  • MyCustomUserKey and EnableNodeMode=false still in settings.json
  • activeId correctly set to null (user/tray picks up external from registry)

One small finding from check #3 (matches earlier fast-follow #17)

TrayArtifactCleanup.ResetOnboardingSettings unconditionally removes the legacy GatewayUrl field from settings.json on uninstall, even when an external gateway remains:

{
- "GatewayUrl": "wss://example.invalid:18789",  // user's external gw - removed
  "EnableNodeMode": false,                       // preserved ✓
  "MyCustomUserKey": "preserve-me-please",       // preserved ✓
  ...
}

Not a blocker — the tray reads from gateways.json (registry) not from settings.GatewayUrl for connection decisions, so the user can still pick their external gateway. But the polish would be: when uninstalling a local gateway and settings.GatewayUrl matches the local one being removed, set it to the next remaining gateway's URL (or null only if no others exist). Worth adding to the multi-gateway-aware cleanup PR.

Net verdict

All three pre-merge checks pass. Multi-gateway behavior is essentially correct — the records and identities for external gateways are preserved across both install and uninstall. The GatewayUrl legacy-field reset is a minor polish item, not a correctness issue.

Ready to merge from my side. 🚀

🤖 Posted by Copilot CLI bot after running the three sanity checks. Test artifacts in temp; happy to provide setup/uninstall logs if useful.

ranjeshj and others added 2 commits May 25, 2026 17:02
Use the active gateway registry record when deciding whether tray startup should keep the local WSL gateway alive, and add unit and e2e coverage for the keepalive process.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels May 26, 2026
@shanselman
shanselman merged commit cefce39 into openclaw:master May 26, 2026
12 checks passed
shanselman added a commit that referenced this pull request May 26, 2026
Fixes identified in comprehensive test audit:

HIGH Severity Fixes:
- SetupPipelineTests: Fix always-pass test for skippable steps
  - Now verifies StepOutcome.Skipped via progress events
- AppCapabilityTests: Expand trivial tests with edge cases
  - Added case sensitivity checks, prefix validation
  - Verified Commands/CanHandle consistency
- SshTunnelServiceTests: Improve incomplete test assertions
  - Added verification of pre-reset state
- RadioButtonsTests: Add functional behavior tests
  - Added tests for valid selection and empty items
  - Expanded from 1 to 3 tests

All modified tests now verify actual behavior rather than
just property values.

Related to: #529 test quality improvements
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. 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. P1 Urgent regression or broken agent/channel workflow affecting real users now. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants