Heartbeat: spread interval runs across stable phases by odysseus0 · Pull Request #64560 · openclaw/openclaw

odysseus0 · 2026-04-11T00:51:35Z

Summary

Problem: interval heartbeats are scheduled relative to gateway startup, which clusters provider traffic around the same startup-relative slots instead of distributing it uniformly across the configured interval.
Why it matters: synchronized heartbeat turns can hammer model providers even when the average heartbeat rate is reasonable.
What changed: interval heartbeats now use a stable per-agent phase derived from gateway identity plus agent id, spreading runs uniformly across the configured interval.
What did NOT change (scope boundary): default cadence stays at 30m, and targeted/action-driven wakes keep the existing cooldown behavior.

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Closes #
Related #
This PR fixes a bug or regression

Root Cause (if applicable)

Root cause: the scheduler used now + interval for initial and follow-up interval heartbeats, so each gateway stayed pinned to startup-relative times instead of being distributed across the interval.
Missing detection / guardrail: there was no scheduler test asserting stable phase distribution independent of startup time.
Contributing context (if known): no central coordinator exists, so startup-relative scheduling becomes the de facto global distribution strategy.

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/infra/heartbeat-schedule.test.ts, src/infra/heartbeat-runner.scheduler.test.ts
Scenario the test should lock in: heartbeats for a given gateway/agent get a stable phase inside the configured interval, config reload preserves future due slots, and retry wakes do not push the schedule out indefinitely.
Why this is the smallest reliable guardrail: the bug lives in pure scheduling math plus the runner seam; end-to-end coverage would be slower without adding signal.
Existing test that already covers this (if any): src/infra/heartbeat-runner.scheduler.test.ts covered the old interval scheduler behavior.
If no new test is added, why not: N/A

User-visible / Behavior Changes

Heartbeat interval runs are spread across the configured interval instead of clustering around startup-relative times.
Default heartbeat cadence remains 30m.

Diagram (if applicable)

Before:
[heartbeat stream] -> [startup-relative timer] -> [traffic clusters at similar interval slots]

After:
[heartbeat stream] -> [stable hashed per-agent phase] -> [traffic distributed across the interval]

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)
If any Yes, explain risk + mitigation:

Repro + Verification

Environment

OS: macOS
Runtime/container: Node 24 / pnpm
Model/provider: N/A
Integration/channel (if any): N/A
Relevant config (redacted): agents.defaults.heartbeat.every="30m"

Steps

Start multiple gateways with the same heartbeat interval.
Compare when their next interval heartbeat fires.
Restart them and compare the next interval heartbeat again.

Expected

Each gateway/agent keeps its own stable slot within the interval.

Actual

Before this change, heartbeats stayed clustered around startup-relative times.

Evidence

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

Human Verification (required)

Verified scenarios: targeted scheduler tests pass for stable phase math and runner scheduling behavior.
Edge cases checked: config reload preserves future due slots; retry wakes do not permanently shift interval scheduling; targeted wakes still use cooldown-style now + interval behavior.
What you did not verify: full pnpm build currently fails on an unrelated baseline issue resolving acpx/runtime from extensions/acpx/src/runtime.ts.

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)
Config/env changes? (No)
Migration needed? (No)
If yes, exact upgrade steps:

Risks and Mitigations

Risk: stable phase scheduling changes when interval changes or when a gateway identity is regenerated.
- Mitigation: that is expected and bounded; with a stable identity and unchanged interval, phases remain deterministic across restarts.

aisle-research-bot · 2026-04-11T00:51:44Z

🔒 Aisle Security Analysis

We found 1 potential security issue(s) in this PR:

#	Severity	Title
1	🟡 Medium	Targeted heartbeat wakes can indefinitely postpone scheduled interval heartbeats (scheduler starvation)

1. 🟡 Targeted heartbeat wakes can indefinitely postpone scheduled interval heartbeats (scheduler starvation)

Property	Value
Severity	Medium
CWE	CWE-400
Location	`src/infra/heartbeat-runner.ts:1240-1251`

Description

advanceAgentSchedule() updates an agent’s nextDueMs differently based on reason.

For normal interval ticks (reason === "interval"), nextDueMs is aligned to the stable phase (computeNextHeartbeatPhaseDueMs).
For targeted/action-driven wakes (hook/manual/exec-event/cron/etc), nextDueMs is set to now + intervalMs.
The global timer (scheduleNext) schedules the next interval wake based on the minimum nextDueMs across agents.

As a result, any targeted wake that successfully runs a heartbeat pushes that agent’s next scheduled interval slot out to now + intervalMs, overriding the phase-aligned cadence. If an untrusted or attacker-controllable event source can repeatedly trigger targeted wakes (e.g. HTTP hook wake via requestHeartbeatNow({ reason: "hook:wake" })), they can keep pushing nextDueMs forward, effectively starving/suppressing interval-driven heartbeats indefinitely.

Impact:

Availability/monitoring: periodic health/heartbeat checks can be delayed indefinitely.
Security monitoring: heartbeat-driven alerts/telemetry may be masked by suppressing scheduled runs.

Vulnerable code:

agent.nextDueMs =
  reason === "interval"
    ? computeNextHeartbeatPhaseDueMs({ nowMs: now, intervalMs: agent.intervalMs, phaseMs: agent.phaseMs })
    : now + agent.intervalMs;

