Skip to content

Merge branch 'main' of github.com:openclaw/openclaw#46858

Closed
JunJD wants to merge 2 commits into
openclaw:mainfrom
JunJD:main
Closed

Merge branch 'main' of github.com:openclaw/openclaw#46858
JunJD wants to merge 2 commits into
openclaw:mainfrom
JunJD:main

Conversation

@JunJD

@JunJD JunJD commented Mar 15, 2026 •
edited
Loading

Copy link
Copy Markdown

Summary

  • Problem: heartbeat replies treated messages.responsePrefix as a raw string, so template variables
    like {model} were delivered literally instead of being interpolated.
  • Why it matters: normal user-facing replies already resolve {model}, {provider}, and
    {thinkingLevel} from the actual selected model, but heartbeat alerts did not, which made channel
    output inconsistent and broke model-aware prefixes.
  • What changed: wired heartbeat runs into the existing model-selection callback path, captured the
    actual provider/model/think level, and resolved the heartbeat response prefix template before
    building outbound heartbeat text.
  • What did NOT change: heartbeat delivery routing, ack suppression behavior, and model selection
    itself are unchanged.

Change Type

  • Bug fix
  • Feature
  • Refactor
  • Docs
  • Security hardening
  • Chore/infra

Scope

  • Gateway / orchestration
  • Skills / tool execution
  • Auth / tokens
  • Memory / storage
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

User-visible / Behavior Changes

  • Heartbeat alert replies now interpolate messages.responsePrefix template variables the same way
    normal replies do.
  • Example: responsePrefix: "[{model}]" now sends [gemini-2.5-flash] Check complete instead of
    [{model}] Check complete.
  • HEARTBEAT_OK detection still strips the resolved prefix before ack suppression logic runs.

Security Impact

  • 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

Repro + Verification

Environment

  • OS: macOS
  • Runtime/container: local repo checkout
  • Model/provider: simulated in heartbeat test callback
  • Integration/channel: Telegram heartbeat delivery
  • Relevant config (redacted): messages.responsePrefix: "[{model}]"

Steps

  1. Configure heartbeat delivery to an external channel.
  2. Set messages.responsePrefix to a template such as "[{model}]".
  3. Trigger a heartbeat alert reply that returns real text instead of HEARTBEAT_OK.
  4. Observe the outbound message prefix before and after the patch.

Expected

  • Heartbeat alert replies use the resolved model name in the prefix.

Actual

  • Verified in regression coverage: heartbeat alert reply delivers [gemini-2.5-flash] Check complete.

Evidence

  • Failing test/log before + passing after

Focused verification run:

  • pnpm test -- src/infra/heartbeat-runner.respects-ackmaxchars-heartbeat-acks.test.ts -t "interpolates responsePrefix template variables for heartbeat alert replies|strips responsePrefix before HEARTBEAT_OK detection and suppresses short ack text"
  • pnpm test -- src/infra/heartbeat-runner.model-override.test.ts -t "passes heartbeatModelOverride from defaults heartbeat config|uses isolated session key when isolatedSession is enabled"

Human Verification

  • Verified scenarios: heartbeat alert replies interpolate {model} in responsePrefix; ack
    suppression still works with prefixed HEARTBEAT_OK replies.
  • Edge cases checked: heartbeat model override path still works; isolated heartbeat session path
    still works.
  • What you did not verify: live end-to-end delivery against a real Telegram bot/app in this
    branch.

Compatibility / Migration

  • Backward compatible? Yes
  • Config/env changes? No
  • Migration needed? No

Failure Recovery

  • How to disable/revert this change quickly: revert this PR.
  • Known bad symptoms reviewers should watch for: heartbeat prefixes showing literal template
    variables again, or ack-only heartbeat replies no longer being suppressed correctly.

Risks and Mitigations

  • Risk: heartbeat prefix handling could diverge from normal reply prefix handling again in future
    refactors.
    • Mitigation: added focused regression coverage for templated heartbeat prefixes and kept the
      change on the existing onModelSelected path.

@greptile-apps

greptile-apps Bot commented Mar 15, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds template variable interpolation (e.g. {model}, {provider}, {thinkingLevel}) to the responsePrefix string used in heartbeat alert and heartbeat-OK messages. When the LLM call completes, a new onModelSelected callback populates a ResponsePrefixContext, and resolveResponsePrefixTemplate is then used to resolve placeholders before the message is delivered.

Key changes:

  • responsePrefix in runHeartbeatOnce is changed from a const to a let, enabling post-LLM-call reassignment with the resolved template value.
  • onModelSelected is wired into both the heartbeatModelOverride and default replyOpts branches, ensuring consistent behaviour in both paths.
  • A new test verifies end-to-end that [{model}] becomes [gemini-2.5-flash] in the final delivered message.

One thing worth noting: the maybeSendHeartbeatOk closure is defined before the try block at line 765 and reads responsePrefix from its outer scope. Because responsePrefix is a let variable and all call sites for maybeSendHeartbeatOk appear after the template resolution at line 829 (inside the try block), this works correctly. However, if maybeSendHeartbeatOk is ever called from a catch or finally block that runs before line 829, it would send the raw template string (e.g. [{model}]) instead of the resolved value — worth verifying in any future changes to error-handling paths.

Confidence Score: 4/5

  • PR is safe to merge; the template-interpolation logic is correct and well-tested for the primary alert path.
  • The implementation correctly leverages JavaScript's closure-by-reference semantics: responsePrefix is updated before every call site of maybeSendHeartbeatOk. The new response-prefix-template.ts helpers are clean and handle unresolved variables gracefully. The one point of caution is that maybeSendHeartbeatOk could theoretically be called with an unresolved template if error-handling code were to invoke it before line 829 — but no such path exists today. Deducting one point for the absence of a test covering the heartbeat-OK path with a template variable.
  • No files require special attention; the catch/finally block of runHeartbeatOnce should be reviewed if maybeSendHeartbeatOk is ever added there in the future.

Last reviewed commit: c28f542

@clawsweeper clawsweeper Bot mentioned this pull request Apr 27, 2026
Closed
@clawsweeper

clawsweeper Bot commented Apr 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Close as superseded. Current main still has the heartbeat responsePrefix interpolation gap, but the same remaining work is now tracked by the newer focused replacement PR #73996, which explicitly lists this PR as a superseded source, carries the same fix boundary, adds stronger regression coverage, and preserves @JunJD credit.

So I’m closing this here and keeping the remaining discussion on the canonical linked item.

Review details

Best possible solution:

Use #73996 as the canonical implementation path, review or land that replacement after CI, and keep both the original reporter and this overlapping source PR credited in the final changelog/body.

Security review:

Security review cleared: The PR only changes heartbeat TypeScript code and a focused Vitest regression test; it does not touch CI, dependencies, scripts, package metadata, permissions, secrets, or other supply-chain surfaces.

What I checked:

Likely related people:

  • steipete: Recent path history shows multiple heartbeat-runner and response-prefix-adjacent changes, plus local blame on the shallow checkout points current visible lines to the latest main snapshot authored by Peter Steinberger. (role: recent maintainer; confidence: high; commits: 65c9eddae87e, f5e7557c70c1, 7f3f108521f4; files: src/infra/heartbeat-runner.ts, src/infra/heartbeat-runner.respects-ackmaxchars-heartbeat-acks.test.ts, src/auto-reply/reply/response-prefix-template.ts)
  • sebslight: Path history shows sebslight introduced dynamic responsePrefix template variables and also touched heartbeat responsePrefix stripping for ack detection, the two contracts that intersect in this bug. (role: introduced behavior; confidence: medium; commits: d0a4cce41e6d, 3518554e2368; files: src/auto-reply/reply/response-prefix-template.ts, src/infra/heartbeat-runner.respects-ackmaxchars-heartbeat-acks.test.ts)
  • vignesh07: Recent GitHub path history for heartbeat-runner.ts includes heartbeat rebase-conflict and inferred-commitment changes by vignesh07, making them a current routing candidate for conflicts in this area. (role: recent heartbeat maintainer; confidence: medium; commits: 7451415f36d5, 8e4035d09a88; files: src/infra/heartbeat-runner.ts)
  • vincentkoc: Recent history on src/channels/reply-prefix.ts shows vincentkoc carrying reply payload/option contract splits that affect the shared prefix-context seam used by the replacement fix. (role: adjacent contract owner; confidence: medium; commits: a88fbf0f6476, 74e7b8d47b18, a976cc2e957e; files: src/channels/reply-prefix.ts)

Codex review notes: model gpt-5.5, reasoning high; reviewed against 70ddeef0e8cd.

@openclaw-clownfish openclaw-clownfish Bot mentioned this pull request Apr 29, 2026
Merged
@clawsweeper clawsweeper Bot closed this Apr 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

size: S

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Heartbeat replies show literal {model} instead of interpolated model name in responsePrefix

1 participant

@JunJD