Skip to content

fix: preserve Core updates across restart timeouts - #1181

Open
TheAngryPit wants to merge 1 commit into
openclaw:mainfrom
TheAngryPit:fix/update-timeout-no-force-terminate
Open

fix: preserve Core updates across restart timeouts#1181
TheAngryPit wants to merge 1 commit into
openclaw:mainfrom
TheAngryPit:fix/update-timeout-no-force-terminate

Conversation

@TheAngryPit

Copy link
Copy Markdown
Contributor

Related: openclaw/openclaw#99666

What Problem This Solves

Fixes an issue where the Windows Companion could interrupt a supported OpenClaw Core update when its automatic gateway repair timed out. After waiting 30 seconds for an in-distro restart, the Companion treated the timeout as a definitive failure and terminated the entire WSL distro, killing the detached Core updater.

Why This Change Was Made

The WSL command runner now carries an explicit timeout result through the gateway controller. Automatic repair treats that result as indeterminate and skips forced distro termination. Definitive non-timeout restart failures retain the existing terminate-and-cold-restart recovery path.

This stays on the Windows Companion boundary. Core already owns the managed-service update handoff and its parent wait budget; the destructive escalation happened later in the external Windows supervisor.

User Impact

Long-running Core updates are no longer killed merely because the Companion's host-side restart wait expired. Users still retain automatic recovery when an in-distro restart fails definitively.

Evidence

Observed before the fix:

  • 21:02:57: Core accepted update.run and started the managed-service handoff.
  • 21:03:21: Companion auto-repair invoked an in-distro gateway restart.
  • 21:03:51: the 30-second wait timed out and Companion invoked wsl.exe --terminate, killing the detached updater.
  • Core remained clean at the old commit while only FETCH_HEAD advanced.
  • With the Companion stopped, the supported CLI update completed successfully.

The regression test now proves that a timed-out in-place restart makes zero terminate calls, while existing tests continue to prove that a definitive failure performs the terminate-and-cold-restart path.

Change Type

  • Bug fix
  • Feature
  • Refactor
  • Docs or instructions
  • Tests or validation
  • Security hardening
  • Chore or infrastructure

Scope

  • Tray or WinUI UX
  • Windows node capability
  • Local MCP or winnode
  • Gateway, connection, or pairing
  • Setup or onboarding
  • Permissions, privacy, or security
  • Tests, CI, or docs

Validation

  • dotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore --filter "FullyQualifiedName~WslGatewayControllerTests": 11 passed, 0 failed, 0 skipped.
  • .\build.ps1: all 5 projects built; 46 documentation files validated.
  • dotnet test .\tests\OpenClaw.Shared.Tests\OpenClaw.Shared.Tests.csproj --no-restore: 3,698 passed, 0 failed, 32 intentional environment/integration skips.
  • dotnet test .\tests\OpenClaw.Tray.Tests\OpenClaw.Tray.Tests.csproj --no-restore: 2,621 passed, 0 failed, 0 skipped.
  • git diff --check: clean apart from Git line-ending conversion notices.

Real Behavior Proof

  • Environment tested: Windows 11 with WSL 2; isolated source worktree based on Windows Companion origin/main 12b84457a46492b8e3f4df99ea1ea17370f75992.
  • PR head or commit tested: e4dd1735b47eb7cadc19e0c73e9fea1c93d87e09.
  • Exact steps or command run: focused regression plus the required full repository validation commands listed above.
  • Evidence after fix: the timeout regression returns failure with completion unknown, records zero distro terminations, and records one in-distro restart attempt. The existing definitive-failure regression still records one termination and the cold restart.
  • Observed result: test_proven for the timeout decision and preserved definitive-failure path.
  • Screenshot or artifact links verified? (Yes/No/N/A): N/A.
  • Not verified or blocked: the installed official Companion remains unmodified, so the fixed source path is not yet runtime_proven or end_to_end_proven.

Security Impact

  • New permissions or capabilities? (Yes/No): No.
  • Secrets or tokens handling changed? (Yes/No): No.
  • New or changed network calls? (Yes/No): No.
  • Command or tool execution surface changed? (Yes/No): Yes.
  • Data access scope changed? (Yes/No): No.
  • If any answer is Yes, explain the risk and mitigation: automatic repair no longer invokes the destructive host-side terminate command after an ambiguous timeout. The existing forced recovery remains available after a definitive non-timeout failure.

Compatibility and Migration

  • Backward compatible? (Yes/No): Yes.
  • Config or environment changes? (Yes/No): No.
  • Migration needed? (Yes/No): No.
  • If yes, list the exact upgrade steps: N/A.

Review Conversations

  • I replied to or resolved every bot review conversation addressed by this PR.
  • I left unresolved only conversations that still need maintainer judgment.

@clawsweeper

clawsweeper Bot commented Aug 19, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

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

@clawsweeper clawsweeper Bot added merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Aug 19, 2026
@clawsweeper

clawsweeper Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed August 21, 2026, 4:48 PM ET / 20:48 UTC.

ClawSweeper review

What this changes

The branch carries a WSL command timeout through gateway restart recovery so an ambiguous timeout skips forced distro termination while confirmed failures still cold-restart.

Merge readiness

⚠️ Ready for maintainer review - 2 items remain

This PR remains necessary: current main still collapses a WSL timeout into ordinary failure and then force-terminates the distro. The focused patch preserves definitive-failure recovery, has no blocking correctness findings, and the updated real Windows/WSL proof is sufficient.

Priority: P2
Reviewed head: e4dd1735b47eb7cadc19e0c73e9fea1c93d87e09

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused, well-covered recovery-policy fix with convincing real WSL runtime evidence.
Proof confidence 🐚 platinum hermit (4/6) Sufficient (logs): The contributor supplied exact-head, real Windows/WSL production-boundary proof for both the timeout-preserving path and the retained definitive-failure recovery path.
Patch quality 🦞 diamond lobster (5/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (logs): The contributor supplied exact-head, real Windows/WSL production-boundary proof for both the timeout-preserving path and the retained definitive-failure recovery path.
Evidence reviewed 6 items Current-main behavior remains unfixed: Current main returns the same failed result for a timed-out WSL command and the tray adapter escalates every unsuccessful restart to ForceRestartAsync, so it still has the reported destructive path.
Timeout is now preserved end to end: The patch marks a runner timeout explicitly, forwards it through the gateway controller, and returns before the force-terminate branch when completion is unknown.
Both recovery outcomes are covered: The added regression asserts zero termination calls for a timeout, while the adjacent existing test retains terminate-plus-cold-restart coverage for a definitive failure.
Findings None None.
Security None None.

Live Verification

Command: dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore --filter "FullyQualifiedName~WslGatewayControllerTests"

Result: FAIL (failed) — execution before step 1 run: sh -lc pnpm install --ignore-scripts --frozen-lockfile failed: ! Corepack is about to download https://registry.npmjs.org/pnpm/-/pnpm-11.22.0.tgz

sh -lc pnpm install --ignore-scripts --frozen-lockfile failed: ! Corepack is about to download https://registry.npmjs.org/pnpm/-/pnpm-11.22.0.tgz

Assertions:

  • FAIL expect_output: Passed!

How this fits together

The tray’s managed-local gateway repair restarts an app-owned WSL gateway after a verified transport failure. Its restart result determines whether the repair coordinator reconnects normally or performs a destructive cold restart of the distro.

flowchart LR
A[Core update handoff] --> B[Gateway repair monitor]
B --> C[In-distro gateway restart]
C --> D{Restart result}
D -->|Confirmed failure| E[Terminate and cold restart]
D -->|Timed out| F[Wait for later recovery]
E --> G[Gateway reconnect]
F --> G
Loading

Before merge

  • Resolve merge risk (P2) - Availability tradeoff: a genuinely wedged distro that presents as a host-side timeout now waits for a later repair attempt instead of being cold-restarted immediately.
  • Complete next step (P2) - No discrete mechanical repair remains: the patch has no actionable review finding and now includes sufficient real-behavior proof.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Patch scope 3 production files changed, 1 test file changed The change is confined to timeout propagation, the restart escalation decision, and regression coverage.
Production versus test growth production +21/-4, tests +23/-0 The small production policy change is matched by a targeted timeout regression test.

Merge-risk options

Maintainer options:

  1. Accept the timeout-safety tradeoff (recommended)
    Land the explicit timeout branch: it avoids interrupting an active update while retaining immediate cold recovery for definite restart failures.

Technical review

Best possible solution:

Keep timeout as an explicit indeterminate outcome, reserve cold restart for confirmed failures, and let the existing bounded repair monitor observe later recovery.

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

Yes. Current main still routes any unsuccessful in-distro restart to force termination, and the contributor supplied a current-head real WSL 2 before-and-after recovery run for that exact path.

Is this the best way to solve the issue?

Yes. Propagating an explicit timeout state at the WSL runner boundary is the narrowest maintainable way to distinguish unknown completion from a confirmed restart failure.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against d3ed6c78bf14.

Labels

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The contributor supplied exact-head, real Windows/WSL production-boundary proof for both the timeout-preserving path and the retained definitive-failure recovery path.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (logs): The contributor supplied exact-head, real Windows/WSL production-boundary proof for both the timeout-preserving path and the retained definitive-failure recovery path.
  • remove status: 📣 needs proof: Current PR status label is status: 👀 ready for maintainer look.
  • remove rating: 🦐 gold shrimp: Current PR rating is rating: 🐚 platinum hermit, so this older rating label is no longer current.

Label justifications:

  • P2: This fixes a bounded gateway-recovery failure that can interrupt a supported Core update.
  • merge-risk: 🚨 availability: The changed decision controls whether an unavailable managed WSL gateway is immediately cold-restarted or left for later recovery.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (logs): The contributor supplied exact-head, real Windows/WSL production-boundary proof for both the timeout-preserving path and the retained definitive-failure recovery path.
  • proof: sufficient: Contributor real behavior proof is sufficient. The contributor supplied exact-head, real Windows/WSL production-boundary proof for both the timeout-preserving path and the retained definitive-failure recovery path.

Evidence

What I checked:

Likely related people:

  • Ranjesh: History search attributes the introduction of the WSL managed-gateway restarter surface to the gateway terminal-controls work. (role: feature introducer; confidence: high; commits: 0b2f9fea3dd9; files: src/OpenClaw.Tray.WinUI/Services/ManagedLocalGatewayRepairAdapters.cs)
  • Barbara Kudiess: The WSL controller and related tests trace through the setup-wizard dependency-recovery work. (role: adjacent WSL recovery contributor; confidence: medium; commits: 1e6951331f1c; files: src/OpenClaw.Connection/WslGatewayController.cs, tests/OpenClaw.Tray.Tests/WslGatewayControllerTests.cs)

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.

History

Review history (1 earlier review cycle)
  • reviewed 2026-08-19T23:44:29.070Z sha e4dd173 :: needs real behavior proof before merge. :: none

@TheAngryPit

Copy link
Copy Markdown
Contributor Author

Exact-head real Windows/WSL recovery proof (e4dd1735)

Fresh runtime proof now exercises the exact production recovery chain on Windows with WSL 2:

WslExeCommandRunner -> WslGatewayController -> WslManagedLocalGatewayRestarter

Environment: Windows 10.0.26200.9267, WSL 2.7.12.0, uniquely imported disposable distro OpenClawTimeoutLab-1181. The source was the already disposable OpenClawUpdateLab. No production Gateway, Companion settings, credentials, pairing, provider request or proof listener was used.

Timeout path

  • Exact 30-second runner timeout observed at 30,140 ms.
  • Result propagated as timed out and the restarter returned completion is unknown without force termination.
  • The PID 1 plus mount-namespace lifetime fingerprint remained unchanged immediately, after the detached 35-second handoff completed, and through a later successful recovery invocation.
  • Detached completion marker: present.
  • Cold-restart marker: absent.

Definitive-failure path

  • First real in-distro restart invocation exited 42, not timed out.
  • The same production restarter retained host-side terminate plus cold restart.
  • Final result: success.
  • Distro lifetime fingerprint changed, the pre-terminate sentinel was gone, and both attempt markers were present.

Fresh validation

  • Full repository build: all five projects passed; 46 documentation files validated.
  • Shared: 3,698 passed, 0 failed, 32 environment-only skipped.
  • Tray: 2,621 passed, 0 failed.
  • Scoped structured review: TruffleHog clean; no accepted/actionable findings; patch correct at confidence 0.99.

Redacted runtime artifact SHA-256: EA4EDE3EC62854BB60AABEEF61B1E3317AC7A775CA21D417287297A5D11B7FA9.

Cleanup completed: only OpenClawTimeoutLab-1181 was unregistered; its generated export and VHDX were removed. Final readback shows Ubuntu, OpenClawUpdateLab and OpenClawGateway stopped, no OpenClaw process, and no checked proof/Gateway listener.

Proof level: runtime-proven for the exact timeout-versus-definitive-failure owner boundary on a real disposable WSL 2 distro. This does not claim full Companion UI end-to-end proof or installed-package promotion.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Aug 21, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Aug 21, 2026
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. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants