Repository navigation
fix: Gateway keepalive stays off after a failed auto-repair restart - #2111
Conversation
Auto-repair of a setup-managed local Gateway force-terminates the WSL distro before restarting it, and that also kills the keepalive. The keepalive was re-armed only when the restart succeeded, so a failed restart left the distro without one until something else asked for it (about 6 hours in the report). ManagedLocalGatewayRepairCoordinator now tries to re-arm the keepalive after any attempted restart, success or failure, while it still holds the repair lease. It skips the re-arm when the repair was cancelled or the user had already stopped, disconnected, or switched away from the gateway before the re-arm began. That check runs once, before the re-arm; a manual Stop or Switch waits on the lease held across it. The re-arm is best effort: a failure is logged and never changes the repair result. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
🦞👀 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 maintainer review before merge. What this changesRestore the managed WSL Gateway keepalive after failed restart attempts while preserving cancellation and explicit user intent. Example: Auto-repair terminates OpenClawGateway, then its cold restart fails.
Review scores
ProductKind: Bug fix · Worth it: Yes · Fix scope: Complete Merge readiness✅ Ready for maintainer review Keep this PR open: it addresses a source-confirmed recovery defect still present on main, and no introduced correctness defect was identified. Priority: P1 Before mergeNone. FindingsNone. Tests
Agent review detailsHow this fits togetherThe tray repair coordinator receives managed-local connection failures, invokes WSL restart operations, restores keepalive through the existing service, and reconnects only after a successful restart. flowchart TD
A[Connection failure monitor] --> B[Repair coordinator]
B --> C[Lifecycle lease and intent checks]
C --> D[WSL restart adapter]
D --> E[Keepalive restoration]
E --> F[Release lifecycle lease]
F --> G[Return failure or reconnect]
Technical reviewBest possible solution: Restore keepalive in the existing coordinator's guarded cleanup path while retaining the current service, lifecycle lease, restart budget, and user-intent checks. Do we have a high-confidence way to reproduce the issue? Current main establishes termination followed by a fallible cold restart and failure returns that bypass keepalive restoration; no native reproduction ran during this review. Is this the best way to solve the issue? The patch is a focused repair using existing owners, and its attempted-restart guard avoids restoration on budget-only exits without adding a competing supervisor. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 22c111d4c209. Provenance checked
TestingProof path: unit tests only. SecurityNone. EvidenceWhat I checked:
Likely related people:
Preflight sourceSource unknown. Source comparison at the observed PR snapshot only. Completion is self-reported; body, policy, tests, execution, and report bytes are not verified. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. LabelsLabel changes:
Label justifications:
Rating scale6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior. WorkflowClawSweeper edits this one comment on every review. Comment Reviewed November 11, 2026, 5:49 AM ET / 10:49 UTC. |
|
Hi! I'm a Copilot triage agent working overnight for @shanselman (I'm not Scott). I wrote this fix for #2092 (Auto-repair force-terminates the WSL distro but skips the keepalive re-arm when the restart fails): failed recovery can leave keepalive off, and wanted to leave a short status for whoever reviews it. What was checked on
What is not proven
Update at 18:11 UTC: merged the same reviewed head as |
Closes #2092
What Problem This Solves
Fixes: the local WSL Gateway's keepalive stays dead after an auto-repair restart fails, so the distro idles out and restarts uncleanly until something else re-arms it.
User Impact
User impact: when auto-repair force-terminates the Gateway's WSL distro and the restart then fails, the keepalive is now re-armed anyway, so the distro does not drift into repeated unclean stops. Behavior after a successful restart is unchanged. The re-arm is skipped when the repair was cancelled, or the user had already stopped, disconnected, or switched away from the gateway before it began.
Why This Change Was Made
WslManagedLocalGatewayRestarterescalates toWslGatewayController.ForceRestartAsync, which runswsl.exe --terminateand kills every session of the distro, the keepalive (wsl.exe -d <distro> -- sleep infinity) included.ManagedLocalGatewayRepairCoordinatorre-armed the keepalive only on the success path, so a failed or aborted restart after the terminate left the distro without one. In the report that lasted about 6 hours.The re-arm now runs from the inner
finally, while the repair lease is still held (a manual Stop or Switch waits on that lease, so it cannot interleave), and only when a restart was actually attempted. It is skipped if the call was cancelled orGetRestartAbortResultshows the user had already stopped or switched away. That keeps the guard raised in the issue triage: an unconditional re-arm could revive a Gateway the user had just stopped. The re-arm is best effort (an exception is logged and never changes the repair result).WslGatewayKeepAliveService.TryEnsureAsyncremains the policy owner and stays idempotent, so it still skips fixture mode, unmanaged gateways, and an already running keepalive.Known limits: the abort check runs once, before the re-arm starts, and
TryEnsureAsynctakes no cancellation token, so a Disconnect or shutdown that lands during the re-arm itself is not observed. That is benign: Disconnect never touches WSL, Stop runs inside the distro and leaves the keepalive alone (a keepalive that is up after a Stop is the normal state), and the keepalive is detached on purpose. The old success path had the same window. The re-arm also keeps the lease the success path already held, andTryEnsureAsyncinspects WSL processes with its own per-call timeouts, so a manual Stop or Switch queued behind a slow re-arm waits for it. I left both as they are to keep this fix small.Evidence
The reporter's tray log (issue #2092) shows two
wsl.exe --terminate OpenClawGatewaycalls with no[WslKeepAlive]line until about 6 hours later. The real WSL flow needs a wedged distro to reproduce, so I did not reproduce it live. Instead the coordinator tests drive it with fakes and I mutation-checked them against the production change:FailedRestart_StillRearmsKeepAlivecases fail.restartAttemptedguard removed: exactlyRestartBudget_ExhaustsThenResetAllowsAgainfails.SlowRearmAfterFailedRestart_KeepsTheLeaseUntilItCompletesfails.FailedRestart_StillRearmsKeepAlivecases and the slow-re-arm test fail.Also covered: the re-arm runs while the lease is still held, a slow re-arm keeps the lease until it completes, and a re-arm that throws after a failed restart keeps the
RestartFailedresult and still releases the lease. Exact commands and counts are under Validation.Change Type
Scope
winnodeRequired proof pools
none: no visual, setup, pairing, or host-specific surface changes. The decision is covered by coordinator unit tests with fakes. The real WSL path cannot be reproduced without a wedged distro.Validation
./build.ps1: exit 0.dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --filter "FullyQualifiedName~ManagedLocalGatewayRepairCoordinatorTests|FullyQualifiedName~WslGatewayControllerTests|FullyQualifiedName~WslKeepAlivePolicyTests": 63 passed, 0 failed.dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj: 5761 passed, 0 failed, 27 skipped.dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj: 6296 passed, 0 failed.git diff --check: clean. Fresh worktree, so--no-restorewas omitted on the first run.OPENCLAW_REPO_ROOTpointed at the worktree and inheritedOPENCLAW_*variables were cleared. Before and after file listings of%LOCALAPPDATA%\OpenClawTrayand%APPDATA%\OpenClawTraywere identical.Real Behavior Proof
22c111d4.ManagedLocalGatewayRepairCoordinatorTests, 28 tests).RestartFailed; a slow re-arm keeps the lease until it completes; a re-arm that throws is logged and the lease is still released; user stop, gateway switch, and cancellation record zero re-arms; a restart budget that is exhausted and then reset re-arms only for attempted restarts.Yes/No/N/A):N/A, no visual change.wsl.exe --terminate. That needs a wedged distro, so it is covered by the unit tests above and the reporter's log instead.Security Impact
Yes/No):NoYes/No):NoYes/No):NoYes/No):No. The same existing keepalive call is made in more cases, still gated byWslKeepAlivePolicy.CanManageGateway.Yes/No):NoYes, explain the risk and mitigation: N/ACompatibility and Migration
Yes/No):YesYes/No):NoYes/No):NoReview Conversations
Prepared by a Copilot agent on behalf of @shanselman during overnight triage; it has not had a human review yet.