PR #72137: fix(gateway): collapse phantom diff entries from empty-container normalization (#72061)

• Repository: openclaw/openclaw
• Author: jawwad-ali
• State: open | merged: False
• Link: https://github.com/openclaw/openclaw/pull/72137

Description (problem / solution / changelog)

Summary

• Problem: every skill toggle on 2026.4.24 forces a full gateway restart with plugins.entries.{browser,xai,openai,openrouter}.config (and other unedited paths) appearing in the reload diff. All webchat clients drop on code=1012 reason=service restart.
• Why it matters: the on-disk config blocks for those plugin entries contain only enabled: true. Runtime-side normalization injects an empty config: {} into the post-write source snapshot, so a no-op edit looks like a write to many sibling paths and the reload plan classifies them as restart-requiring (per buildGatewayReloadPlan's default-restart fallback).
• What changed: diffConfigPaths now treats undefined, {}, and [] as equivalent "no value" shapes via a new isEmptyConfigBranch helper. Skill toggles only report skills.entries.<id>.enabled and stay on the hot-reload path.
• What did NOT change (scope boundary): no edits to buildGatewayReloadPlan, the in-process write protocol, the source/runtime snapshot split, the skills-snapshot invalidation prefixes, or any caller of diffConfigPaths. requireApiKey, secrets/auth surfaces, and CODEOWNERS-restricted paths are untouched.

Change Type (select all)

• Bug fix
• Feature
• Refactor required for the fix
• Docs
• Security hardening
• Chore/infra

Scope (select all touched areas)

• Gateway / orchestration
• Skills / tool execution
• Auth / tokens
• Memory / storage
• Integrations
• API / contracts
• UI / DX
• CI/CD / infra

Linked Issue/PR

• Closes #72061
• This PR fixes a bug or regression

Root Cause (if applicable)

• Root cause: diffConfigPaths recurses into plain objects and falls through to return [prefix] when one side is undefined and the other is a non-empty (or empty) container. Equivalent "no value" shapes (missing field, {}, []) therefore produce phantom path entries. Write-time config normalization in the gateway can introduce empty config: {} entries for plugins.entries.<id> (and similar siblings) that the on-disk source omits, so every skill toggle write produces phantom diffs at those siblings. buildGatewayReloadPlan then treats plugins.entries.<id>.config as an unknown restart-requiring path, so even a no-op write triggers gateway restart, dropping all webchat sockets.
• Missing detection / guardrail: no test in src/gateway/config-reload.test.ts covered the missing-vs-empty-container case for diffConfigPaths, and the skill-toggle integration path never asserted that only skills.entries.<id>.enabled should appear in the diff when sibling plugin entries are normalized.
• Contributing context (if known): the existing array-equality short-circuit (isDeepStrictEqual for two arrays) covered the symmetrical-arrays case but not the asymmetric undefined ↔ {} ↔ [] cases that runtime-side normalization introduces.

Regression Test Plan (if applicable)

• Coverage level that should have caught this:
  • Unit test
  • Seam / integration test
  • End-to-end test
  • Existing coverage already sufficient
• Target test or file: src/gateway/config-reload.test.ts (describe("diffConfigPaths"))
• Scenario the test should lock in: four cases under the existing diffConfigPaths block —
  • undefined field vs empty plain object on the same path returns no paths,
  • undefined field vs empty array returns no paths,
  • empty plain object vs empty array returns no paths,
  • a missing-to-populated container still returns a change at the parent path,
  • the full #72061 scenario (skill toggle plus four sibling plugins.entries.<id>.config: {} injections plus an empty messages: {} branch) returns exactly ["skills.entries.coding-agent.enabled"].
• Why this is the smallest reliable guardrail: diffConfigPaths is the single source of truth for the changed-paths list that drives buildGatewayReloadPlan. Asserting equivalence at the comparator boundary covers every caller (in-process writes, watcher reads, and the recovery path) without needing integration plumbing.
• Existing test that already covers this (if any): N/A.
• If no new test is added, why not: 5 new test cases added (4 micro-cases + 1 full-issue reproduction).

User-visible / Behavior Changes

• Toggling a skill no longer triggers a full gateway restart and no longer drops active webchat clients with code=1012 reason=service restart. Hot-reload still fires for the skills snapshot. No config schema, contract, or default changes.

Diagram (if applicable)

Before:
[skill toggle] -> writeConfigFile -> notifyRuntimeConfigWriteListeners
  -> diffConfigPaths(prev_source, next_source)
       -> reports skills.entries.<id>.enabled
       -> phantom-reports plugins.entries.{browser,xai,openai,openrouter}.config
       -> phantom-reports messages, logging.redactSensitive, agents.defaults.*, ...
  -> buildGatewayReloadPlan -> restartGateway=true (unknown plugin paths)
  -> SIGUSR1 -> all webchat clients disconnect (code=1012)

After:
[skill toggle] -> writeConfigFile -> notifyRuntimeConfigWriteListeners
  -> diffConfigPaths(prev_source, next_source)
       -> isEmptyConfigBranch short-circuits undefined ≈ {} ≈ []
       -> reports only skills.entries.<id>.enabled
  -> buildGatewayReloadPlan -> restartGateway=false, restartHeartbeat=false
  -> hot reload only; webchat clients stay connected

Security Impact (required)

• New permissions/capabilities? No
• Secrets/tokens handling changed? No
• New/changed network calls? No
• Command/tool execution surface changed? No
• Data access scope changed? No

The fix is contained to the in-memory comparator that drives reload classification. All existing restart triggers (gateway.auth.token, gateway.auth.mode, gateway.port, plugin enabled/disabled, etc.) continue to fire, because those produce real (non-empty) value diffs and bypass the new short-circuit.

Repro + Verification

Environment

• OS: Linux 6.x (Debian/Ubuntu) inside Docker, also reproduced on Windows 11 dev host
• Runtime/container: Node 24.11.0, pnpm 10.20.0
• Model/provider: irrelevant — bug is in config-reload pre-model
• Integration/channel (if any): WebUI Skills page → toggle any skill
• Relevant config (redacted): plugins.entries.{browser,xai,openai,openrouter} containing only enabled: true (no nested config block)

Steps

1. pnpm install
2. Reproduce on main (pre-fix): add the new test cases and run pnpm test src/gateway/config-reload.test.ts → treats missing fields and empty plain objects as equivalent and the full #72061 scenario fail.
3. Apply this branch: pnpm test src/gateway/config-reload.test.ts → 68/68 pass.
4. Targeted gates:
   • pnpm tsgo:core → clean
   • pnpm tsgo:core:test → clean
   • pnpm lint → 0 warnings, 0 errors
   • pnpm format → no changes
   • pnpm check:changed → exit 0

Expected

• The skill-toggle reload diff lists skills.entries.<id>.enabled only.
• buildGatewayReloadPlan produces a hot-reload plan, not a restart plan.

Actual

• Matches expected on the new tests.

Evidence

• Failing test/log before + passing after
• Trace/log snippets
• Screenshot/recording
• Perf numbers (if relevant)

The new full-#72061 test reconstructs the issue's reload-evaluator log verbatim and asserts the changed-paths list collapses to ["skills.entries.coding-agent.enabled"]. Without the fix the test reports the four phantom plugins.entries.<id>.config paths and the unrelated logging/models/agents/messages paths, exactly matching the issue body.

Human Verification (required)

• Verified scenarios:
  • Pre-fix: reproduced phantom-diff classification by running the new test against unfixed source — fails with the four plugins.entries.<id>.config phantom paths.
  • Post-fix: full pnpm test src/gateway/config-reload.test.ts (68 tests) passes locally.
  • Lint/format clean across the full lint shards (pnpm lint).
  • Type-checks pass for both tsgo:core and tsgo:core:test.
• Edge cases checked:
  • Missing-to-populated container (undefined → { trustedHosts: ["..."] }) still reports the change at the parent path.
  • Existing duplicate-path emission for plugin install timestamps is unchanged.
  • Existing array-equality short-circuit and array-of-objects diff continue to behave identically.
  • The skills-snapshot invalidation prefixes still fire for skills.* (not affected).
• What I did not verify: live gateway restart behavior end-to-end (no Docker harness available locally). The behavior is asserted at the seam that drives the reload plan, which is the same boundary the issue's logs print from.

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.

Compatibility / Migration

• Backward compatible? Yes — all existing classification behavior is preserved; only equivalent "no value" shapes are collapsed.
• Config/env changes? No
• Migration needed? No

Risks and Mitigations

• Risk: a caller of diffConfigPaths somewhere in the codebase relies on the previous behavior of treating undefined ≠ {} for empty containers.
  • Mitigation: searched callers; the function is only used to drive the gateway reload plan and the skills-invalidation check. Both consume the result as "did anything meaningful change?", which is preserved. The added test asserts that a populated container is still reported when transitioning from missing.
• Risk: a future config field uses {} and undefined as semantically distinct values.
  • Mitigation: today no schema in src/config/types.openclaw.js distinguishes them, and the equivalence is conventional across the runtime-snapshot/source-config split. If that ever changes, the helper is the single place to refine.

🤖 AI-assisted (Claude Code). Test level: fully tested via pnpm test src/gateway/config-reload.test.ts. I understand the change.

Changed files

• src/gateway/config-reload.test.ts (modified, +72/-0)
• src/gateway/config-reload.ts (modified, +25/-0)