Recommendation

Keep the phase-aligned schedule independent from targeted/action-driven runs.

One approach:

Always compute the next phase due time for interval scheduling.
Track a separate cooldownUntilMs / lastRunMs to prevent too-frequent runs due to targeted wakes.
When handling targeted wakes, update only the cooldown, not the phase schedule.

Example (sketch):

// state
type HeartbeatAgentState = {
  // ...
  phaseMs: number;
  nextDueMs: number;        // next interval slot (phase aligned)
  cooldownUntilMs: number;  // min time before any run is allowed
};

function advanceAfterRun(agent: HeartbeatAgentState, now: number) {
  agent.cooldownUntilMs = now + agent.intervalMs;
  agent.nextDueMs = computeNextHeartbeatPhaseDueMs({
    nowMs: now,
    intervalMs: agent.intervalMs,
    phaseMs: agent.phaseMs,
  });
}

// When deciding to run on interval ticks:
if (reason === "interval" && (now < agent.nextDueMs || now < agent.cooldownUntilMs)) continue;

// When deciding to run on targeted wakes:
if (now < agent.cooldownUntilMs) return { status: "skipped", reason: "cooldown" };

Additionally, consider rate-limiting/authenticating external wake sources (e.g. hook endpoints) so they cannot be abused to manipulate scheduler timing.

Analyzed PR: #64560 at commit 774ede6

_{Last updated on: 2026-04-11T02:41:05Z}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 42d50ed944

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-04-11T00:55:11Z

+      reason === "interval"
+        ? computeNextHeartbeatPhaseDueMs({
+            nowMs: now,
+            intervalMs: agent.intervalMs,
+            phaseMs: agent.phaseMs,


Preserve cooldown after non-interval wakes

After a manual/exec/hook wake, the code still sets nextDueMs = now + interval, but the next interval-triggered run immediately snaps back to the hashed phase via computeNextHeartbeatPhaseDueMs, which can schedule another heartbeat far sooner than a full interval (e.g., minutes or milliseconds later depending on phase). Before this commit, subsequent runs stayed on now + interval, so this introduces a cadence regression for action-driven wakes and can unexpectedly increase heartbeat frequency after user/event-triggered runs.

Useful? React with 👍 / 👎.

greptile-apps · 2026-04-11T00:55:35Z

Greptile Summary

This PR replaces startup-relative now + interval heartbeat scheduling with a stable per-agent phase derived from SHA-256(schedulerSeed:agentId) % intervalMs, so that restart waves no longer cluster heartbeats into provider spikes. Interval wakes snap to the phase slot; targeted/action-driven wakes retain the existing cooldown (now + intervalMs) and then snap back on the next interval tick. Test coverage covers phase stability, config-reload preservation, retry non-drift, and multi-agent routing.

Confidence Score: 5/5

Safe to merge — scheduling math is correct, edge cases are handled, and test coverage is thorough.

No P0 or P1 findings. The phase derivation, cycle-position arithmetic, config-reload preservation, and cooldown/phase bifurcation all behave correctly. The only possible nit is that resolveHeartbeatPhaseMs reads only 4 bytes of the SHA-256 digest (minor modulo bias for large intervals, negligible for the spreading goal), but this is firmly P2 and does not affect correctness.

No files require special attention.

_{Reviews (1): Last reviewed commit: "Heartbeat: spread interval runs across s..." | Re-trigger Greptile}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c67169b4ef

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-04-11T02:13:49Z

+    return createHash("sha256")
+      .update(process.env.HOME ?? "")
+      .update("\0")
+      .update(process.cwd())
+      .digest("hex");


Use unique fallback scheduler seed per gateway

When loadOrCreateDeviceIdentity() fails (for example on read-only or permission-limited state dirs), the fallback seed is derived only from HOME and cwd. In many containerized/service deployments those values are identical across replicas, so different gateways end up with the same schedulerSeed and therefore the same per-agent phase, re-synchronizing interval heartbeats and negating this commit’s load-spreading goal under exactly the degraded path this fallback is meant to handle.

Useful? React with 👍 / 👎.

odysseus0 · 2026-04-11T02:40:26Z

Merged via squash.

Prepared head SHA: 774ede64082e0968943836894c85870dbdc0fe68
Merge commit: 9a4a9a5993cc7862efa4bd876be755fe6c399702

Thanks @odysseus0!

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 774ede6408

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-04-11T02:43:19Z

+    prev &&
+    prev.intervalMs === intervalMs &&
+    prev.phaseMs === phaseMs &&
+    prev.nextDueMs > params.nowMs


Keep missed due slot when heartbeat config hot-reloads

resolveNextHeartbeatDueMs only preserves the previous schedule when prev.nextDueMs > nowMs. On hot reloads (src/gateway/server-reload-handlers.ts:87-89 calls heartbeatRunner.updateConfig for heartbeat-relevant config changes), if reload occurs just after a due boundary, this logic recomputes from nowMs and skips the overdue run until the next phase slot, delaying heartbeats by up to a full interval. Before this change, lastRunMs + interval allowed immediate catch-up for overdue slots.

Useful? React with 👍 / 👎.

@odysseus0