fix(local-ai): add non-destructive recovery - #1384
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
Codex review: needs real behavior proof before merge. Reviewed September 11, 2026, 2:32 PM ET / 18:32 UTC (Revision 9). ClawSweeper reviewWhat this changesAdds a dedicated Local AI retry flow that preserves the existing WSL gateway while repairing installation assets, updating provider routing, and coordinating rollback. Merge readiness⛔ Blocked before merge - 13 items remain The recovery fix remains necessary on main. Both prior rollback blockers remain, and integration with the busy-GPU change exposes another installation blocker. Likely related people: shanselman and joelagnel (routing candidates, medium confidence). Priority: P0 Review scores
Verification
How this fits togetherLocal AI setup connects the Windows model runtime to the Gateway running in WSL. Recovery uses gateway records and installation receipts to select the existing installation, repair its assets, and update where the Gateway sends inference requests. flowchart TD
A[Local AI retry] --> B[Check gateway ownership]
B --> C[Existing gateway recovery]
B --> D[Fresh setup or visible warning]
C --> E[Repair model and runtime]
E --> F[Update Gateway provider]
F --> G[Restart and complete]
F --> H[Coordinate route and receipt rollback]
Decision needed
Why: The PR explicitly leaves these capacity-dependent checks blocked and requests maintainer acceptance; source inspection cannot supply that acceptance. Before merge
Findings
Agent review detailsSecurityNeeds attention: Recovery can republish a stale endpoint before validating its live listener; no unrelated supply-chain changes were found. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Keep the in-product non-destructive repair flow, bind rollback to verified live listener ownership, coordinate process cleanup with the retained route, and preserve deferred model loading. Do we have a high-confidence way to reproduce the issue? Yes, from source: main sends retry into general onboarding, and the proposed rollback paths expose explicit stale-listener and same-port triggers. No runtime reproduction was executed in this read-only review. Is this the best way to solve the issue? The dedicated recovery pipeline is the right boundary, but this implementation is not yet safe: listener authorization must precede provider restoration, cleanup must follow the actual retained route, and installation must retain main's deferred-load behavior. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against c71121cdcdc9. LabelsLabel justifications:
EvidenceSecurity concerns:
What I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (8 earlier review cycles)
|
Route retry through an owner-bound recovery pipeline, preserve existing WSL gateways, and fall back to provisioning only when managed gateway absence is proven. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4f97b3a3-def7-4f33-b7d5-ece171b4fd0b
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4f97b3a3-def7-4f33-b7d5-ece171b4fd0b
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4f97b3a3-def7-4f33-b7d5-ece171b4fd0b
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4f97b3a3-def7-4f33-b7d5-ece171b4fd0b
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4f97b3a3-def7-4f33-b7d5-ece171b4fd0b
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4f97b3a3-def7-4f33-b7d5-ece171b4fd0b
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4f97b3a3-def7-4f33-b7d5-ece171b4fd0b
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4dd93324-efc9-4189-83fd-baf07a032781
bded03c to
a008e42
Compare
Preserve non-destructive recovery rollback semantics while adopting the schema-4 Hugging Face cache and runtime identity contract. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 23b83636-cf4a-4df3-aa00-899a9fbe04a1
…tive replacement runtime during recovery rollback - PreserveLocalAiRecoveryGatewayStep now probes the original (A) llama-server endpoint's health before restoring its manifest receipt on rollback, instead of trusting a stale receipt. If the probe fails, the replacement (B) receipt is preserved rather than pointing Gateway at a possibly-dead process. - StartLocalAiRuntimeStep.RollbackAsync now skips disposing the replacement runtime when a recovery provider-transition rollback could not confirm the Gateway config actually stopped routing to it, avoiding a race where the runtime is killed while Gateway still points at its endpoint. - Added unit tests for both fixes and updated existing recovery-rollback tests to stub the new health probe.
|
Pushed two targeted rollback fixes for the recovery-rollback race conditions identified in review:
Added regression tests for both fixes and updated existing recovery-rollback tests to stub the new health probe. Full validation: \�uild.ps1, Shared.Tests (3991 passed), Tray.Tests (2908 passed), SetupEngine.Tests (1191 passed, 1 pre-existing skip). |
# Conflicts: # tests/OpenClaw.SetupEngine.Tests/SetupPipelineTests.cs
Problem
Replaces #1357 (Add non-destructive Local AI recovery), supersedes #1342 (Add recovery flow for local AI setup), and fixes #1307.
Local AI Retry setup or download could enter the destructive-capable onboarding pipeline, recreate an existing app-owned WSL gateway, or dead-end when the gateway was exact but the Local AI receipt was missing or incomplete.
Fix
Integrated dependencies
621f4100f8b40ac84e844bcc39b01fe03a2238bb.3ce56b8902d558a2d43a1308b4d61ccbf6856274.origin/main7dab793673ff00a43fd74259ed83321894ef601e.LocalAiGatewayProviderCoordinatorTestsoverlap was resolved without weakening either lifecycle contract.Current head:
c9dee4102e2a8f22bb0d7edea081c3d52f357abc, including currentorigin/main7dab793673ff00a43fd74259ed83321894ef601e.Required proof pools
windows-wsl-dgx-blackwell: Not verified / blocked. The available RTX 4080 SUPER exposes 16,376 MiB, below the approximately 22.2 GiB minimum for the smallest current new-install catalog recipe.windows-winui-interactive: Not verified / blocked. The current-catalog hardware gate correctly prevents an enabled recovery review on this host, so no current-head enabled recovery-page screenshot/video is claimed.windows-wsl-gateway-e2e: Not verified / blocked for the real recovery transaction. Hyper-V firewall policy blocks WSL-to-Windows loopback without elevation, so no real existing-gateway provider replacement, restart, inference, and receipt-compensation transaction is claimed.Validation
Current-head local validation:
.\build.ps1: passeddotnet test .\tests\OpenClaw.Shared.Tests\OpenClaw.Shared.Tests.csproj --no-restore: 3,991 passed, 32 skippeddotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore: 2,908 passeddotnet test .\tests\OpenClaw.SetupEngine.Tests\OpenClaw.SetupEngine.Tests.csproj --no-restore: 1,186 passed, 1 environment-dependent cross-volume test skippedLocalAiInstallRecoveryTests: 44 passed, 1 environment-dependent cross-volume test skippeddotnet test .\tests\OpenClaw.Connection.Tests\OpenClaw.Connection.Tests.csproj --no-restore: 793 passed, 1 environment-dependent cross-volume test skippedReviews
Real behavior proof
Current-head automated coverage establishes:
Prior host evidence is intentionally not counted as current-catalog recovery proof: pinned legacy Qwen3.5 9B and llama.cpp
b10655loaded successfully, but that does not satisfy the current catalog or enabled WinUI recovery gate.Not verified / blocked: current-catalog native inference, enabled recovery-page interactive proof, real WSL-to-Windows reachability, and a live existing-gateway provider/restart/receipt recovery transaction. Keep this PR draft with
status: 📣 needs proofuntilwindows-wsl-dgx-blackwelland interactive proof are available or a maintainer explicitly accepts the scoped proof gap.Ownership notes
WindowManager.ShowLocalAiSetupAsyncselects provision/recovery/blocked routing;BuildLocalAiRecoveryStepsowns the exact-owner non-destructive transactionSecurity impact
No new credentials or permissions. Recovery fails closed on ambiguous ownership and binds WSL/provider mutation to one revalidated registry owner, distro, canonical loopback endpoint/port, and retained receipt.