Skip to content

fix(infra): include update timeout in managed-service handoff parent exit wait#99695

Merged
steipete merged 11 commits into
openclaw:mainfrom
ZOOWH:fix/99666-handoff-parent-timeout
Jul 6, 2026
Merged

fix(infra): include update timeout in managed-service handoff parent exit wait#99695
steipete merged 11 commits into
openclaw:mainfrom
ZOOWH:fix/99666-handoff-parent-timeout

Conversation

@ZOOWH

@ZOOWH ZOOWH commented Jul 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What Problem This Solves

Fixes #99666. Gateway self-update fails with managed-service-handoff-parent-timeout because the parent exit grace period is hardcoded to 60 seconds. When the gateway is draining active tasks, the parent doesn't exit within 60s and the handoff fails.

Root Cause / Fix

startManagedServiceUpdateHandoff computed parentExitTimeoutMs as restartDelayMs + 60_000 regardless of the update timeout. The caller already passes timeoutMs but it was not used.

Fix: Raise the default to 300s (matching typical restart drain budget), use Math.max(300_000, timeoutMs ?? 0) so larger explicit timeouts also extend the parent exit wait, and add a 30s shutdown/scheduling reserve so the helper does not time out before the parent finishes shutting down after a full drain window.

Changes (2 files, +18/-2):

  • src/infra/update-managed-service-handoff.ts: 2 lines changed + 30s reserve constant
  • src/infra/update-managed-service-handoff.test.ts: 2 tests updated for new reserve

Evidence

Production Function Proof

$ node --import tsx proof.ts
Proof: parentExitTimeoutMs includes 30 s shutdown reserve

  PASS default (no timeoutMs) = 330000
  PASS 5min timeout = 330000
  PASS small timeout (floor at 300s + reserve) = 330000
  PASS 30min + 500ms restart = 1830500

ALL CHECKS PASSED

Test Runner

$ node scripts/run-vitest.mjs src/infra/update-managed-service-handoff.test.ts
 Test Files  1 passed (1)
      Tests  13 passed (13)

Autoreview + Formatting

autoreview → clean (0.95, zero findings)
oxfmt --check → clean  |  oxlint → clean  |  git diff --check → clean

Change Type

  • fix — bug fix for gateway self-update

Security Impact

  • None. Changes only the timeout calculation; no new permissions or dependencies.

@clawsweeper

clawsweeper Bot commented Jul 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 6, 2026, 3:06 AM ET / 07:06 UTC.

Summary
The branch aligns managed-service update handoff parent-exit timing with Gateway restart-drain settings and expands update.run/update.auto restart handoff handling, with focused tests and a changelog entry.

PR surface: Source +108, Tests +304, Docs +1. Total +413 across 13 files.

Reproducibility: yes. by source inspection. Current main and v2026.6.11 can drain restart work for the default 300s while the managed-service helper times out after restart delay plus 60s; this review did not run a live Windows npm-global update.

Review metrics: 3 noteworthy metrics.

  • Timeout wait behavior: 1 default changed, 1 indefinite path added. Managed handoff parent-exit timeout now follows restart-drain configuration plus reserve instead of a fixed 60s wait, so failure timing changes before merge.
  • Operator override behavior: 1 environment override changed. OPENCLAW_NO_RESPAWN no longer disables supervised update exits, which is operator-visible compatibility behavior.
  • Release-owned changelog: 1 entry added. The PR edits CHANGELOG.md, which repository policy reserves for release generation rather than normal PR branches.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #99666
Summary: This PR is the open candidate fix for the managed-service handoff parent-timeout bug described by the linked issue; adjacent merged PRs introduced and refined the same handoff surface but did not close this timeout mismatch.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
Result: blocked until stronger real behavior proof is added.

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

Rank-up moves:

  • [P1] Add current-head managed update.run or update.auto proof with redacted logs, terminal output, or a reproducible validation transcript.
  • [P1] Have the likely owner accept or tune the longer parent wait and OPENCLAW_NO_RESPAWN behavior before merge.
  • Remove the CHANGELOG.md entry and keep release-note context in the PR body or squash message.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The PR body includes terminal proof for the older two-file timeout calculation, but current head now changes the managed update run loop and auto-update paths without matching live logs or a validation transcript. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P2] Current-head real behavior proof is insufficient: the posted terminal proof covers the older two-file timeout calculation, not the expanded update.run/update.auto, respawn, and run-loop behavior at df47c3a.
  • [P2] Merging intentionally delays managed-handoff parent-timeout failure to the configured restart-drain budget plus reserve, and an indefinite drain config now makes the helper wait indefinitely for the parent to exit.
  • [P1] Merging changes operator compatibility for supervised update restarts because OPENCLAW_NO_RESPAWN no longer prevents exiting to the service manager in this path.

Maintainer options:

  1. Refresh proof and accept the policy (recommended)
    Require current-head managed update handoff proof for update.run or update.auto, then land if maintainers accept the longer wait and supervised exit behavior.
  2. Tune wait or override semantics before merge
    Adjust the parent-exit budget or OPENCLAW_NO_RESPAWN handling if maintainers want faster stuck-parent recovery or stronger operator override compatibility.
  3. Replace with a narrower repair
    Close or pause this branch if the expanded restart-upgrade behavior is not worth the compatibility and availability risk for this bug.

Next step before merge

  • [P1] The next action is maintainer policy acceptance plus current-head real behavior proof; ClawSweeper cannot supply the contributor/live proof automatically.

Maintainer decision needed

  • Question: Should this expanded managed-update policy land as written, including configured or indefinite parent-exit waits and supervised update exits despite OPENCLAW_NO_RESPAWN, once current-head proof is refreshed?
  • Rationale: The code path appears coherent, but it changes availability timing and an operator override beyond the stale PR proof, so maintainer intent is needed before merge.
  • Likely owner: steipete — steipete is assigned and authored the current-head policy expansion commits that need the acceptance decision.
  • Options:
    • Accept current policy after proof (recommended): Refresh current-head managed update proof, then land if maintainers accept slower stuck-parent failure detection and supervised update exits as the right reliability tradeoff.
    • Tune compatibility first: Cap or split the parent-exit wait and preserve, document, or explicitly retire OPENCLAW_NO_RESPAWN behavior before merge.
    • Re-scope to timeout-only fix: Pause this branch and replace it with a narrower PR that only aligns the helper parent wait with the default restart drain.

Security
Cleared: No concrete security or supply-chain regression was found; the diff changes existing process/update handoff behavior but adds no dependency, workflow, permission, secret, or package-resolution surface.

Review findings

  • [P3] Remove the release-owned changelog entry — CHANGELOG.md:28
Review details

Best possible solution:

Land after current-head managed-service update proof, explicit maintainer acceptance or tuning of the wait and OPENCLAW_NO_RESPAWN behavior, and removal of the release-owned changelog entry.

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

Yes by source inspection. Current main and v2026.6.11 can drain restart work for the default 300s while the managed-service helper times out after restart delay plus 60s; this review did not run a live Windows npm-global update.

Is this the best way to solve the issue?

Yes for the owner boundary: the shared managed-service handoff helper and Gateway restart loop are the right places to align parent-exit wait with restart-drain behavior. The current branch still needs current-head proof, maintainer acceptance for the compatibility tradeoff, and removal of the release-owned changelog edit.

Full review comments:

  • [P3] Remove the release-owned changelog entry — CHANGELOG.md:28
    Late catch: this line was already present in the earlier reviewed head, but AGENTS.md says CHANGELOG.md is release-owned and normal PRs should leave release-note context in the PR body, squash message, or commit message. Please drop this entry so release generation owns the final changelog.
    Confidence: 0.91
    Late finding: first raised on code an earlier review cycle already covered.

Overall correctness: patch is correct
Overall confidence: 0.84

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • remove rating: 🦐 gold shrimp: Current PR rating is rating: 🦪 silver shellfish, so this older rating label is no longer current.

Label justifications:

  • P2: The PR targets a bounded but real managed Gateway self-update availability bug rather than a repo-wide emergency.
  • merge-risk: 🚨 compatibility: The diff changes OPENCLAW_NO_RESPAWN behavior for supervised update restarts, which can affect existing operator setups.
  • merge-risk: 🚨 availability: The diff changes process restart, shutdown, parent-wait, and helper timeout behavior for managed update handoffs.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body includes terminal proof for the older two-file timeout calculation, but current head now changes the managed update run loop and auto-update paths without matching live logs or a validation transcript. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +108, Tests +304, Docs +1. Total +413 across 13 files.

View PR surface stats
Area Files Added Removed Net
Source 6 173 65 +108
Tests 6 328 24 +304
Docs 1 1 0 +1
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 13 502 89 +413

What I checked:

  • Repository policy read: Root AGENTS.md and scoped gateway AGENTS.md files were read fully; their PR review, proof, compatibility, gateway, and changelog guidance affected this review. (AGENTS.md:1, 86d53890943a)
  • Current main still has fixed helper wait: Current main defines a fixed 60000ms parent-exit grace and serializes parentExitTimeoutMs as restartDelayMs plus that fixed grace. (src/infra/update-managed-service-handoff.ts:22, 86d53890943a)
  • Current main restart drain can exceed helper wait: Current main defaults Gateway restart drain to 300000ms, which can keep the parent alive longer than the helper's fixed 60s parent wait. (src/cli/gateway-cli/run-loop.ts:21, 86d53890943a)
  • Latest release still has the mismatch: v2026.6.11 still has PARENT_EXIT_GRACE_MS at 60000ms and parentExitTimeoutMs as restartDelayMs plus that fixed grace, so this fix is not shipped. (src/infra/update-managed-service-handoff.ts:21, e085fa1a3ffd)
  • PR head helper behavior: At PR head, startManagedServiceUpdateHandoff accepts restartDrainTimeoutMs and serializes parentExitTimeoutMs as restartDelayMs plus restartDrainTimeoutMs plus a 30000ms reserve, or null for an indefinite configured drain. (src/infra/update-managed-service-handoff.ts:660, df47c3a0aa11)
  • PR head update.run path: At PR head, update.run passes resolved gateway reload deferral timeout into the managed handoff helper and schedules the SIGUSR1 restart immediately after helper creation. (src/gateway/server-methods/update.ts:247, df47c3a0aa11)

Likely related people:

  • steipete: Assigned on the live PR and authored the current-head expansion across managed handoff timing, update.run/update.auto, process respawn, and run-loop behavior. (role: recent area contributor and likely follow-up owner; confidence: high; commits: df47c3a0aa11, 678c9de11f8d, 57ded4c0bfab; files: src/infra/update-managed-service-handoff.ts, src/gateway/server-methods/update.ts, src/infra/update-startup.ts)
  • BKF-Gitty: Merged PR metadata for the managed update.run handoff shows this contributor authored the original supervised handoff flow that owns the parent-exit wait behavior. (role: introduced managed handoff behavior; confidence: high; commits: 22a0ca9e3b22; files: src/infra/update-managed-service-handoff.ts, src/gateway/server-methods/update.ts, src/infra/update-control-plane-sentinel.ts)
  • giodl73-repo: Merged systemd update-handoff work changed the same managed-service helper and Gateway update surfaces for supervised service behavior. (role: adjacent platform repair contributor; confidence: medium; commits: 1f28c3e42b5e; files: src/infra/update-managed-service-handoff.ts, src/gateway/server-methods/update.ts)
  • 0xRain: History for the Gateway run loop shows this contributor added active-turn restart drain behavior that the helper wait now needs to respect. (role: introduced restart-drain invariant; confidence: medium; commits: acb9cbb89834; files: src/cli/gateway-cli/run-loop.ts)
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.
Review history (4 earlier review cycles)
  • reviewed 2026-07-04T01:13:50.678Z sha 45d5580 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-06T01:23:47.748Z sha 45d5580 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-06T06:20:02.249Z sha 844bd39 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-06T06:27:34.284Z sha 844bd39 :: needs real behavior proof before merge. :: none

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. 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. P2 Normal backlog priority with limited blast radius. labels Jul 3, 2026
@ZOOWH
ZOOWH force-pushed the fix/99666-handoff-parent-timeout branch from f584667 to 4a03bb7 Compare July 3, 2026 23:29
@ZOOWH

ZOOWH commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Addressed P2: default parent-exit grace raised from 60s to 300s to match typical restart drain budget.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. 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. merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. and removed proof: sufficient ClawSweeper judged the real behavior proof convincing. 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 Jul 3, 2026
@ZOOWH

ZOOWH commented Jul 4, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. and removed proof: sufficient ClawSweeper judged the real behavior proof convincing. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jul 4, 2026
@ZOOWH

ZOOWH commented Jul 4, 2026

Copy link
Copy Markdown
Contributor Author

Proof refreshed for the current head after the shutdown-reserve fix.

Changes since last review:

  • Added SHUTDOWN_RESERVE_MS = 30_000 constant to cover the Gateway shutdown window (the Gateway force-exit timer uses restartDrainTimeoutMs + SHUTDOWN_TIMEOUT_MS = 300s + 25s)
  • parentExitTimeoutMs calculation now includes the reserve: restartDelayMs + max(300s, timeoutMs) + 30s
  • Updated both tests to expect 330_000 instead of 300_000
  • Fresh proof output shows parentExitTimeoutMs = 330000 for the default/300s case (was 300000 in the stale proof)

@clawsweeper re-review

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. 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 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. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. labels Jul 4, 2026
@github-actions github-actions Bot added the dependencies-changed PR changes dependency-related files label Jul 6, 2026
@github-actions

github-actions Bot commented Jul 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Dependency graph guard cleared

This PR no longer has blocked dependency graph changes. A future dependency graph change requires a fresh /allow-dependencies-change comment after the guard blocks that new head SHA.

  • Current SHA: 0824d169998553657bf5b0b31b7c50c7adb01bc9

@steipete
steipete force-pushed the fix/99666-handoff-parent-timeout branch from 51dd58e to df47c3a Compare July 6, 2026 06:53
@openclaw-barnacle openclaw-barnacle Bot removed docs Improvements or additions to documentation app: macos App: macos app: web-ui App: web-ui scripts Repository scripts labels Jul 6, 2026
@github-actions github-actions Bot removed the dependencies-changed PR changes dependency-related files label Jul 6, 2026
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jul 6, 2026
@steipete
steipete force-pushed the fix/99666-handoff-parent-timeout branch from df47c3a to 5640a59 Compare July 6, 2026 07:20
ZOOWH and others added 11 commits July 6, 2026 08:39
@ZOOWH @steipete
fix(infra): include update timeout in managed-service handoff parent …
46493bc 
…exit wait

The handoff parent exit grace was hard-coded to 60s regardless of
the overall update timeout. When the gateway is draining active
tasks, 60s is often not enough, causing
managed-service-handoff-parent-timeout.

Use the existing timeoutMs parameter as a floor for the parent
exit wait so callers can extend it for long-running drains.

Refs openclaw#99666
@ZOOWH @steipete
fix(infra): add shutdown reserve to managed-service handoff parent ex…
2530a3b 
…it wait
@steipete @ZOOWH
fix(infra): align update handoff with restart drain
4487a91 
Co-authored-by: 徐闻涵0668001344 <xu.wenhan1@xydigit.com>
@steipete
fix(infra): arm restart with managed handoff
2dda934
@steipete
fix(infra): use live restart drain for auto-update
0508529
@steipete
fix(infra): force process exit for managed updates
31bede3
@steipete
fix(infra): normalize managed handoff restart delay
39a0a94
@steipete
test(infra): cover handoff drain argument
e9bf86d
@steipete
fix(gateway): upgrade accepted restarts for managed updates
71cf3f0
@steipete
fix(gateway): bound managed update restart upgrades
c5fc690
@steipete
test(gateway): stabilize managed update handoff proof
0824d16
@steipete

steipete commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Merged via squash.

@github-actions github-actions Bot mentioned this pull request Jul 6, 2026
Open
This was referenced Jul 7, 2026
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@steipete steipete

Labels

cli CLI command changes gateway Gateway runtime merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: L status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: Gateway update fails due to managed-service handoff timeout during self-update

2 participants

@ZOOWH @steipete