Skip to content

fix(gateway): mark SIGUSR1 token consumed on restartIntent path, reset stale tokens on in-process restart#84334

Closed
6a6f686e6e79 wants to merge 2 commits into
openclaw:mainfrom
6a6f686e6e79:fix/update-run-restart-coalesced
Closed

fix(gateway): mark SIGUSR1 token consumed on restartIntent path, reset stale tokens on in-process restart#84334
6a6f686e6e79 wants to merge 2 commits into
openclaw:mainfrom
6a6f686e6e79:fix/update-run-restart-coalesced

Conversation

@6a6f686e6e79

@6a6f686e6e79 6a6f686e6e79 commented May 19, 2026 •
edited
Loading

Copy link
Copy Markdown

Summary

update.run from the control UI downloads the new package but the gateway never restarts to pick it up. Every attempt logs restart coalesced (already in-flight) and is silently dropped for the lifetime of the process.

Root cause

Two bugs combine to make the state permanent:

Bug 1 — src/cli/gateway-cli/run-loop.ts (onSigusr1):
When a restartIntent file is present, the handler calls request() and returns without calling markGatewaySigusr1RestartHandled(). This leaves emittedRestartToken > consumedRestartToken. Every subsequent scheduleGatewaySigusr1Restart() call hits the hasUnconsumedRestartSignal() guard and is dropped.

Bug 2 — src/infra/restart.ts (resetGatewayRestartStateForInProcessRestart):
This function clears timers and deferral polls but never aligns the token counters. A stale unconsumed token from the previous lifecycle carries forward into the next server iteration after an in-process restart.

Fix

  1. Call markGatewaySigusr1RestartHandled() before the restartIntent early-return in onSigusr1 so the token is consumed regardless of which restart path is taken.
  2. Align consumedRestartToken = emittedRestartToken in resetGatewayRestartStateForInProcessRestart() as a safety net so a stale token cannot survive an in-process restart boundary.

Reproduction

  1. Run the gateway as a long-lived systemd user service
  2. Let a new version become available and trigger update.run (UI or CLI)
  3. Observe: npm package updates on disk but gateway PID never changes
  4. Every retry logs restart coalesced (already in-flight)

Workaround (until patched)

systemctl --user restart openclaw-gateway

Real behavior log

Platform: systemd user service (openclaw-gateway.service), Ubuntu 24.04 LTS / WSL2

Two update.run attempts 11 minutes apart. changedPaths=<n/a> on the second attempt confirms the npm package updated successfully on attempt 1; only the restart was broken. Gateway process <pid-a> never changed between attempts.

# Attempt 1 — 15:25:47
<hostname> node[<pid-a>]: [restart] request coalesced (already in-flight) reason=update.run actor=openclaw-control-ui device=<device-id> ip=unknown-ip
<hostname> node[<pid-a>]: [gateway] update.run completed actor=openclaw-control-ui device=<device-id> ip=unknown-ip conn=<conn-uuid> changedPaths=<n/a> restartReason=update.run status=ok
<hostname> node[<pid-a>]: [gateway] update.run restart coalesced actor=openclaw-control-ui device=<device-id> ip=unknown-ip conn=<conn-uuid> delayMs=0

# Attempt 2 — 11 minutes later, same stuck process
<hostname> node[<pid-a>]: [restart] request coalesced (already in-flight) reason=update.run actor=openclaw-control-ui device=<device-id> ip=unknown-ip
<hostname> node[<pid-a>]: [gateway] update.run completed actor=openclaw-control-ui device=<device-id> ip=unknown-ip conn=<conn-uuid> changedPaths=<n/a> restartReason=update.run status=ok
<hostname> node[<pid-a>]: [gateway] update.run restart coalesced actor=openclaw-control-ui device=<device-id> ip=unknown-ip conn=<conn-uuid> delayMs=0

# Workaround: systemctl --user restart openclaw-gateway
<hostname> node[<pid-a>]: [gateway] signal SIGTERM received
<hostname> node[<pid-a>]: [gateway] received SIGTERM; shutting down
<hostname> node[<pid-a>]: [gateway] shutdown error: Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/home/<user>/.npm-global/lib/node_modules/openclaw/dist/hook-runner-global-DIpzvALt.js' imported from /home/<user>/.npm-global/lib/node_modules/openclaw/dist/server.impl-D3CWr7f5.js
# hook-runner-global-DIpzvALt.js is a hashed filename from the new package version. The old process was holding stale module refs because npm swapped the package on disk but the process never restarted.

<hostname> systemd: Stopped openclaw-gateway.service
<hostname> systemd: Started openclaw-gateway.service
<hostname> node[<pid-b>]: [gateway] loading configuration...
<hostname> node[<pid-b>]: [gateway] resolving authentication...
<hostname> node[<pid-b>]: [gateway] starting HTTP server...
<hostname> node[<pid-b>]: [gateway] ready
# New PID, updated binary loaded successfully

Regression tests

Two focused tests cover the two fixes and are included in this PR:

  • src/cli/gateway-cli/run-loop.test.ts: "marks the in-flight token consumed before routing SIGUSR1 with a restart intent" asserts that markGatewaySigusr1RestartHandled is called before request() is routed in the restartIntent branch. Fails on current main, passes with the fix.
  • src/infra/infra-runtime.test.ts: "aligns stale emit/consume token counters on in-process restart boundary" asserts that resetGatewayRestartStateForInProcessRestart() clears a stale unconsumed token so subsequent restart requests are not coalesced. Fails on current main, passes with the fix.

Post-patch note

Post-patch behavior is validated by the regression tests added in this PR; both fail against current main and pass with the patch applied. End-to-end log proof of update.run succeeding post-patch is omitted because reproducing it requires cutting a new release for the patched gateway to pull from, which is outside contributor scope.

@6a6f686e6e79
fix(gateway): mark SIGUSR1 token consumed on restartIntent path, rese…
7601e69 
…t stale tokens on in-process restart

In onSigusr1, when a restartIntent file is present the handler calls
request() and returns without calling markGatewaySigusr1RestartHandled().
This leaves emittedRestartToken > consumedRestartToken for the lifetime of
the process. Every subsequent scheduleGatewaySigusr1Restart() call hits the
hasUnconsumedRestartSignal() guard and is silently coalesced, making the
update.run restart permanently stuck.

Fix 1 (root cause): call markGatewaySigusr1RestartHandled() before the
restartIntent early-return in onSigusr1 so the token is consumed regardless
of which restart path is taken.

Fix 2 (safety net): align the token counters in
resetGatewayRestartStateForInProcessRestart() so a stale unconsumed token
from a previous lifecycle cannot carry forward into the next server iteration
after an in-process restart.
@openclaw-barnacle openclaw-barnacle Bot added cli CLI command changes size: XS triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 19, 2026
@clawsweeper

clawsweeper Bot commented May 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Thanks for the context here. I did a careful shell check against current main, and this is already implemented.

Close as implemented: current main and shipped releases already solve the observable update.run stuck-restart/coalesced-token failure through the merged gateway lifecycle-runtime restart fix, while this stale branch is now conflicting and only retains optional reset-boundary hardening.

Root-cause cluster
Relationship: superseded
Canonical: #84890
Summary: The merged lifecycle-runtime restart PR is the canonical shipped fix for the same observable update.run stuck restart/coalesced-token failure; UI feedback work remains adjacent rather than a backend scheduler fix.

Members:

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

So I’m closing this as already implemented rather than keeping a duplicate issue open.

Review details

Best possible solution:

Keep the shipped lifecycle-runtime restart fix and close this conflicting branch; open a new narrow current-main PR only if maintainers want the extra reset-boundary hardening.

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

No current-main failing reproduction remains for the original update.run stuck-restart symptom: current source consumes/clears the token in the relevant SIGUSR1 paths and the merged lifecycle-runtime fix handles the package-swap import failure. The branch's reset-boundary hardening is a source-level leftover, not the reported user failure.

Is this the best way to solve the issue?

No, this branch is no longer the best way to solve the issue. The best path is the shipped current-main restart lifecycle fix, with any extra reset-boundary guard proposed separately against today's SIGUSR1 authorization flow.

Security review:

Security review cleared: The diff only touches gateway restart state and tests; no dependency, workflow, secret, package, or third-party code execution change was found.

AGENTS.md: found and applied where relevant.

What I checked:

  • Current source consumes authorized restart intents: Current main consumes SIGUSR1 restart authorization and calls markGatewaySigusr1RestartHandled before routing a restartIntent request, covering the token leak this branch originally patched in run-loop.ts. (src/cli/gateway-cli/run-loop.ts:734, 640258d7b31d)
  • Current source protects against SIGUSR1 handler import failure: Current main eagerly loads lifecycle.runtime before installing signal listeners and catches SIGUSR1 handler failures to mark the restart token handled, matching the shipped fix for the package-swap coalescing failure described in the PR logs. (src/cli/gateway-cli/run-loop.ts:111, 640258d7b31d)
  • Current coalescing contract inspected: scheduleGatewaySigusr1Restart still coalesces while an emitted token is unconsumed, so the merged token-handling fixes are directly tied to the reported permanent coalescing symptom. (src/infra/restart.ts:843, 640258d7b31d)
  • Fix provenance: Merge commit 4a91385 landed fix(gateway): eager-load lifecycle runtime to survive in-place upgrades #84890, which added the eager lifecycle load and SIGUSR1 catch path for the same stuck update.run restart class. (src/cli/gateway-cli/run-loop.ts:111, 4a9138556ea6)
  • Release provenance: GitHub compare reports v2026.5.22 is ahead of 4a91385 with behind_by 0 and merge_base equal to the fix commit, proving the fix shipped by v2026.5.22. (4a9138556ea6)
  • Live PR state: GitHub reports this PR as CONFLICTING/DIRTY against main, so it is not a clean landing path for any remaining hardening. (416ac3830e3a)

