fix(heartbeat): skip pendingFinalDelivery when reply strips to HEARTBEAT_OK by hclsys · Pull Request #79270 · openclaw/openclaw

hclsys · 2026-05-08T05:35:58Z

Summary

keep heartbeat replies that strip to acknowledgement-only text out of pendingFinalDelivery so HEARTBEAT_OK does not become durable replay text
clear existing ack-only heartbeat pending replay state before getReplyFromConfig retries it, preserving normal pending replay for real content
let recent ack-only pending state pass the heartbeat runner's 30s requests-in-flight defer guard instead of self-refreshing the skip window
add regression coverage for ack-only suppression, real heartbeat content preservation, and runner defer behavior

Audit

Audit A (existing helper): reused existing stripHeartbeatToken; no new token parser
Audit B (shared callers): helper contract preserved; call sites add heartbeat-specific gating only
Audit C (broader rival): no non-hclsys rival PR found; this updates existing hclsys fix(heartbeat): skip pendingFinalDelivery when reply strips to HEARTBEAT_OK #79270 instead of opening a duplicate

Real behavior proof

Behavior or issue addressed: heartbeat ack text such as HEARTBEAT_OK or All clear.\n\nHEARTBEAT_OK must not become durable pending final-delivery replay state, while real heartbeat notification text must still be preserved for delivery recovery.
Real environment tested: local OpenClaw checkout on current origin/main (0f31b6424e) using production TypeScript modules through node --import tsx, plus the affected project test shards.
Exact steps or command run after this patch:

node --import tsx - <<'NODEEOF'
import { stripHeartbeatToken } from './src/auto-reply/heartbeat.ts';
const ack = stripHeartbeatToken('HEARTBEAT_OK', { mode: 'heartbeat' });
const preambleAck = stripHeartbeatToken('All clear.\n\nHEARTBEAT_OK', { mode: 'heartbeat' });
const real = stripHeartbeatToken('Sent daily summary to channel.', { mode: 'heartbeat' });
console.log(JSON.stringify({ ack, preambleAck, real }, null, 2));
NODEEOF
pnpm test src/infra/heartbeat-runner.skips-busy-session-lane.test.ts src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts
pnpm test src/infra/heartbeat-runner.returns-default-unset.test.ts src/infra/heartbeat-runner.skips-busy-session-lane.test.ts src/auto-reply/reply/agent-runner.runreplyagent.e2e.test.ts src/auto-reply/reply/dispatch-from-config.reply-dispatch.test.ts src/auto-reply/reply/session.heartbeat-no-reset.test.ts
pnpm check:changed

Evidence after fix: terminal output from the production heartbeat token classifier after this patch:

{
  "ack": {
    "shouldSkip": true,
    "text": "",
    "didStrip": true
  },
  "preambleAck": {
    "shouldSkip": true,
    "text": "",
    "didStrip": true
  },
  "real": {
    "shouldSkip": false,
    "text": "Sent daily summary to channel.",
    "didStrip": false
  }
}

Observed result after fix: the production classifier now separates ack-only heartbeat output from real notification text; the changed call sites use that result so ack-only pending state is skipped/cleared and real text remains eligible for pending final delivery. Local focused heartbeat runner + agent-runner tests passed 40 tests, broader acceptance tests passed 87 tests, and pnpm check:changed passed.
What was not tested: no live gateway heartbeat daemon was run, no real 60-minute production heartbeat interval was observed, and no external Telegram/Discord delivery adapter was invoked.

clawsweeper · 2026-05-08T05:39:09Z

Codex review: needs real behavior proof before merge.

Summary
This PR suppresses or clears acknowledgement-only HEARTBEAT_OK heartbeat pending final-delivery state, preserves real heartbeat content, adjusts the heartbeat defer check, and adds regression tests plus a changelog entry.

Reproducibility: yes. source-reproducible. The linked report gives deterministic v2026.5.7 steps, and current main source shows the pending write, heartbeat replay attempt update, and defer-window loop; I did not run a live gateway reproduction in this read-only review.

Real behavior proof
Needs stronger real behavior proof before merge: The PR supplies terminal classifier output and test results, but not after-fix proof of the actual pending final-delivery write/clear path in a real setup. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, ask a maintainer to comment @clawsweeper re-review.

Next step before merge
The PR needs contributor follow-up for both the remaining ackMaxChars cleanup bug and real behavior proof; automation should not take over while the proof gate is unsatisfied.

Security
Cleared: The diff is limited to TypeScript heartbeat/session logic, tests, and changelog text, with no dependency, workflow, secret, or supply-chain surface change.

Review findings

[P2] Thread ackMaxChars into pending replay cleanup — src/auto-reply/reply/get-reply.ts:349

Review details

Best possible solution:

Keep the PR focused on heartbeat pending delivery, thread the same resolved ackMaxChars policy through write, defer, and replay cleanup paths, and add redacted terminal or log proof of the actual session-store state transitions.

Do we have a high-confidence way to reproduce the issue?

Yes, source-reproducible. The linked report gives deterministic v2026.5.7 steps, and current main source shows the pending write, heartbeat replay attempt update, and defer-window loop; I did not run a live gateway reproduction in this read-only review.

Is this the best way to solve the issue?

No, not yet. Reusing stripHeartbeatToken is the right direction, but the replay cleanup path must use the same resolved ackMaxChars policy as the runner/write path before this is the narrow maintainable fix.

Full review comments:

[P2] Thread ackMaxChars into pending replay cleanup — src/auto-reply/reply/get-reply.ts:349
This new cleanup branch classifies existing pending heartbeat text with the default 300-character ack window, while the runner contract allows heartbeat.ackMaxChars to be lowered and current coverage expects ackMaxChars: 0 to deliver HEARTBEAT_OK 🦞. A pre-existing pending value with short real content can therefore be cleared instead of replayed. Pass the resolved heartbeat ack options into this cleanup check and cover the non-default case.
Confidence: 0.89

Overall correctness: patch is incorrect
Overall confidence: 0.87

What I checked:

Current main persists heartbeat ack text before stripping: Current main writes any non-empty final payload into pendingFinalDeliveryText; there is no heartbeat-specific stripHeartbeatToken gate before the durable pending write. (src/auto-reply/reply/agent-runner.ts:1902, 132bcebe4110)
Current main refreshes pending replay attempts on heartbeat: Current main returns the pending final-delivery text on heartbeat and updates pendingFinalDeliveryLastAttemptAt, attempt count, and updatedAt, matching the linked loop mechanics. (src/auto-reply/reply/get-reply.ts:343, 132bcebe4110)
Current main defers recent pending final-delivery sessions: The heartbeat runner still skips when pendingFinalDelivery is true and the session updatedAt is inside the 30-second defer window. (src/infra/heartbeat-runner.ts:1221, 132bcebe4110)
PR head threads ackMaxChars through the write path only: The PR head resolves heartbeatAckMaxChars before persisting new pending heartbeat text, but that fix is not applied to the existing-pending cleanup path in get-reply.ts. (src/auto-reply/reply/agent-runner.ts:1907, c114a56ff06d)
PR head cleanup path still uses default ackMaxChars: isHeartbeatPendingFinalDeliveryEffectivelyEmpty calls stripHeartbeatToken(text, { mode: "heartbeat" }) without the resolved heartbeat ackMaxChars, so pre-existing pending text can be cleared under a different policy than the runner uses. (src/auto-reply/reply/get-reply.ts:349, c114a56ff06d)
ackMaxChars is an existing delivery contract: Docs describe agents.defaults.heartbeat.ackMaxChars, and current tests cover ackMaxChars: 0 delivering HEARTBEAT_OK 🦞 instead of suppressing it. (src/infra/heartbeat-runner.respects-ackmaxchars-heartbeat-acks.test.ts:158, 132bcebe4110)

Likely related people:

MertBasar0: The durable main-session pending final-delivery behavior appears to date to commit c240e71, which touched the central pending delivery and heartbeat recovery surface. (role: introduced behavior; confidence: high; commits: c240e718e91e; files: src/auto-reply/reply/agent-runner.ts, src/auto-reply/reply/get-reply.ts, src/infra/heartbeat-runner.ts)
steipete: Current line blame for the pending write, retry, and defer blocks points to Peter Steinberger's recent strict-gate cleanup commit, so he is a useful current-layout routing owner even though the product behavior predates that cleanup. (role: recent maintainer; confidence: medium; commits: 9e58cc82c8d3; files: src/auto-reply/reply/agent-runner.ts, src/auto-reply/reply/get-reply.ts, src/infra/heartbeat-runner.ts)
pashpashpash: Recent merged heartbeat work includes response-tool and Codex heartbeat prompt behavior near this runner surface, making this a plausible adjacent reviewer for heartbeat semantics. (role: adjacent heartbeat owner; confidence: medium; commits: 5bcc3213434e, b2c3202a15e7; files: src/infra/heartbeat-runner.ts, src/auto-reply/heartbeat.ts)

Remaining risk / open question:

The existing-pending cleanup path can still clear short post-token content that a configured ackMaxChars value would require delivering.
The contributor-supplied proof does not yet demonstrate the changed pending final-delivery session-store behavior outside mocked tests.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 297a16453661.

shouldSkipHeartbeatPendingFinalDelivery was using the default 300-char threshold regardless of per-agent heartbeat config. Replace with inline logic that resolves ackMaxChars from cfg.agents[agentId].heartbeat -> cfg.agents.defaults.heartbeat -> DEFAULT_HEARTBEAT_ACK_MAX_CHARS. Also fix: store the stripped text (remainder after HEARTBEAT_OK) rather than the raw payload text. Previously pendingFinalDeliveryText would have contained the HEARTBEAT_OK prefix, causing heartbeat-runner to re-deliver it verbatim on retry. Resolves clawsweeper P2 review finding on openclaw#79270.

shouldSkipHeartbeatPendingFinalDelivery was using the default 300-char threshold regardless of per-agent heartbeat config. Replace with inline logic that resolves ackMaxChars from cfg.agents[agentId].heartbeat -> cfg.agents.defaults.heartbeat -> DEFAULT_HEARTBEAT_ACK_MAX_CHARS. Also fix: store the stripped text (remainder after HEARTBEAT_OK) rather than the raw payload text. Previously pendingFinalDeliveryText would have contained the HEARTBEAT_OK prefix, causing heartbeat-runner to re-deliver it verbatim on retry. Resolves clawsweeper P2 review finding on #79270.

clawsweeper · 2026-05-23T18:12:28Z

🦞🧹
ClawSweeper could not start a re-review for this item.

Reason: re-review requires an open issue or PR.

shouldSkipHeartbeatPendingFinalDelivery was using the default 300-char threshold regardless of per-agent heartbeat config. Replace with inline logic that resolves ackMaxChars from cfg.agents[agentId].heartbeat -> cfg.agents.defaults.heartbeat -> DEFAULT_HEARTBEAT_ACK_MAX_CHARS. Also fix: store the stripped text (remainder after HEARTBEAT_OK) rather than the raw payload text. Previously pendingFinalDeliveryText would have contained the HEARTBEAT_OK prefix, causing heartbeat-runner to re-deliver it verbatim on retry. Resolves clawsweeper P2 review finding on openclaw#79270.

openclaw-barnacle Bot added size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 8, 2026

HCL added 4 commits May 8, 2026 13:40

fix(heartbeat): ignore ack-only pending delivery replay

f17fff7

test(heartbeat): cover ack-only pending delivery loops

65f0bed

chore(changelog): note heartbeat pending replay fix

8a8435e

test(heartbeat): preserve real pending delivery text

38a3ad3

openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 8, 2026

openclaw-barnacle Bot added size: M and removed size: S labels May 8, 2026

This comment was marked as low quality.

Sign in to view

clawsweeper Bot mentioned this pull request May 8, 2026

fix: break heartbeat pendingFinalDelivery death loop (#79258) #79303

Closed

This comment was marked as low quality.

Sign in to view

steipete mentioned this pull request May 10, 2026

fix(heartbeat): prevent ack-only pending delivery loops #80357

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(heartbeat): skip pendingFinalDelivery when reply strips to HEARTBEAT_OK#79270

fix(heartbeat): skip pendingFinalDelivery when reply strips to HEARTBEAT_OK#79270
hclsys wants to merge 5 commits into
openclaw:mainfrom
hclsys:fix/heartbeat-pending-delivery-on-heartbeat-ok-79258

hclsys commented May 8, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented May 8, 2026 •

edited

Loading

Uh oh!

This comment was marked as low quality.

This comment was marked as low quality.

This comment was marked as low quality.

clawsweeper Bot commented May 23, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

hclsys commented May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Audit

Real behavior proof

Uh oh!

clawsweeper Bot commented May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as low quality.

This comment was marked as low quality.

This comment was marked as low quality.

clawsweeper Bot commented May 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

hclsys commented May 8, 2026 •

edited

Loading

clawsweeper Bot commented May 8, 2026 •

edited

Loading

clawsweeper Bot commented May 23, 2026 •

edited

Loading