Skip to content

Fix Local AI retry recovery - #1342

Closed
karkarl wants to merge 1 commit into
mainfrom
fix-local-ai-retry-recovery
Closed

karkarl wants to merge 1 commit into
mainfrom
fix-local-ai-retry-recovery

Conversation

@karkarl

@karkarl karkarl commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

Fixes #1307

Summary

  • Route Local AI Retry setup or download through a Local AI recovery command instead of full onboarding.
  • Add a recovery-only setup pipeline that preserves the existing WSL distro and gateway install, repairs Local AI assets/configuration, verifies WSL reachability, and restarts the gateway.
  • Start the setup UI directly at the Local AI review page so Install & setup is recomputed from current capability/setup state instead of staying stale-disabled.

Required proof pools

  • windows-wsl-dgx-blackwell: Local AI WSL-visible NVIDIA setup/restart/inference proof is required for hardware-backed behavior.
  • windows-wsl-gateway-e2e: Local AI recovery must preserve product WSL/gateway state and prove gateway invocation.
  • windows-winui-interactive: current-head visual proof is required for the Local AI retry and Install & setup recovery path.

Validation

  • .\build.ps1: passed, all builds succeeded.
  • $env:OPENCLAW_REPO_ROOT=(Get-Location).Path; dotnet test .\tests\OpenClaw.Shared.Tests\OpenClaw.Shared.Tests.csproj --no-restore: passed, 3937 passed, 32 skipped, 0 failed.
  • $env:OPENCLAW_REPO_ROOT=(Get-Location).Path; dotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore: passed, 2858 passed, 0 skipped, 0 failed.
  • $env:OPENCLAW_REPO_ROOT=(Get-Location).Path; dotnet test .\tests\OpenClaw.SetupEngine.Tests\OpenClaw.SetupEngine.Tests.csproj --no-restore: passed, 1058 passed, 0 skipped, 0 failed.
  • $env:OPENCLAW_REPO_ROOT=(Get-Location).Path; dotnet test .\tests\OpenClaw.SetupEngine.Tests\OpenClaw.SetupEngine.Tests.csproj --no-restore --filter "FullyQualifiedName~SetupPipelineTests": passed, 29 passed, 0 skipped, 0 failed.
  • $env:OPENCLAW_REPO_ROOT=(Get-Location).Path; dotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore --filter "FullyQualifiedName~LocalAiPageViewModelTests": passed, 3 passed, 0 skipped, 0 failed.
  • python .agents\skills\autoreview\scripts\autoreview --mode local: no significant issues reported by rubber-duck closeout; helper failed closed on one path because the diff bundle exceeded the local safety limit.

Real behavior proof

  • Focused regression proof: LocalAiPageViewModelTests verifies Retry setup or download now invokes ShowLocalAiSetupRecovery and does not invoke full onboarding.
  • Focused recovery-pipeline proof: SetupPipelineTests.BuildLocalAiRecoverySteps_PreservesExistingWslGateway verifies the retry pipeline excludes WSL/gateway destructive steps such as cleanup-stale-distro, cleanup-stale-gateway, create-wsl, configure-wsl, and install-cli, while still including Local AI repair, WSL verification, gateway configuration, and gateway restart.
  • Not verified / blocked: no interactive WinUI plus WSL-visible NVIDIA/Gateway proof was captured on this host. The required proof pools above remain declared for maintainer-scheduled validation.

Ownership notes

  • Old owner: Local AI settings retried via the general onboarding command and default setup pipeline.
  • New owner: WindowManager.ShowLocalAiSetupRecoveryAsync opens setup in Local AI recovery mode, and SetupStepFactory.BuildLocalAiRecoverySteps owns the non-destructive retry step list.
  • Preserved invariant: App.xaml.cs remains only the composition-root command forwarder; setup window orchestration stays in WindowManager, and full onboarding behavior is unchanged for normal setup.

Route Local AI retry setup through a non-destructive recovery pipeline that preserves the existing WSL sandbox and restarts the gateway after Local AI reconfiguration. Add focused regression coverage for the recovery route and pipeline shape.

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

clawsweeper Bot commented Sep 5, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P0 Emergency: data loss, security bypass, crash loop, or unusable core runtime. 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. 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. labels Sep 5, 2026
@clawsweeper

clawsweeper Bot commented Sep 5, 2026

Copy link
Copy Markdown

Codex review: blocked before merge. Reviewed September 5, 2026, 6:58 PM ET / 22:58 UTC.

ClawSweeper review

What this changes

Routes Local AI retry through a setup pipeline that preserves the existing WSL gateway, opens the Local AI review screen directly, and changes command-output draining.

Regression provenance

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

Merge readiness

Blocked before merge - 5 items remain

This remains necessary: main and the latest release still route retry through full onboarding. The dedicated recovery path is useful, but two introduced regressions block landing.

Priority: P0
Reviewed head: c44f02325677d2677a113fd6f09d4de828f98f98

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The focused recovery direction and reported tests are useful, but missing-install routing and timeout regressions prevent landing.
Proof confidence 🌊 off-meta tidepool Not applicable: The author is a collaborator, so the external-contributor proof gate does not apply. The captured evidence covers command routing and pipeline shape through tests; it explicitly leaves the setup window, native inference recovery, and preserved WSL gateway behavior unverified in the declared maintainer-scheduled pools.
Patch quality 🦐 gold shrimp (3/6) 2 actionable review findings remain.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The author is a collaborator, so the external-contributor proof gate does not apply. The captured evidence covers command routing and pipeline shape through tests; it explicitly leaves the setup window, native inference recovery, and preserved WSL gateway behavior unverified in the declared maintainer-scheduled pools.
Evidence reviewed 6 items Current main and release still need the fix: Both fetched main and the supplied latest-release commit route RetrySetup to ShowOnboarding. The related open issue documents sandbox destruction and an unusable recovery screen.
Latest-release comparison: The same onboarding retry route exists in v2026.7.1-4 at the supplied release commit.
Missing-install retry has no provisioning path: CanRetrySetup permits Unknown and NotInstalled model states without requiring an installed gateway. The new recovery list omits distro creation, CLI installation, and gateway service installation, but later invokes VerifyLocalAiWslStep against that distro.
Findings 2 actionable findings [P1] [P1] Preserve provisioning when the managed gateway is absent
[P1] [P1] Bound the default output drain by the command deadline
Security None None.

How this fits together

The Windows companion launches setup when users retry Local AI installation. Setup prepares the native inference runtime, connects it to the WSL gateway, and returns users to the companion.

flowchart TD
 A[Local AI retry] --> B[Setup window]
 B --> C[Local AI review]
 C --> D[Recovery pipeline]
 D --> E[Native inference runtime]
 D --> F[Existing WSL gateway]
 F --> G[Gateway restart and health check]
Loading

Before merge

  • [P1] Preserve provisioning when the managed gateway is absent (P1) - CanRetrySetup allows a missing or unknown model without requiring an existing gateway. This unconditional redirect therefore sends users with no app-owned distro, including interrupted first installations, into a pipeline that never creates the distro or installs the gateway. It eventually fails at VerifyLocalAiWslStep after potentially downloading and verifying the model. Detect the existing installation before selecting recovery and retain a usable provisioning route for genuinely missing installations without recreating existing ones.
  • [P1] Bound the default output drain by the command deadline (P1) - When the direct child exits but a descendant retains stdout or stderr, this wait never observes timeoutCts: the caller passes only ct. A finite-timeout command can now remain pending indefinitely, and RunInWslAsync has no way to opt into the escape flag. The existing pipe-holder fixture demonstrates the trigger, but its test now explicitly enables the flag. Allow trailing output to drain within the remaining deadline and retain default-path coverage for inherited handles.
  • Resolve merge risk (P1) - Fresh-install and existing-install recovery have not been demonstrated through the declared Windows proof pools.
  • Resolve merge risk (P1) - The supplied tests do not establish complete output capture and bounded completion together for real commands whose descendants retain redirected handles.
  • Complete next step (P2) - Repair missing-gateway routing and deadline-aware output draining, then complete the declared Windows recovery proof before landing.

Findings

  • [P1] [P1] Preserve provisioning when the managed gateway is absent — src/OpenClaw.Tray.WinUI/Presentation/LocalAiPageViewModel.cs:191
  • [P1] [P1] Bound the default output drain by the command deadline — src/OpenClaw.SetupEngine/CommandRunner.cs:262
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test changes production +200/-46; tests +97/-6 Production growth implements the dedicated recovery route and output-drain policy; tests primarily cover routing, pipeline shape, and command capture.

Merge-risk options

Maintainer options:

  1. Preserve both recovery entry states and bounded execution (recommended)
    Repair missing-gateway routing and restore deadline-aware output draining, then validate fresh installation and existing-gateway recovery.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Preserve non-destructive recovery for existing managed gateways, route genuinely absent installations through supported provisioning without deleting existing distributions, and bound default command output draining by the remaining command deadline while retaining trailing output; add focused regression coverage.

Technical review

Best possible solution:

Keep recovery non-destructive, select provisioning only when the managed gateway is genuinely absent, and preserve command deadlines while draining trailing output.

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

Yes, source establishes both failure paths: retry with no managed gateway reaches WSL verification without provisioning, and a normally exited command with a pipe-holding descendant bypasses its timeout. Neither was executed during this read-only review.

Is this the best way to solve the issue?

Partly: reusing setup steps in a dedicated recovery pipeline is appropriate, but unconditional recovery selection and unlimited default output draining are not safe replacements for the existing contracts.

Full review comments:

  • [P1] [P1] Preserve provisioning when the managed gateway is absent — src/OpenClaw.Tray.WinUI/Presentation/LocalAiPageViewModel.cs:191
    CanRetrySetup allows a missing or unknown model without requiring an existing gateway. This unconditional redirect therefore sends users with no app-owned distro, including interrupted first installations, into a pipeline that never creates the distro or installs the gateway. It eventually fails at VerifyLocalAiWslStep after potentially downloading and verifying the model. Detect the existing installation before selecting recovery and retain a usable provisioning route for genuinely missing installations without recreating existing ones.
    Confidence: 0.97
  • [P1] [P1] Bound the default output drain by the command deadline — src/OpenClaw.SetupEngine/CommandRunner.cs:262
    When the direct child exits but a descendant retains stdout or stderr, this wait never observes timeoutCts: the caller passes only ct. A finite-timeout command can now remain pending indefinitely, and RunInWslAsync has no way to opt into the escape flag. The existing pipe-holder fixture demonstrates the trigger, but its test now explicitly enables the flag. Allow trailing output to drain within the remaining deadline and retain default-path coverage for inherited handles.
    Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.97

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 305bb4ef5334.

Labels

Label changes:

  • add P0: The linked report describes destruction of an existing WSL sandbox and blocked in-product recovery.
  • add merge-risk: 🚨 compatibility: The unconditional recovery route removes provisioning from retry when the managed gateway is absent.
  • add merge-risk: 🚨 availability: Default command execution can remain pending beyond its timeout when inherited output handles stay open.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
  • add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The author is a collaborator, so the external-contributor proof gate does not apply. The captured evidence covers command routing and pipeline shape through tests; it explicitly leaves the setup window, native inference recovery, and preserved WSL gateway behavior unverified in the declared maintainer-scheduled pools.

Label justifications:

  • P0: The linked report describes destruction of an existing WSL sandbox and blocked in-product recovery.
  • merge-risk: 🚨 compatibility: The unconditional recovery route removes provisioning from retry when the managed gateway is absent.
  • merge-risk: 🚨 availability: Default command execution can remain pending beyond its timeout when inherited output handles stay open.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🌊 off-meta tidepool and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Not applicable: The author is a collaborator, so the external-contributor proof gate does not apply. The captured evidence covers command routing and pipeline shape through tests; it explicitly leaves the setup window, native inference recovery, and preserved WSL gateway behavior unverified in the declared maintainer-scheduled pools.

Evidence

Acceptance criteria:

  • [P1] ./build.ps1.
  • [P1] dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore.
  • [P1] dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore.
  • [P1] dotnet test ./tests/OpenClaw.SetupEngine.Tests/OpenClaw.SetupEngine.Tests.csproj --no-restore.
  • [P1] ./scripts/validate-mxc-e2e.ps1.

What I checked:

  • Current main and release still need the fix: Both fetched main and the supplied latest-release commit route RetrySetup to ShowOnboarding. The related open issue documents sandbox destruction and an unusable recovery screen. (src/OpenClaw.Tray.WinUI/Presentation/LocalAiPageViewModel.cs:191, 305bb4ef5334)
  • Latest-release comparison: The same onboarding retry route exists in v2026.7.1-4 at the supplied release commit. (src/OpenClaw.Tray.WinUI/Presentation/LocalAiPageViewModel.cs:191, c7c6fb06be03)
  • Missing-install retry has no provisioning path: CanRetrySetup permits Unknown and NotInstalled model states without requiring an installed gateway. The new recovery list omits distro creation, CLI installation, and gateway service installation, but later invokes VerifyLocalAiWslStep against that distro. (src/OpenClaw.SetupEngine/SetupPipeline.cs:52, c44f02325677)
  • Output drain bypasses the command deadline: The default drain branch waits only on caller cancellation, while RunAsync passes ct instead of its timeout token. A descendant retaining redirected handles can therefore keep the call pending beyond its timeout. The existing pipe-holder test now opts into the escape flag instead of covering the default behavior. (src/OpenClaw.SetupEngine/CommandRunner.cs:262, c44f02325677)
  • Validation and declared proof coverage: The captured PR body reports a successful build and Shared, Tray, and SetupEngine tests, with 3937, 2858, and 1058 passing respectively. It explicitly reports no interactive WinUI, NVIDIA, or gateway recovery proof. Repository proof-pool guidance treats these declarations as pending work, not completed evidence. No builds, tests, or reviewer helpers were executed during this read-only review. (docs/PROOF_POOLS.md:27, c44f02325677)
  • Feature-history routing: Available main history connects Dallin Romney to command-drain maintenance, Joel to managed Local AI, and Scott Hanselman to Local AI setup actions. Blame inspection could not complete because missing historical objects required an unavailable network fetch; no source-line introduction attribution is claimed. (src/OpenClaw.SetupEngine/CommandRunner.cs, cb610b2b3d4e)

Likely related people:

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

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Cover absent and existing managed gateways with distinct, non-destructive retry routes.
  • Prove bounded completion and complete trailing output together for normal and inherited-pipe commands.
  • Complete the declared Windows proof pools with redacted evidence of retry, preserved gateway state, and successful recovery.

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.

@shanselman

Copy link
Copy Markdown
Collaborator

Superseded by draft #1357, which preserves @karkarl's non-destructive Local AI recovery direction, removes the unrelated CommandRunner changes, updates onto current main, and adds owner-bound provision/recovery routing plus rollback safety.

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. P0 Emergency: data loss, security bypass, crash loop, or unusable core runtime. 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.

Local AI: "Retry setup or download" destroys WSL sandbox, then "Install & setup" stays greyed out

2 participants