Skip to content

fix(heartbeat): suppress no-op system event replies#73785

Open
pfrederiksen wants to merge 2 commits into
openclaw:mainfrom
pfrederiksen:fix/73149-silent-noop-runtime
Open

fix(heartbeat): suppress no-op system event replies#73785
pfrederiksen wants to merge 2 commits into
openclaw:mainfrom
pfrederiksen:fix/73149-silent-noop-runtime

Conversation

@pfrederiksen

@pfrederiksen pfrederiksen commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add centralized no-op sentinel detection for silent heartbeat/system-event replies
  • suppress exact HEARTBEAT_OK, NO_REPLY, NO_NEW_AUDIO, and SESSION_WATCHDOG_OK outputs before delivery
  • keep meaningful relayable exec summaries deliverable without moving cron events out of heartbeat-mode ack suppression
  • preserve newer heartbeat prefix/model-selection behavior after rebasing onto current origin/main

Fixes #73149.

Real behavior proof

  • Behavior or issue addressed: Heartbeat/system-event handoffs could visibly deliver no-op sentinel replies such as exact HEARTBEAT_OK, while still needing to deliver meaningful exec/system-event summaries.

  • Real environment tested: Local OpenClaw checkout for PR fix(heartbeat): suppress no-op system event replies #73785 on Linux/Node 22 after rebasing onto current origin/main at 3de97057d0.

  • Exact steps or command run after this patch: Ran node --import tsx tmp/pr73785-runtime-proof.mjs from the patched checkout. The script imported the patched runHeartbeatOnce, used the real in-process system-event queue, registered the channel plugin send path with outbound capture, seeded a real file-backed session store, and exercised both cron-event and exec-event paths.

  • Evidence after fix: Terminal output copied from the after-fix proof command:

    {
      "cron": {
        "name": "cron",
        "result": {
          "status": "ran",
          "durationMs": 27
        },
        "provider": "cron-event",
        "sent": []
      },
      "exec": {
        "name": "exec",
        "result": {
          "status": "ran",
          "durationMs": 1257
        },
        "provider": "exec-event",
        "sent": [
          {
            "to": "[email protected]",
            "text": "Command completed: uploaded report.txt"
          }
        ]
      }
    }
  • Observed result after fix: A cron-event reply of HEARTBEAT_OK Watchdog clean stayed in heartbeat-mode short-ack suppression and produced no outbound send. A meaningful exec-event reply still crossed the send boundary as Command completed: uploaded report.txt.

  • What was not tested: No real WhatsApp or Telegram message was sent; the proof used a dummy WhatsApp group id and captured the outbound send locally to avoid test noise in real channels.

Validation

  • CI=true pnpm exec oxfmt --check src/agents/model-runtime-policy.test.ts extensions/telegram/src/polling-session.ts src/auto-reply/tokens.ts src/infra/heartbeat-runner.ts src/infra/heartbeat-runner.respects-ackmaxchars-heartbeat-acks.test.ts
  • CI=true pnpm exec oxlint src/agents/model-runtime-policy.test.ts extensions/telegram/src/polling-session.ts src/auto-reply/tokens.ts src/infra/heartbeat-runner.ts src/infra/heartbeat-runner.respects-ackmaxchars-heartbeat-acks.test.ts
  • CI=true pnpm exec vitest run --config test/vitest/vitest.infra.config.ts src/infra/heartbeat-runner.respects-ackmaxchars-heartbeat-acks.test.ts — 16/16 passed
  • CI=true pnpm check:test-types
  • git diff --check

@greptile-apps

greptile-apps Bot commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR refactors heartbeat reply suppression by centralizing no-op token detection into isNoOpSentinelReplyText in tokens.ts, then applying it early in normalizeHeartbeatReply before further token stripping. The previous exec-completion exception (!hasExecCompletion in shouldSkipMain and the execFallbackText fallback) is replaced by simply treating system events with the "message" strip mode, so that HEARTBEAT_OK/NO_REPLY/etc. replies are suppressed even for exec events while meaningful summaries are still delivered.

Confidence Score: 5/5

Safe to merge — logic is straightforward, well-tested, and the behavioral change is intentional and correctly implemented.

No P0 or P1 issues found. The sentinel detection is correctly gated, the early-return path in normalizeHeartbeatReply is sound, the removal of the old execFallbackText workaround is clean, and three new test cases validate all key scenarios.

No files require special attention.

Reviews (1): Last reviewed commit: "fix(heartbeat): suppress no-op system ev..." | Re-trigger Greptile

@pfrederiksen
pfrederiksen force-pushed the fix/73149-silent-noop-runtime branch 2 times, most recently from 7d4d936 to 4483377 Compare April 28, 2026 20:46
@clawsweeper

clawsweeper Bot commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 3, 2026, 4:57 PM ET / 20:57 UTC.

Summary
The PR adds no-op sentinel detection for heartbeat/system-event replies, applies it in heartbeat normalization and exec fallback handling, adds heartbeat regression tests, and includes small Telegram and run-node type fixes.

PR surface: Source +48, Tests +133, Other +1. Total +182 across 5 files.

Reproducibility: yes. from source inspection: current main can strip a pure heartbeat token and then restore it through relayable exec fallback. I did not run a live current-main repro in this read-only pass.

Review metrics: 1 noteworthy metric.

  • Live mergeability: mergeable=CONFLICTING, mergeStateStatus=DIRTY. Maintainers need a refreshed branch before the exact heartbeat delivery merge result can be reviewed or landed.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #73149
Summary: This PR is the candidate fix for the broad cron/heartbeat no-op sentinel leakage report; the open route-matched fallback PR is adjacent message-delivery work, not a replacement.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦞 diamond lobster ✨ media proof bonus
Patch quality: 🦐 gold shrimp
Result: needs maintainer review before merge.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Refresh the branch onto current main while preserving both sentinel suppression and current placeholder-only suppression.
  • Rerun focused heartbeat regression tests and exact merge-head checks after the refresh.

Risk before merge

  • [P1] Live GitHub reports the PR as conflicting, so maintainers cannot review or land the exact merge result yet.
  • [P1] The conflict is in central heartbeat delivery suppression logic; a bad refresh could re-deliver no-op sentinels, drop meaningful exec summaries, or lose current main's internal-placeholder-only suppression.

Maintainer options:

  1. Refresh and validate the heartbeat merge head (recommended)
    Resolve the current heartbeat-runner conflict while preserving sentinel suppression and current placeholder suppression, then rerun focused heartbeat tests and exact merge-head checks.
  2. Pause for a heartbeat owner
    If nobody can safely refresh central delivery logic, leave the PR paused rather than landing stale message-delivery behavior.

Next step before merge

  • [P2] Manual review is needed because the remaining blocker is conflict resolution and exact merge-head validation in central heartbeat delivery logic.

Security
Cleared: No concrete security or supply-chain concern was found; the diff does not change dependencies, workflows, secrets handling, package resolution, or privileged execution paths.

Review details

Best possible solution:

Refresh the branch onto current main while preserving both no-op sentinel suppression and current internal-placeholder-only suppression, then rerun focused heartbeat and exact merge-head validation.

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

Yes from source inspection: current main can strip a pure heartbeat token and then restore it through relayable exec fallback. I did not run a live current-main repro in this read-only pass.

Is this the best way to solve the issue?

Yes after refresh: fixing the heartbeat normalization/fallback boundary is narrower and more durable than prompt-only or channel-specific suppression. The conflicted branch cannot be treated as final until it is resolved against current main.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 010b61746379.

Label changes

Label justifications:

  • P2: The PR addresses a user-visible heartbeat/system-event message leak with limited blast radius and no security or data-loss signal.
  • merge-risk: 🚨 message-delivery: The unresolved conflict sits in heartbeat delivery suppression logic and can change whether system-event replies are sent or suppressed.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (screenshot): The PR body includes after-fix terminal output for cron and exec paths, and Mantis supplied Telegram Desktop screenshot proof for the visible sentinel behavior.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix terminal output for cron and exec paths, and Mantis supplied Telegram Desktop screenshot proof for the visible sentinel behavior.
  • proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. The PR body includes after-fix terminal output for cron and exec paths, and Mantis supplied Telegram Desktop screenshot proof for the visible sentinel behavior.
  • mantis: telegram-visible-proof: Mantis should capture Telegram visible proof. The PR changes visible Telegram heartbeat/sentinel delivery behavior that is suitable for Telegram Desktop proof.
