fix(gateway): skip config.patch restart for hot-reloadable changes by LarytheLord · Pull Request #46397 · openclaw/openclaw

LarytheLord · 2026-03-14T16:55:39Z

Summary

Problem: config.patch always queued scheduleGatewaySigusr1Restart() after writing the config, even when the changed paths were already classified as hot-reloadable or no-op by the gateway reload planner.
Why it matters: control-plane and agent-driven config edits unnecessarily bounced the gateway for changes that the file watcher can already apply in-process.
What changed: config.patch now reuses the existing reload planner plus the patched gateway.reload.mode to decide whether a restart is actually needed; when it is not, the handler skips both restart-sentinel creation and SIGUSR1 scheduling.
What did NOT change (scope boundary): config.apply is unchanged, and explicit config.patch calls still restart for restart-required paths (and when reload mode is off or restart).

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

User-visible / Behavior Changes

config.patch no longer forces a gateway restart for hot-reloadable / no-op config changes when the patched reload mode allows in-process reloads.
In that no-restart path, the response now returns restart: null and sentinel: null.
Gateway tool/system prompt wording now reflects that config.patch hot-reloads when possible.

Security Impact (required)

New permissions/capabilities? (Yes/No) No
Secrets/tokens handling changed? (Yes/No) No
New/changed network calls? (Yes/No) No
Command/tool execution surface changed? (Yes/No) No
Data access scope changed? (Yes/No) No
If any Yes, explain risk + mitigation:

Repro + Verification

Environment

OS: macOS (local dev machine)
Runtime/container: Node v25.6.1, pnpm 10.23.0
Model/provider: n/a
Integration/channel (if any): gateway control-plane config RPC
Relevant config (redacted): base config with gateway.reload.mode="restart", patched to "hybrid" for the hot-reload regression case

Steps

Start from a config where gateway.reload.mode is restart.
Call config.patch with a browser-profile change plus gateway.reload.mode: "hybrid".
Observe whether the handler still writes a restart sentinel / queues SIGUSR1.

Expected

Hot-reloadable config.patch writes should persist and return without scheduling a restart.

Actual

Before this change, config.patch always queued restart+sentry work even for hot-reloadable changes.
After this change, the hot-reload case skips both, while restart-required paths still schedule them.

Evidence

Added regression coverage in src/gateway/config-reload.test.ts and src/gateway/server-methods/config.restart.test.ts.
Verified with:
- pnpm exec vitest run src/gateway/config-reload.test.ts src/gateway/server-methods/config.restart.test.ts src/agents/system-prompt.test.ts src/agents/openclaw-gateway-tool.test.ts
- pnpm exec oxfmt --check src/agents/system-prompt.ts src/agents/tools/gateway-tool.ts src/gateway/config-reload.test.ts src/gateway/config-reload.ts src/gateway/server-methods/config.ts src/gateway/server-methods/config.restart.test.ts
- pnpm tsgo
- pnpm build

Human Verification (required)

What you personally verified (not just CI), and how:

Verified scenarios:
- hot-reloadable browser patch skips restart/sentinel
- restart-required gateway patch still schedules restart/sentinel
- patched gateway.reload.mode is used for the restart decision
Edge cases checked:
- no-op path set returns no restart from the planner helper
- off / restart modes still keep explicit config.patch restart behavior
What you did not verify:
- I did not run a live LaunchAgent/systemd gateway and reproduce the end-to-end restart bounce outside the test harness.

Review Conversations

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

If a bot review conversation is addressed by this PR, resolve that conversation yourself. Do not leave bot review conversation cleanup for maintainers.

Compatibility / Migration

Backward compatible? (Yes/No) Yes
Config/env changes? (Yes/No) No
Migration needed? (Yes/No) No
If yes, exact upgrade steps:

Failure Recovery (if this breaks)

How to disable/revert this change quickly:
- Revert this PR; the previous behavior was unconditional restart on every config.patch write.
Files/config to restore:
- src/gateway/config-reload.ts
- src/gateway/server-methods/config.ts
Known bad symptoms reviewers should watch for:
- hot-reloadable config.patch changes unexpectedly returning restart again
- restart-required patches skipping restart/sentinel when they should not

Risks and Mitigations

Risk: the restart/no-restart boundary could drift from the file watcher’s reload classification in the future.
- Mitigation: the handler now reuses the existing reload planner instead of open-coding its own path rules, and the new tests cover both hot/no-op and restart-required cases.

greptile-apps · 2026-03-14T16:59:34Z

Greptile Summary