Likely related people:

  • myps6415: Authored the merged PR that eagerly loads the gateway lifecycle runtime and catches SIGUSR1 listener failures for the same stuck update/restart failure class. (role: recent related fix author; confidence: high; commits: 4a9138556ea6; files: src/cli/gateway-cli/run-loop.ts)
  • steipete: History shows repeated work on gateway config/update restart flow, SIGUSR1 orphan recovery, and restart deferral in the central files. (role: feature-history contributor; confidence: high; commits: 71c31266a1db, 680eff63fbf8, ad57e561c6a8; files: src/cli/gateway-cli/run-loop.ts, src/infra/restart.ts, src/gateway/server-methods/update.ts)
  • joeykrug: Authored the SIGUSR1 in-process restart state reset work that is adjacent to this PR's reset-boundary hardening. (role: adjacent restart lifecycle contributor; confidence: medium; commits: 4e9f933e88e4; files: src/cli/gateway-cli/run-loop.ts)
  • Agustin Rivera: Current blame for the inspected restart lines points to the latest main integration commit, so this person is useful for routing current-surface questions but not as original feature provenance. (role: recent current-main carrier; confidence: low; commits: 6ead09230284; files: src/cli/gateway-cli/run-loop.ts, src/infra/restart.ts)

Codex review notes: model internal, reasoning high; reviewed against 640258d7b31d; fix evidence: release v2026.5.22, commit 4a9138556ea6.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. labels May 19, 2026
@6a6f686e6e79 @claude
test(gateway): add regression tests for restart coalescing fix
416ac38 
- run-loop.test.ts: assert markGatewaySigusr1RestartHandled is called
  before request() in the restartIntent branch of onSigusr1; both call
  count and invocation order are verified. Uses waitForLoopCondition for
  reliable async completion rather than fixed setImmediate counts.
- infra-runtime.test.ts: assert resetGatewayRestartStateForInProcessRestart
  aligns stale emit/consume token counters so a post-reset restart request
  is not coalesced.

Both tests fail against current main and pass with the patch.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
@6a6f686e6e79

6a6f686e6e79 commented May 20, 2026

Copy link
Copy Markdown
Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented May 20, 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.

Re-review progress:

@clawsweeper

clawsweeper Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg

🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat.

Where did the egg go?
  • The egg game starts only after the PR passes the real-behavior proof check.
  • Before that, no creature, rarity, or ASCII portrait is rolled. The treat waits for real proof.
  • This is still just collectible flavor: proof affects review readiness, not creature quality.

@6a6f686e6e79

6a6f686e6e79 commented May 20, 2026

Copy link
Copy Markdown
Author

Note on the Real behavior proof workflow failure (run #26133827467):

Producing after-fix logs requires the patched gateway to be in a released version that an end user can update.run to. As an external contributor I can't cut a release, so end-to-end post-patch evidence isn't reproducible from my side.

What's in the PR instead:

  • Before-fix logs in the PR body document the exact symptom on a real systemd user service (two update.run attempts 11 minutes apart, same stuck PID, restart coalesced on both, plus an ERR_MODULE_NOT_FOUND on workaround shutdown showing the new package was already on disk while the old process held stale module refs).
  • Regression tests in commit 2 (src/cli/gateway-cli/run-loop.test.ts, src/infra/infra-runtime.test.ts) fail against current main and pass with this patch. They cover both the restartIntent token-consumption path and the reset-time token alignment.

The workflow output notes that a maintainer can apply a proof: override when appropriate; flagging here in case that's the right path. Happy to do anything else that's reproducible from a contributor setup.

@clawsweeper clawsweeper Bot mentioned this pull request May 21, 2026
Merged
This was referenced Jun 18, 2026
Closed
Merged
@clawsweeper clawsweeper Bot removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 30, 2026
@openclaw-barnacle openclaw-barnacle Bot added the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jun 30, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 30, 2026
@clawsweeper

clawsweeper Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper applied the proposed close for this PR.

@clawsweeper clawsweeper Bot closed this Jun 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

cli CLI command changes merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. P1 High-priority user-facing bug, regression, or broken workflow. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@6a6f686e6e79