Evidence reviewed

PR surface:

Source +48, Tests +133, Other +1. Total +182 across 5 files.

View PR surface stats
Area Files Added Removed Net
Source 3 56 8 +48
Tests 1 134 1 +133
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 1 1 0 +1
Total 5 191 9 +182

What I checked:

  • Root and scoped policy read: Root, extensions, Telegram, scripts, and agents guidance were read; the review applied the repository policy that heartbeat fallback and message-delivery behavior are merge-risk-sensitive, and Telegram-visible behavior needs real proof. (AGENTS.md:1, 010b61746379)
  • Live PR state: GitHub reports head 9da5df9 as mergeable=CONFLICTING and mergeStateStatus=DIRTY, so exact merge-head validation cannot be trusted yet. (9da5df9d2d24)
  • Current main still has the fallback leak shape: Current main still restores original reply text for relayable exec completions when normalization strips to empty, which can re-deliver a pure HEARTBEAT_OK unless the PR's sentinel guard is integrated. (src/infra/heartbeat-runner.ts:2050, 010b61746379)
  • Current main has adjacent placeholder suppression to preserve: Current main now carries isInternalPlaceholderOnly handling in normalizeHeartbeatReply, so conflict resolution must preserve that newer suppression while adding the PR's sentinel handling. (src/infra/heartbeat-runner.ts:873, 010b61746379)
  • PR implementation: The PR head adds isNoOpSentinelReplyText, applies it before heartbeat stripping, and prevents exec fallback restoration when the source reply is a no-op sentinel. (src/infra/heartbeat-runner.ts:824, 9da5df9d2d24)
  • PR regression coverage: The PR adds focused tests for exact no-op sentinels, exec HEARTBEAT_OK suppression, meaningful exec summary delivery, and cron short-ack suppression. (src/infra/heartbeat-runner.respects-ackmaxchars-heartbeat-acks.test.ts:361, 9da5df9d2d24)

Likely related people:

  • TurboTheTurtle: Authored the current-main stream-error placeholder suppression in the same heartbeat normalization/fallback area that now conflicts with this PR refresh. (role: recent adjacent contributor; confidence: medium; commits: b580258e94e0; files: src/infra/heartbeat-runner.ts, src/infra/heartbeat-runner.tool-response.test.ts)
  • vincentkoc: Recent work touched heartbeat response-tool prompts, quoted silent-reply handling, and the run-node declaration file adjacent to this PR. (role: adjacent heartbeat and silent-reply contributor; confidence: medium; commits: 877eb1cbed04, 96c6f8022c24, 7546231762ff; files: src/infra/heartbeat-runner.ts, src/auto-reply/tokens.ts, scripts/run-node.d.mts)
  • steipete: Recent history includes heartbeat scheduler and central runner changes in the same runtime file. (role: recent heartbeat area contributor; confidence: medium; commits: bbc4bee7a22b, 180a970ac045; files: src/infra/heartbeat-runner.ts)
  • obviyus: Recent Telegram polling-session work touches the plugin file changed by this PR and is relevant to the Telegram-visible proof surface. (role: Telegram adjacent contributor; confidence: medium; commits: b7ec2851f38d; files: extensions/telegram/src/polling-session.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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 keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@pfrederiksen

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented May 14, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@pfrederiksen
pfrederiksen force-pushed the fix/73149-silent-noop-runtime branch from 2116d31 to 49a3a14 Compare May 14, 2026 05:44
@openclaw-barnacle openclaw-barnacle Bot added the proof: supplied External PR includes structured after-fix real behavior proof. label May 14, 2026
@pfrederiksen
pfrederiksen force-pushed the fix/73149-silent-noop-runtime branch from 49a3a14 to 50ec204 Compare May 15, 2026 20:20

pfrederiksen commented May 15, 2026

Copy link
Copy Markdown
Contributor Author

Updated for ClawSweeper’s asks:

  • Rebased onto current origin/main (0b7ff665f6).
  • Fixed the P2 by keeping cron events on heartbeat-mode ack suppression; only relayable exec completions use message-mode normalization.
  • Added regression coverage for cron-event short-ack suppression (HEARTBEAT_OK Watchdog clean now produces no send).
  • Added a terminal real-behavior proof to the PR body showing cron no-op sends [] while meaningful exec summaries still cross the send boundary.

Validation run:

  • CI=true pnpm exec oxfmt --check src/auto-reply/tokens.ts src/infra/heartbeat-runner.ts src/infra/heartbeat-runner.respects-ackmaxchars-heartbeat-acks.test.ts
  • CI=true pnpm exec oxlint src/auto-reply/tokens.ts src/infra/heartbeat-runner.ts src/infra/heartbeat-runner.respects-ackmaxchars-heartbeat-acks.test.ts
  • CI=true pnpm exec vitest run --config test/vitest/vitest.infra.config.ts src/infra/heartbeat-runner.respects-ackmaxchars-heartbeat-acks.test.ts — 16/16 passed
  • git diff --check

CI=true pnpm check:test-types currently fails in unrelated current-main fixture src/agents/model-runtime-policy.test.ts because ModelProviderConfig now requires baseUrl.

@clawsweeper re-review

Re-review progress:

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

clawsweeper Bot commented May 15, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Updated the PR body with the exact real-behavior proof fields required by the policy and verified it locally with evaluateRealBehaviorProof before posting.

@openclaw-barnacle 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 15, 2026
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 15, 2026
@pfrederiksen
pfrederiksen force-pushed the fix/73149-silent-noop-runtime branch from 50ec204 to bc82710 Compare May 15, 2026 20:45
@openclaw-barnacle openclaw-barnacle Bot added channel: telegram Channel integration: telegram agents Agent runtime and tooling labels May 15, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels May 30, 2026
@openclaw-barnacle

Copy link
Copy Markdown

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

@openclaw-barnacle openclaw-barnacle Bot added stale Marked as stale due to inactivity and removed stale Marked as stale due to inactivity labels Jun 14, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. mantis: telegram-visible-proof Mantis should capture Telegram visible proof. and removed proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. mantis: telegram-visible-proof Mantis should capture Telegram visible proof. labels Jun 19, 2026
@clawsweeper
clawsweeper Bot temporarily deployed to qa-live-shared June 19, 2026 14:53 Inactive
@openclaw-mantis

Copy link
Copy Markdown
Contributor

Mantis Telegram Desktop Proof

Summary: Mantis captured native Telegram Desktop before/after GIFs showing the no-op heartbeat reply is suppressed in this PR.

Main screenshot This PR screenshot
Baseline native Telegram Desktop screenshot Candidate native Telegram Desktop screenshot
Main This PR
Baseline native Telegram Desktop proof GIF Candidate native Telegram Desktop proof GIF

Motion-trimmed clips:

Raw QA files: https://artifacts.openclaw.ai/mantis/telegram-desktop/pr-73785/run-27832738513-1/index.json

@clawsweeper clawsweeper Bot added proof: 🎥 video Contributor real behavior proof includes video or recording evidence. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. proof: 🎥 video Contributor real behavior proof includes video or recording evidence. labels Jun 19, 2026
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. labels Jun 29, 2026
@clawsweeper clawsweeper Bot added merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. and removed merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. labels Jul 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

channel: telegram Channel integration: telegram mantis: telegram-visible-proof Mantis should capture Telegram visible proof. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. P2 Normal backlog priority with limited blast radius. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. proof: sufficient ClawSweeper judged the real behavior proof convincing. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. scripts Repository scripts size: M status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cron/heartbeat no-op runs can leak visible messages despite silence/no-op semantics

1 participant