This PR fixes a genuine bug where config.patch unconditionally scheduled a SIGUSR1 restart after every config write, even for changes the gateway's own file-watcher could already apply in-process. The fix introduces shouldConfigPatchTriggerGatewayRestart in config-reload.ts that reuses the existing buildGatewayReloadPlan planner together with the patched config's gateway.reload.mode to decide whether a restart is actually needed — keeping the restart path for off/restart modes and for restart-required paths in hot/hybrid modes, while skipping both the sentinel write and SIGUSR1 scheduling for hot-reloadable and no-op changes.

Key observations:

Logic is correct and well-scoped: the planner re-use prevents the restart/hot-reload classification from drifting between the file-watcher and config.patch paths in the future.
Using the patched config's mode is intentional and tested: the regression test in config.restart.test.ts directly verifies that patching gateway.reload.mode from "restart" → "hybrid" in the same call is evaluated using the new mode.
hot mode asymmetry is by design: shouldConfigPatchTriggerGatewayRestart returns true for restart-required paths in hot mode (the explicit API call forces the restart), whereas the file-watcher's applySnapshot silently ignores them in hot mode — this is the stated PR intent.
off mode behaviour preserved: in off mode all non-empty patches (including hot-reloadable and kind: "none" no-op paths) still restart via config.patch. This is backward-compatible and ensures changes take effect since the file-watcher does nothing in off mode, but it means a user who sets mode = "off" to suppress automatic restarts will still see a restart on any explicit config.patch call.
Response shape change: restart and sentinel are now nullable in the config.patch response. Consumers that assume these fields are always non-null will need to null-check them.
Test coverage is thorough across all four reload modes and both the hot-reloadable and restart-required path classifications.

Confidence Score: 4/5

Safe to merge; the implementation is correct and the regression test suite covers all key branches.
The core logic is sound, tests are comprehensive, and the change is backward-compatible. The one point held back: the off mode always restarts for any non-empty changedPaths (including kind: "none" no-op paths), which is preserved backward-compatible behavior but could be surprising for users who intend off mode to suppress all automatic restarts. Additionally, restart/sentinel in the config.patch response are now nullable, which may affect downstream consumers that don't null-check these fields.
Downstream callers that consume the config.patch response and access response.restart or response.sentinel without null-checking should be audited — these fields are now null on the hot-reload path.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/gateway/config-reload.ts
Line: 81-83

Comment:
**`off` mode restarts all non-empty patches, including no-op paths**

When `mode === "off"` the function returns `true` for *any* non-empty `changedPaths` — including paths classified as `kind: "none"` (e.g. `meta.version`, `identity`, `logging`). The file-watcher path in `applySnapshot` handles `off` mode by doing nothing at all, so this "always restart in `off` mode" behaviour is the only way to guarantee changes written by `config.patch` take effect in-process. That reasoning is sound, but it means a user who sets `mode = "off"` to suppress auto-restarts will still see a restart on *every* explicit `config.patch` call, even for purely cosmetic/no-op fields.

If that edge case matters, you could mirror the `hot`/`hybrid` path and short-circuit when `buildGatewayReloadPlan(changedPaths).restartGateway === false` before the mode guard. For now the backward-compatible behaviour is preserved, so this is just something to be aware of.

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: f5282c8}

greptile-apps · 2026-03-14T16:59:39Z

+  if (settings.mode === "off" || settings.mode === "restart") {
+    return true;
+  }


off mode restarts all non-empty patches, including no-op paths

When mode === "off" the function returns true for any non-empty changedPaths — including paths classified as kind: "none" (e.g. meta.version, identity, logging). The file-watcher path in applySnapshot handles off mode by doing nothing at all, so this "always restart in off mode" behaviour is the only way to guarantee changes written by config.patch take effect in-process. That reasoning is sound, but it means a user who sets mode = "off" to suppress auto-restarts will still see a restart on every explicit config.patch call, even for purely cosmetic/no-op fields.

