Skip to content

Comments

gateway: hot-reload heartbeat when agents.list changes#4108

Open
jifanchn wants to merge 3 commits intoopenclaw:mainfrom
jifanchn:feat/agents-list-heartbeat-reload
Open

gateway: hot-reload heartbeat when agents.list changes#4108
jifanchn wants to merge 3 commits intoopenclaw:mainfrom
jifanchn:feat/agents-list-heartbeat-reload

Conversation

@jifanchn
Copy link

@jifanchn jifanchn commented Jan 29, 2026

Summary

When agents are added or removed via config changes, the heartbeat scheduler now restarts automatically. This ensures newly added agents get their heartbeat tasks scheduled without requiring a full gateway restart.

Use case: In multi-tenant deployments where agents are dynamically registered (e.g., via extension plugins), new agents need their heartbeat tasks to start without restarting the entire gateway.

Changes

  • Added agents.list to BASE_RELOAD_RULES with restart-heartbeat action
  • Added test case to verify the behavior

Test plan

  • pnpm exec vitest run src/gateway/config-reload.test.ts passes (9 tests)
  • CI passes

Greptile Overview

Greptile Summary

This PR extends the gateway config hot-reload rules so that changes under agents.list trigger a heartbeat scheduler restart, ensuring newly added/removed agents get their heartbeat tasks (re)scheduled without a full gateway restart. It also adds a unit test asserting that a change to agents.list sets restartHeartbeat in the reload plan.

The change fits into the existing buildGatewayReloadPlan mechanism in src/gateway/config-reload.ts, where config path prefixes map to either no-op, hot actions (like restarting cron/heartbeat), or full gateway restart.

Confidence Score: 4/5

  • This PR is low-risk and likely safe to merge; the behavior change is localized to config reload planning.
  • Changes are limited to adding a new reload prefix and a simple unit test; no runtime control flow changes outside the existing reload-plan mapping. Main risk is the test not fully reflecting real-world diffConfigPaths output for agents.list shape changes.
  • src/gateway/config-reload.ts, src/gateway/config-reload.test.ts

(2/5) Greptile learns from your feedback when you react with thumbs up/down!

@openclaw-barnacle openclaw-barnacle bot added the gateway Gateway runtime label Jan 29, 2026
@jifanchn jifanchn force-pushed the feat/agents-list-heartbeat-reload branch from dadf336 to e7f4c69 Compare January 31, 2026 16:09
@jifanchn
Copy link
Author

The checks (node, format, pnpm format) failure is a pre-existing issue on main branch (commit 9c29853). The files modified in this PR (src/gateway/config-reload.ts and src/gateway/config-reload.test.ts) pass format checks locally:

$ pnpm exec oxfmt --check src/gateway/config-reload.ts src/gateway/config-reload.test.ts
All matched files use the correct format.

The format issues are in docs/automation/cron-jobs.md and package.json, which are not touched by this PR.

When agents.list configuration changes, the heartbeat scheduler should
restart to pick up new agent configurations. This enables dynamic agent
registration workflows where new agents are added to the config file
and their heartbeat tasks are automatically scheduled without requiring
a full gateway restart.
@jifanchn jifanchn force-pushed the feat/agents-list-heartbeat-reload branch from e7f4c69 to fe311a4 Compare January 31, 2026 16:24
@openclaw-barnacle openclaw-barnacle bot added the docs Improvements or additions to documentation label Jan 31, 2026
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 3, 2026

Additional Comments (2)

src/gateway/config-reload.ts
[P1] agents.list reload prefix likely won’t match actual diff path (agents.list.<id>)

diffConfigPaths will produce a path like agents.list.<agentId> when an element is added/removed (because it treats objects recursively), not agents.list. Your new rule { prefix: "agents.list", ... } will match agents.list.<id> (good), but only if the config structure is an object keyed by agentId. If agents.list is actually an array (as the name suggests), diffConfigPaths will emit just agents.list on any change (array diff returns [prefix]), which is also covered. The ambiguous naming makes this easy to mis-assume; a targeted test for add/remove that uses the real shape of agents.list would prevent regressions.

Also appears in tests: src/gateway/config-reload.test.ts:88-92 (test only asserts the literal path, not a realistic diff).

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/gateway/config-reload.ts
Line: 56:58

Comment:
[P1] `agents.list` reload prefix likely won’t match actual diff path (`agents.list.<id>`)

`diffConfigPaths` will produce a path like `agents.list.<agentId>` when an element is added/removed (because it treats objects recursively), not `agents.list`. Your new rule `{ prefix: "agents.list", ... }` will match `agents.list.<id>` (good), but only if the config structure is an object keyed by agentId. If `agents.list` is actually an *array* (as the name suggests), `diffConfigPaths` will emit just `agents.list` on any change (array diff returns `[prefix]`), which is also covered. The ambiguous naming makes this easy to mis-assume; a targeted test for add/remove that uses the real shape of `agents.list` would prevent regressions.

Also appears in tests: `src/gateway/config-reload.test.ts:88-92` (test only asserts the literal path, not a realistic diff).

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

src/gateway/config-reload.test.ts
[P2] Test doesn’t validate actual config diff behavior for agent add/remove

This test calls buildGatewayReloadPlan(["agents.list"]) directly, but the real trigger is diffConfigPaths(prev, next). Depending on whether agents.list is an array or an object, the diff path might be agents.list or agents.list.<id> (or deeper). Consider constructing prev/next configs and asserting diffConfigPaths output drives the restart heartbeat behavior, so the test covers the end-to-end contract rather than a hard-coded string.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/gateway/config-reload.test.ts
Line: 88:92

Comment:
[P2] Test doesn’t validate actual config diff behavior for agent add/remove

This test calls `buildGatewayReloadPlan(["agents.list"])` directly, but the real trigger is `diffConfigPaths(prev, next)`. Depending on whether `agents.list` is an array or an object, the diff path might be `agents.list` or `agents.list.<id>` (or deeper). Consider constructing `prev/next` configs and asserting `diffConfigPaths` output drives the restart heartbeat behavior, so the test covers the end-to-end contract rather than a hard-coded string.

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

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

Address code review feedback from greptile-apps:
- Add test that verifies diffConfigPaths produces 'agents.list' for array changes
- Add test for agent removal scenario
- Tests now cover the end-to-end contract from config diff to reload plan
@jifanchn
Copy link
Author

jifanchn commented Feb 3, 2026

Thanks for the review feedback! I've addressed both concerns:

P1 - agents.list prefix matching:

  • Verified that agents.list is indeed an array (not an object keyed by agentId)
  • diffConfigPaths returns "agents.list" for any array change (add/remove/modify)
  • The prefix rule { prefix: "agents.list", ... } correctly matches this path

P2 - End-to-end test coverage:

  • Added two new tests that use diffConfigPaths(prev, next) with real config shapes
  • restarts heartbeat when agent is added to agents.list (end-to-end) - verifies adding an agent
  • restarts heartbeat when agent is removed from agents.list - verifies removing an agent
  • Both tests confirm that diffConfigPaths produces "agents.list" and the reload plan sets restartHeartbeat: true

The key insight is that diffConfigPaths treats arrays atomically - any change to the array returns just [prefix] (line 160-165 in config-reload.ts), so the "agents.list" prefix rule works correctly for all array mutations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Improvements or additions to documentation gateway Gateway runtime

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant