fix(heartbeat): clear pendingFinalDelivery* on send success#83187
Conversation
openclaw-barnacle
Bot
added
size: S
triage: mock-only-proof
|
Codex review: needs maintainer review before merge. Reviewed June 24, 2026, 3:06 PM ET / 19:06 UTC. Summary PR surface: Source +62, Tests +298. Total +360 across 2 files. Reproducibility: yes. Current Review metrics: 2 noteworthy metrics.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land this PR or an equivalent focused heartbeat-runner fix after maintainer acceptance of the createdAt ownership guard and normal merge gates. Do we have a high-confidence way to reproduce the issue? Yes. Current Is this the best way to solve the issue? Yes. Heartbeat-runner is the narrow path that knows visible heartbeat delivery or duplicate suppression succeeded, and the ownership guard preserves older pending-final recovery state that this heartbeat did not produce. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against eac1d3349c29. Label changesLabel justifications:
Evidence reviewedPR surface: Source +62, Tests +298. Total +360 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
clawsweeper
Bot
added
P1
openclaw-barnacle
Bot
added
proof: supplied
clawsweeper
Bot
added
the
proof: sufficient
openclaw-barnacle
Bot
removed
the
proof: sufficient
clawsweeper
Bot
added
proof: sufficient
|
ClawSweeper PR egg: ✨ hatched 🥚 common Clockwork Signal Puff. Rarity: 🥚 common. Trait: hums during re-review. DetailsShare on X: post this hatch
About:
|
|
Sibling PR #87583 was closed as superseded by this canonical cleanup path, which makes sense for the send-success portion. One remaining gap from #87583 may still be worth porting here: duplicate-suppression cleanup. #83187 clears
I ran a disposable local OpenClaw source-runtime proof on #87583 ( Relevant duplicate-suppression proof: {
"scenario": "duplicate-skip",
"result": { "status": "ran", "durationMs": 51 },
"sendCallCount": 0,
"before": {
"pendingFinalDelivery": true,
"pendingFinalDeliveryText": "PR87583 duplicate-skip heartbeat payload",
"pendingFinalDeliveryCreatedAt": 1779957646044,
"pendingFinalDeliveryLastAttemptAt": 1779957706044,
"pendingFinalDeliveryAttemptCount": 2,
"pendingFinalDeliveryLastError": "redacted prior delivery failure",
"pendingFinalDeliveryContext": { "channel": "telegram", "to": "proof-chat" },
"pendingFinalDeliveryIntentId": "intent-duplicate-skip",
"lastHeartbeatText": "PR87583 duplicate-skip heartbeat payload",
"lastHeartbeatSentAt": 1779957706044
},
"after": {
"pendingFinalDelivery": "<absent>",
"pendingFinalDeliveryText": "<absent>",
"pendingFinalDeliveryCreatedAt": "<absent>",
"pendingFinalDeliveryLastAttemptAt": "<absent>",
"pendingFinalDeliveryAttemptCount": "<absent>",
"pendingFinalDeliveryLastError": "<absent>",
"pendingFinalDeliveryContext": "<absent>",
"pendingFinalDeliveryIntentId": "<absent>",
"lastHeartbeatText": "PR87583 duplicate-skip heartbeat payload",
"lastHeartbeatSentAt": 1779957706044
},
"clearedAllPendingFields": true,
"deliveryEvidence": "duplicate suppression skipped outbound send because lastHeartbeatText matched within 24h"
}If maintainers want this PR to remain the single canonical fix, please consider porting the duplicate-suppression cleanup and regression test from #87583 rather than reopening the sibling PR. |
|
Heads up: this PR needs to be updated against current |
openclaw-barnacle
Bot
removed
the
proof: sufficient
|
Button clicked, thanks @RomneyDa |
clawsweeper
Bot
added
the
proof: sufficient
Heartbeat-driven agent runs can leave pendingFinalDelivery set on the session; runHeartbeatOnce wrote lastHeartbeatText/lastHeartbeatSentAt on send success but never cleared the recovery fields, so the next heartbeat hit the get-reply redelivery short-circuit and skipped the agent. Extend the existing post-success updateSessionStore block to null all seven pendingFinalDelivery* fields, mirroring clearPendingFinalDeliveryAfterSuccess in dispatch-from-config.ts. Refs: openclaw#83184
clawsweeper
Bot
added
rating: 🦐 gold shrimp
Add the eighth pending-final field to CLEARED_PENDING_FINAL_DELIVERY_FIELDS so the heartbeat send-success and duplicate-skip paths match the canonical clearPendingFinalDeliveryAfterSuccess cleanup; a delivered heartbeat no longer leaves stale intent metadata behind. Seed and assert the field in both regression cases. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_01SkPieH67337ieQPWVXQgMD
clawsweeper
Bot
added
rating: 🐚 platinum hermit
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
clawsweeper
Bot
removed
the
rating: 🦐 gold shrimp
…ending-final-delivery
…tbeat-clear-pending-final-delivery
The send-success and duplicate-skip clears retired any pendingFinalDelivery unconditionally, so a heartbeat could erase an older, unsatisfied user-facing final it never produced (e.g. a message_tool_only run that does not refresh pending). Gate both clears on ownership via a shared helper: clear only when pendingFinalDeliveryCreatedAt is at/after the run start. Add regression tests covering the older-pending preserve case on both clear paths. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_01SkPieH67337ieQPWVXQgMD
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
5 participants
Summary
Fixes #83184.
The heartbeat send path in
src/infra/heartbeat-runner.ts:2011-2022wrotelastHeartbeatText/lastHeartbeatSentAtafter a successfulsendDurableMessageBatchbut never nulled thependingFinalDelivery*recovery fields. A heartbeat-driven agent run that previously setpendingFinalDelivery: true(viasrc/auto-reply/reply/agent-runner.ts:2169) therefore left the session permanently stuck: subsequent heartbeats short-circuited in get-reply's redelivery path, the agent was never re-invoked, and the symptom surfaced as silent loss of notifications.This change extends the existing post-success
updateSessionStoreblock to clear all eightpendingFinalDelivery*fields, mirroring the canonicalclearPendingFinalDeliveryAfterSuccesshelper atsrc/auto-reply/reply/dispatch-from-config.ts:365-391(already invoked from the user-message dispatch path at:1671).Affected versions: verified on
2026.5.7through2026.5.16-beta.2; fix applies against currentmain(2026.5.17).Update 1 — pending-final field parity (ClawSweeper review)
ClawSweeper flagged that the shared
CLEARED_PENDING_FINAL_DELIVERY_FIELDSset cleared seven of the eight currentpendingFinalDelivery*fields, omittingpendingFinalDeliveryIntentId— which the canonicalclearPendingFinalDeliveryAfterSuccessdoes clear and whichSessionEntry(src/config/sessions/types.ts:353) defines as durable pending-final metadata. A delivered heartbeat therefore left stale intent metadata behind, inconsistent with the dispatch path the helper claims to mirror.Fixed by adding
pendingFinalDeliveryIntentId: undefinedto the constant (src/infra/heartbeat-runner.ts), which both heartbeat clear sites (send-success and duplicate-skip) already spread, so the heartbeat set now matches the canonical eight-field cleanup exactly. Both regression cases now seedpendingFinalDeliveryIntentIdand assert it is cleared. Re-verified locally: removing only the new constant line turns both cases RED (expected 'intent-send-success' to be undefined,expected 'intent-duplicate-skip' to be undefined); restored, the file and ClawSweeper's listed acceptance-criteria siblings (3 files / 57 tests) pass;oxfmt --checkand oxlint clean on both touched files.The live Docker trace under Real behavior proof below predates this addition, so its bundle grep and observations describe the original seven
…: void 0lines.Update 2 — pending-final ownership guard (ClawSweeper re-review)
On re-review ClawSweeper escalated to a P1: the send-success and duplicate-skip clears retired any
pendingFinalDeliverywhenever the flag/text was set, without checking the current heartbeat run owned it. Confirmed against the code — the 30s defer gate only protects recent substantive pending; heartbeat get-reply does not redeliver substantive pending (src/auto-reply/reply/get-reply.ts:510, "must not replay user-facing pending finals through a different delivery target"); and amessage_tool_only/suppressed run does not refresh pending (src/auto-reply/reply/agent-runner.ts:2541,pendingText = suppressDelivery ? "" : …). So an older user-facing final — a delivery-failure recovery marker the get-reply redelivery short-circuit owns (src/auto-reply/reply/dispatch-from-config.ts:3378) — could be erased by a later heartbeat that never satisfied it.Fixed by gating both clear sites on ownership via a shared
heartbeatRunOwnsPendingFinalDeliveryhelper (src/infra/heartbeat-runner.ts): clear only whenpendingFinalDeliveryCreatedAt >= startedAt, i.e. the pending was stamped during this run. The run's own satisfied pending is still cleared (the original fix); an older final the run did not produce is preserved for its own recovery path.Two regression tests added: an older, unowned pending-final is preserved across both the send-success and duplicate-skip paths. Verified RED against the unconditional clear (
expected undefined to be true) → GREEN with the guard; the two original "clears its own pending" cases were updated to seed an ownedcreatedAtand still pass. Full heartbeat-runner suite (16 files / 176 tests) and the siblingsrc/auto-reply/reply/get-reply.fast-path.test.ts(19 tests) pass;oxfmt --checkand oxlint clean on both touched files.Verification
Commands run locally against this branch on Ubuntu 24.04, Node 22.22.2, pnpm 11.1.0:
node scripts/run-vitest.mjs src/infra/heartbeat-runner.clears-pending-final-delivery.test.ts— new regression test, 1 passed (also confirmed RED before the fix landed in the same branch).node scripts/run-vitest.mjs <all 16 src/infra/heartbeat-runner.*.test.ts non-e2e files>— 16 files / 152 tests passed.pnpm check:changed— green: conflict markers, changelog attributions, wildcard re-export guards, duplicate coverage, dependency pin guard, package patch guard, typecheck core, typecheck core tests, lint core (0/0), media download helper guard, runtime sidecar loader guard, runtime import cycles, webhook body guard, pairing store guard, pairing account guard.oxfmt --checkand core oxlint — both clean on the two touched files.The regression file covers two cases. The send-success case seeds five
pendingFinalDelivery*fields via the store seeder plus a heartbeat-ackpendingFinalDeliveryText(so the heartbeat-defer check atheartbeat-runner.ts:~1328does not short-circuit), patches in the three the seeder does not expose (pendingFinalDeliveryLastAttemptAt,pendingFinalDeliveryContext,pendingFinalDeliveryIntentId), runsrunHeartbeatOncewith a substantive reply, and asserts all eight fields areundefinedafterward — plus thatlastHeartbeatText/lastHeartbeatSentAtare written, proving the success branch ran. The duplicate-skip case asserts the same eight-field clear when aresponsePrefix-prefixed payload was already delivered, so the run short-circuits in the duplicate-suppression branch without a new send.Real behavior proof
Behavior addressed: heartbeat send-success path leaves
pendingFinalDelivery*set, causing the next heartbeat to skip the agent via the get-reply redelivery short-circuit.Real environment tested: Docker-deployed gateway on Ubuntu 24.04 (kernel 6.8.0-111-generic). Prior to this patch the gateway was on
OpenClaw 2026.5.12; the bug was deterministic there, confirmed by an out-of-band cron job (*/10 * * * *) that nulls the stuck fields wheneverpendingFinalDeliveryTextmatcheslastHeartbeatText. Cron log baseline showed one "cleared stuck" entry per hour (e.g.,2026-05-17T15:50:01Zand2026-05-17T16:50:01Z, bothagent:main:main) — i.e., every substantive heartbeat left the session stuck and the cron had to clean up.Exact steps or command run after this patch:
fix/heartbeat-clear-pending-final-delivery, currentmain+ the diff in this PR; reportedOpenClaw 2026.5.17).DOCKER_BUILDKIT=1 docker build -t openclaw-source:fix-heartbeat-pending-delivery ..docker compose build && docker compose down && docker compose up -d. New container came up at2026-05-17T17:04:33Z.docker compose exec openclaw-gateway grep -c "pendingFinalDeliveryContext: void 0" /app/dist/heartbeat-runner-*.js→1in the send-path bundle (heartbeat-runner-CecDa3Rk.js),0in the runtime stub (heartbeat-runner-DPGtQP6X.js). Context grep aroundlastHeartbeatSentAt: startedAtshows the sevenpendingFinalDelivery*: void 0lines immediately after it, in the sameupdateSessionStorewrite that recordslastHeartbeatText/lastHeartbeatSentAt.agent:main:mainin the host-mountedsessions.jsonand the cron log over the next ~2 hours of real heartbeats.Evidence after fix:
src/infra/heartbeat-runner.clears-pending-final-delivery.test.ts) goes RED onmain(expected true to be undefinedat thependingFinalDeliveryassertion) and GREEN with this patch. Sister tests insrc/infra/heartbeat-runner.*(16 files / 152 tests) remain green.agent:main:main, host-mounted sessions store, redacted preview):2026-05-17T17:04:33Z— patched container started (OpenClaw 2026.5.17).2026-05-17T17:04:41Z— heartbeat scheduler loggedheartbeat: started intervalMs=1800000. Subsequent ticks ran but were all HEARTBEAT_OK (silent, no store write), keepinglastHeartbeatSentAtat the pre-redeploy2026-05-17T16:40:18Zfor ~2.5 hours.2026-05-17T19:10:18Z— first substantive heartbeat after the redeploy.lastHeartbeatSentAtadvanced to1779045018769,lastHeartbeatTextwritten ("One mildly notable item — everything else is promos: …").2026-05-17T19:11:04Z(≤46s after the send write, well before the next*/10cron tick at19:20:00Z) —pendingFinalDelivery: None,pendingFinalDeliveryText: "". This clean state is attributable only to the patched send-path write, not to the cron.2026-05-17T19:20:00Zcron tick — zero new lines inclear-stuck-pending-delivery.log(baseline was 2 lines, post-tick was still 2 lines). On the unpatched image this is the tick that would have logged a "cleared stuck" line, matching the prior15:50/16:50cadence.Observed result after fix: a real substantive heartbeat in the deployed gateway now leaves all seven
pendingFinalDelivery*fields cleared in the sameupdateSessionStorewrite that recordslastHeartbeatText/lastHeartbeatSentAt, and the standing cron mitigation no longer finds anything to clean up.What was not tested:
pendingFinalDelivery: truerecovery branch where final delivery failed and a retry is genuinely needed — that path is still gated by theclearPendingFinalDeliveryAfterSuccessguard indispatch-from-config.ts; this PR only adds the equivalent clear in the heartbeat send-success path.