If that edge case matters, you could mirror the hot/hybrid path and short-circuit when buildGatewayReloadPlan(changedPaths).restartGateway === false before the mode guard. For now the backward-compatible behaviour is preserved, so this is just something to be aware of.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/gateway/config-reload.ts Line: 81-83 Comment: **`off` mode restarts all non-empty patches, including no-op paths** When `mode === "off"` the function returns `true` for *any* non-empty `changedPaths` — including paths classified as `kind: "none"` (e.g. `meta.version`, `identity`, `logging`). The file-watcher path in `applySnapshot` handles `off` mode by doing nothing at all, so this "always restart in `off` mode" behaviour is the only way to guarantee changes written by `config.patch` take effect in-process. That reasoning is sound, but it means a user who sets `mode = "off"` to suppress auto-restarts will still see a restart on *every* explicit `config.patch` call, even for purely cosmetic/no-op fields. If that edge case matters, you could mirror the `hot`/`hybrid` path and short-circuit when `buildGatewayReloadPlan(changedPaths).restartGateway === false` before the mode guard. For now the backward-compatible behaviour is preserved, so this is just something to be aware of. How can I resolve this? If you propose a fix, please make it concise.

… hot-reload (openclaw#46401, openclaw#46397)

steipete · 2026-04-25T08:02:31Z

Closing this as implemented after Codex review.

Current main already fixes the reported restart behavior for config.patch: direct SIGUSR1 restarts are no longer unconditional, no-op patches short-circuit entirely, and the gateway reloader owns restart-vs-hot-reload decisions from the reload plan.

What I checked:

Direct restart is now gated by reload mode and reload plan: config.patch calls shouldScheduleDirectConfigRestart, which resolves reload mode from the patched config and only forces a direct restart for gateway.reload.mode === "off" or for hot mode when the reload plan says a restart is required. (src/gateway/server-methods/config.ts:285, 56fe2aab9c87)
No-op config.patch writes already skip restart entirely: The handler now returns early for changedPaths.length === 0, skipping both the config write and SIGUSR1 scheduling for no-op patches. (src/gateway/server-methods/config.ts:549, 56fe2aab9c87)
Gateway reloader owns restart vs hot-reload on in-process writes: The config reloader subscribes to config writes, computes buildGatewayReloadPlan(...), hot-reloads when possible, restarts in restart mode, and queues restart only for restart-required paths in hybrid mode. (src/gateway/config-reload.ts:271, 56fe2aab9c87)
Regression coverage exists on main: Tests assert that hybrid-mode config.patch auth changes do not call scheduleGatewaySigusr1Restart, while restart-required changes in hot mode still do. (src/gateway/server-methods/config.shared-auth.test.ts:110, 56fe2aab9c87)
Shipped release contains the behavior: The same config.patch restart gating and shared-auth tests are present in tag v2026.4.22, so this is already shipped, not just present on current main. (src/gateway/server-methods/config.ts:285, 00bd2cf7a376)

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

Review notes: reviewed against 56fe2aab9c87; fix evidence: release v2026.4.22, commit 00bd2cf7a376.

openclaw-barnacle Bot added gateway Gateway runtime agents Agent runtime and tooling size: M labels Mar 14, 2026

greptile-apps Bot reviewed Mar 14, 2026

View reviewed changes

xuwei-xy pushed a commit to xuwei-xy/openclaw that referenced this pull request Mar 14, 2026

fix(agents): MiniMax M2.5 XML tool calls + fix(gateway): config.patch…

62ee56c

… hot-reload (openclaw#46401, openclaw#46397)

xuwei-xy mentioned this pull request Mar 14, 2026

fix(agents): MiniMax M2.5 XML tool calls + fix(gateway): config.patch hot-reload (#46401, #46397) #46405

Closed

fix(gateway): skip config.patch restart for hot reloads

cb00010

LarytheLord force-pushed the fix/config-patch-hot-reload-46310 branch from f5282c8 to cb00010 Compare March 15, 2026 05:13

steipete closed this Apr 25, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(gateway): skip config.patch restart for hot-reloadable changes#46397

fix(gateway): skip config.patch restart for hot-reloadable changes#46397
LarytheLord wants to merge 1 commit into
openclaw:mainfrom
LarytheLord:fix/config-patch-hot-reload-46310

LarytheLord commented Mar 14, 2026

Uh oh!

greptile-apps Bot commented Mar 14, 2026

Uh oh!

greptile-apps Bot Mar 14, 2026

Uh oh!

steipete commented Apr 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

LarytheLord commented Mar 14, 2026

Summary

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

User-visible / Behavior Changes

Security Impact (required)

Repro + Verification

Environment

Steps

Expected

Actual

Evidence

Human Verification (required)

Review Conversations

Compatibility / Migration

Failure Recovery (if this breaks)

Risks and Mitigations

Uh oh!

greptile-apps Bot commented Mar 14, 2026

Greptile Summary

Confidence Score: 4/5

Uh oh!

greptile-apps Bot Mar 14, 2026

Choose a reason for hiding this comment

Uh oh!

steipete commented Apr 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants