fix(gateway): re-read config on in-process restart instead of replaying stale snapshot#80041
Conversation
|
Thanks for the context here. I swept through the related work, and this is now duplicate or superseded. This PR is a valid focused bug fix, but it is now superseded because #80161 merged the same one-shot startup snapshot behavior, regression coverage, and changelog entry into current main. So I’m closing this here and keeping the remaining discussion on the canonical linked item. Review detailsBest possible solution: Keep the merged #80161 implementation as the canonical fix and close this duplicate branch. Do we have a high-confidence way to reproduce the issue? Yes. The original failure has a high-confidence source path and the PR body supplies live Docker before/after logs; current main no longer fails because the canonical fix has merged. Is this the best way to solve the issue? No as a merge path. The code shape is sound, but the same fix is already on main through #80161, so merging this branch would only duplicate that work. Security review: Security review cleared: Cleared: the PR only changes gateway startup option plumbing, a focused unit test, and changelog text, with no new dependencies, permissions, or code execution surface. What I checked:
Likely related people:
Codex review notes: model gpt-5.5, reasoning high; reviewed against 9e77a41fcb99; fix evidence: release v2026.5.10-beta.3, commit e5fe9bdef085. |
…ng stale snapshot
The gateway's CLI runner captured the pre-read startup config snapshot once
before entering the restart loop and reused it on every iteration. After a
SIGUSR1 in-process restart, downstream startup writes (auth bootstrap, plugin
auto-enable persistence, control-ui seed) projected the stale snapshot back
to disk, silently clobbering any user config written between gateway boot
and the restart.
Consume the pre-read snapshot exactly once. Subsequent in-process restart
iterations now re-read openclaw.json from disk so user writes between
restarts survive.
Code review
-----------
Approach. The fix introduces a closure-scoped `pendingStartupSnapshot`
variable that holds the pre-read snapshot, then clears it on first read
inside the start callback. This matches the proposed fix in the issue
("only use startupConfigSnapshotRead on the first start call") while
keeping the cold-start optimization intact for the first iteration. An
alternative `isFirstStart` boolean would have been equivalent; the
consume-once shape was chosen because it makes the lifecycle of the
snapshot explicit and avoids a separate flag that has to stay in sync
with the snapshot variable.
Concurrency. `runGatewayLoop`'s `for(;;)` loop
(src/cli/gateway-cli/run-loop.ts:537) calls `params.start` sequentially,
awaiting each call before the next iteration. There is no concurrency on
the closure variable, so no synchronization is needed.
Lock-recovery interaction. `runGatewayLoopWithSupervisedLockRecovery`
(src/cli/gateway-cli/run.ts:382) may re-invoke `startLoop` on a
`GatewayAlreadyRunningLockError`. With this fix, the second invocation has
no snapshot and falls through to a fresh disk read inside the new
`runGatewayLoop` invocation. That is the correct behavior on a retry of a
failed first start; the original buggy code would have replayed the stale
snapshot on the retry too, so this path is also incidentally improved.
Downstream contract. `loadGatewayStartupConfigSnapshot`
(src/gateway/server-startup-config.ts:67) honors `initialSnapshotRead`
when supplied and falls through to `readConfigFileSnapshotWithPluginMetadata`
when absent. The second iteration now exercises the disk-read path that
the existing recovery test
(src/gateway/server-startup-config.recovery.test.ts:244) already covers
in isolation.
Test coverage. New regression in
src/cli/gateway-cli/run.option-collisions.test.ts uses
`runGatewayLoop.mockImplementationOnce` to invoke the start callback twice
and asserts `startupConfigSnapshotRead` is present on the first
`startGatewayServer` call and absent on the second. Composition of this
test with the existing recovery test exercises the full fixed path.
Live behavior proof. Reproduced and verified on `node:24-alpine` with all
`SUPERVISOR_HINT_ENV_VARS` unset to force the in-process restart fallback.
With the buggy closure, an `auth.profiles` entry written to
`openclaw.json` between gateway boot and SIGUSR1 was clobbered by the
gateway's own writeback during restart startup; sha256 and size of the
file reverted to the pre-write state and the gateway logged
"Config overwrite ... -> [stale snapshot]". With the fix applied to the
same `dist/run-D9To-U2i.js`, the user write survived byte-for-byte across
restart, and the gateway surfaced a config validation error on the
user-written profile, independently confirming that the disk re-read
happened. Full evidence (sha256s, sizes, log lines) is in the PR body.
Notes for reviewers. The TypeScript annotation
`typeof startupConfigSnapshotRead | undefined` is technically redundant
because the source type is already nullable, but it reads cleaner at the
declaration site than relying on inference. Happy to drop the annotation
if maintainers prefer. No public seam, gateway protocol surface, or
plugin SDK contract is touched. Codex CLI was not installed on the
contributor's machine, so the recommended `codex review --base origin/main`
pass was skipped; GitHub Codex review will run on the PR.
Fixes openclaw#79947.
80c7863 to
e11f8af
Compare
Summary
Fixes #79947.
The gateway's CLI runner (
src/cli/gateway-cli/run.ts) captured the pre-read startup config snapshot once before entering the restart loop and then reused that same snapshot for every iteration. After a SIGUSR1 in-process restart, downstream startup writes (auth bootstrap, plugin auto-enable persistence, control-ui seed) projected the stale snapshot back to disk and silently clobbered any user config written between boot and the restart.The fix consumes the pre-read snapshot exactly once. Subsequent in-process restart iterations re-read
openclaw.jsonfrom disk so user writes between restarts survive.This matches the proposed fix in the issue body — preserve the cold-start optimization on the first iteration, force disk re-read on every iteration after.
Real behavior proof
Behavior or issue addressed: When the OpenClaw gateway runs in an environment without a supervisor marker (
OPENCLAW_SYSTEMD_UNIT,INVOCATION_ID, etc. unset — the default in container deployments persrc/infra/process-respawn.ts), a SIGUSR1 in-process restart silently overwritesopenclaw.jsonwith a snapshot captured at the original boot, dropping any user config written between boot and the restart. Visible to users as "the bot disappeared after I configured it" — bot tokens revert to empty, auth profiles vanish, channels become disabled. See #79947 for the original report including pre/post sha256s and the exactConfig overwritelog line.Real environment tested: Live Docker reproduction on a contributor host (Linux 7.0.3-zen1-2-zen, Arch Linux), Node 24, Docker image
node:24-alpine. OpenClaw built locally from this branch (package version2026.5.8). AllSUPERVISOR_HINT_ENV_VARS(OPENCLAW_SYSTEMD_UNIT,INVOCATION_ID,SYSTEMD_EXEC_PID,JOURNAL_STREAM, the launchd hints,OPENCLAW_SERVICE_MARKER,OPENCLAW_SERVICE_KIND) explicitly unset inside the container sodetectRespawnSupervisorreturnsnulland the gateway falls through to the in-process restart path documented atsrc/infra/process-respawn.ts:70("container: use in-process restart to keep PID 1 alive"). Gateway invoked withOPENCLAW_ALLOW_ROOT=1since the container runs as root. Two variants of the samedist/run-D9To-U2i.jswere compared: the pre-fix closure shape (BUGGY) and this PR's shape (FIXED). Patch verified to match the source pattern exactly once before swapping.Exact steps or command run after this patch: Inside
node:24-alpine, for each variant ofdist/run-D9To-U2i.js: (1) write a minimalopenclaw.jsonwithgateway.mode=local,gateway.port=18790,gateway.auth.mode=none. (2) Start the gateway in the background:node dist/index.js gateway run --port 18790 --bind loopback --allow-unconfiguredand wait for the[gateway] readylog line (about 6 seconds on the test host). (3)sha256sum /root/.openclaw/openclaw.jsonto record the baseline. (4) Usenode -eto write a restart-required key directly to disk —auth.profiles.repro_79947 = {id, marker: "USER_WROTE_THIS"}andchannels.telegram = {enabled: true, botToken: "USER_WROTE_TOKEN"}. (5)sha256sumagain to confirm the user write landed. (6)kill -USR1 $GW_PIDto trigger the in-process SIGUSR1 restart. (7)sleep 8for the restart cycle. (8)sha256sumandgrep -q USER_WROTE_THIS/grep -q USER_WROTE_TOKENon/root/.openclaw/openclaw.jsonto see whether the user write survived. The full driver shell script bind-mounted the sourcedist/into/openclawread-only and copied to a writable/opt/openclawinside the container.Evidence after fix: The contributor pasted the full terminal output of the live
docker runinvocation. Verbatim console output captured from the host (real, non-mock):BUGGY variant — sha256 + size of
openclaw.jsonfromsha256sumandstat -c%s:Smoking-gun runtime log line printed by the openclaw gateway during the restart iteration (real output captured live, redacted log path is the in-container
/root/.openclaw):Result line emitted by the test driver:
FIXED variant on the same dist tree, only the closure shape in
runGatewayCommanddiffers:Independent confirmation that the FIXED gateway re-read disk on the restart iteration: the same gateway logged a config validation error against the new
auth.profiles.repro_79947payload that we wrote between boot and SIGUSR1. The buggy gateway never saw that payload because it overwrote the file with the stale snapshot before validation ran. Stack trace excerpt printed live by the gateway runtime:Observed result after fix: Post-restart
sha256and byte size ofopenclaw.jsonmatch the post-write state exactly (409d59518ef13671 / 515 bytes), versus the BUGGY run where the post-restart sha (a40253354fc96b38) and size (274bytes) regressed all the way back to the pre-write baseline. The fixed gateway readsopenclaw.jsonfrom disk on the restart iteration — confirmed both by the surviving content and by the fact that the runtime now surfaces a config validation stack trace for the user-written profile rather than silently overwriting it. Same hardware, same image, same dist tree — only the closure shape inrunGatewayCommanddiffers between the two runs.What was not tested: End-to-end macOS launchd reproduction. The bug is platform-agnostic — the in-process restart fallback runs the same code on every platform when no supervisor marker is set, per
src/infra/process-respawn.ts— but I did not run a live launchd reproduction. I also did not exercise the realopenclaw config setCLI flow; the reproduction triggers SIGUSR1 withkill -USR1and writes the user config directly. The reporter's path usedopenclaw config setandpairing approveto trigger the same code path; I verified the bug at the SIGUSR1 boundary which is what the fix targets. Three-or-more consecutive in-process restarts in a single gateway run were not exercised live, but the fix is shaped to handle N restarts (snapshot consumed on first, fresh disk read on every iteration after) and the unit regression covers two iterations.Test plan
pnpm test src/cli/gateway-cli/run.option-collisions.test.ts— 19/19 pass, including the new regression that verifiesstartupConfigSnapshotReadis passed only on the firststartcall within a singlerunGatewayLooppnpm test src/cli/gateway-cli/run-loop.test.ts src/gateway/server-startup-config.recovery.test.ts src/cli/gateway-cli/run.supervised-lock.test.ts— 36/36 passpnpm check:changed— exit 0 (lint, import-cycle, sidecar-loader, webhook/pairing/account guards)pnpm exec oxfmt --check --threads=1on touched files — cleanAI-assisted disclosure
codex review --base origin/mainnot run — Codex CLI was not installed on the contributor's machine when this PR was opened. Happy to run it if a maintainer wants that pass before review.