Skip to content

fix(heartbeat): skip reasoning payloads when includeReasoning is false (#92260)#92262

Closed
liuhao1024 wants to merge 1 commit into
openclaw:mainfrom
liuhao1024:fix/heartbeat-reasoning-payload-selection
Closed

fix(heartbeat): skip reasoning payloads when includeReasoning is false (#92260)#92262
liuhao1024 wants to merge 1 commit into
openclaw:mainfrom
liuhao1024:fix/heartbeat-reasoning-payload-selection

Conversation

@liuhao1024

@liuhao1024 liuhao1024 commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes resolveHeartbeatReplyPayload to skip reasoning/thinking payloads when includeReasoning is false, preventing the heartbeat from delivering internal model reasoning as a user-visible channel message.

Related Issue

Fixes #92260

Type of Change

  • Bug fix (non-breaking)

Changes Made

  • src/auto-reply/heartbeat-reply-payload.ts: Added optional includeReasoning option to resolveHeartbeatReplyPayload. When false, payloads with isReasoning === true are skipped during selection.
  • src/infra/heartbeat-runner.ts: Moved includeReasoning computation before the resolveHeartbeatReplyPayload call and passes it through.
  • src/plugin-sdk/reply-runtime.ts: Added type export for ResolveHeartbeatReplyPayloadOptions.
  • src/auto-reply/heartbeat-reply-payload.test.ts: Added 12 test cases covering reasoning payload skipping, backward compatibility, and edge cases.

How to Test

  1. Run: node scripts/run-vitest.mjs run src/auto-reply/heartbeat-reply-payload.test.ts
  2. All 12 tests should pass
  3. Also verify plugin-sdk tests: node scripts/run-vitest.mjs run src/plugin-sdk/reply-payload.test.ts

Real behavior proof

Behavior or issue addressed:
resolveHeartbeatReplyPayload iterates reply payloads in reverse and returns the last one with outbound content. It never checked isReasoning, so when a reasoning model (e.g. xai/grok-4.3) places a reasoning payload after the final text payload, the reasoning text was selected as the heartbeat reply and delivered to the channel — even when includeReasoning is false.

Real environment tested:
macOS, Node.js v22, vitest 4.1.7

Exact steps or command run after the patch:

node scripts/run-vitest.mjs run src/auto-reply/heartbeat-reply-payload.test.ts --reporter=verbose

Evidence after fix:

 ✓  unit-fast  src/auto-reply/heartbeat-reply-payload.test.ts > resolveHeartbeatReplyPayload > returns undefined for undefined input
 ✓  unit-fast  src/auto-reply/heartbeat-reply-payload.test.ts > resolveHeartbeatReplyPayload > returns single payload directly
 ✓  unit-fast  src/auto-reply/heartbeat-reply-payload.test.ts > resolveHeartbeatReplyPayload > returns undefined for single reasoning payload when includeReasoning is false
 ✓  unit-fast  src/auto-reply/heartbeat-reply-payload.test.ts > resolveHeartbeatReplyPayload > returns single reasoning payload when includeReasoning is true
 ✓  unit-fast  src/auto-reply/heartbeat-reply-payload.test.ts > resolveHeartbeatReplyPayload > returns single reasoning payload by default (backward compat)
 ✓  unit-fast  src/auto-reply/heartbeat-reply-payload.test.ts > resolveHeartbeatReplyPayload > picks the last outbound payload from array
 ✓  unit-fast  src/auto-reply/heartbeat-reply-payload.test.ts > resolveHeartbeatReplyPayload > skips reasoning payloads when includeReasoning is false
 ✓  unit-fast  src/auto-reply/heartbeat-reply-payload.test.ts > resolveHeartbeatReplyPayload > returns undefined when all payloads are reasoning and includeReasoning is false
 ✓  unit-fast  src/auto-reply/heartbeat-reply-payload.test.ts > resolveHeartbeatReplyPayload > returns reasoning payload when includeReasoning is true
 ✓  unit-fast  src/auto-reply/heartbeat-reply-payload.test.ts > resolveHeartbeatReplyPayload > skips null/undefined entries in array
 ✓  unit-fast  src/auto-reply/heartbeat-reply-payload.test.ts > resolveHeartbeatReplyPayload > returns undefined when no outbound content exists
 ✓  unit-fast  src/auto-reply/heartbeat-reply-payload.test.ts > resolveHeartbeatReplyPayload > handles reasoning payload without isReasoning flag as normal payload

 Test Files  1 passed (1)
      Tests  12 passed (12)

Observed result after fix:
Reasoning payloads are correctly skipped when includeReasoning is false. The fix is backward-compatible — existing callers without the options parameter retain the previous behavior.

What was not tested:
Full OpenClaw gateway with a live reasoning model (e.g. xai/grok). The fix is verified via focused unit tests on the payload selection logic.

Checklist

  • Read Contributing Guide
  • Conventional Commits
  • Searched for duplicates
  • Only related changes
  • Tests pass
  • Added tests

@liuhao1024
fix(heartbeat): skip reasoning payloads in resolveHeartbeatReplyPaylo…
43a11ff 
…ad when includeReasoning is false

resolveHeartbeatReplyPayload picks the last outbound-capable payload but
never checked the isReasoning flag. When a reasoning model (e.g. xai/grok)
places a reasoning payload after the final text payload, it was selected as
the heartbeat reply and delivered to the channel — even with
includeReasoning: false.

Add an optional includeReasoning option to resolveHeartbeatReplyPayload.
When false, reasoning payloads (isReasoning === true) are skipped. The
heartbeat runner now computes includeReasoning before calling the function
and passes it through.

Fixes openclaw#92260
@openclaw-barnacle openclaw-barnacle Bot added size: S proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 11, 2026
@clawsweeper

clawsweeper Bot commented Jun 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 11, 2026, 4:52 PM ET / 20:52 UTC.

Summary
Review failed before ClawSweeper could summarize the requested change.

PR surface: Source +16, Tests +84. Total +100 across 4 files.

Reproducibility: unclear. The review failed before ClawSweeper could establish a reproduction path.

Review metrics: none identified.

Merge readiness
Overall: 🌊 off-meta tidepool
Proof: 🌊 off-meta tidepool
Patch quality: 🌊 off-meta tidepool
Result: rating does not apply to this item.

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

Risk before merge

  • [P1] No close action taken because the review did not complete.

Maintainer options:

  1. Decide the mitigation before merge
    Retry the Codex review after fixing the execution failure.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] Review did not complete, so no work-lane recommendation was made.
Review details

Best possible solution:

Retry the Codex review after fixing the execution failure.

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

Unclear. The review failed before ClawSweeper could establish a reproduction path.

Is this the best way to solve the issue?

Unclear. Retry the review first so ClawSweeper can evaluate the actual issue and fix direction.

AGENTS.md: unclear because the file could not be read completely.

Codex review notes: model internal, reasoning high; reviewed against 9827490f5f73.

Label changes

Label changes:

  • add rating: 🌊 off-meta tidepool: Overall readiness is 🌊 off-meta tidepool; proof is 🌊 off-meta tidepool and patch quality is 🌊 off-meta tidepool.

Label justifications:

  • rating: 🌊 off-meta tidepool: Overall readiness is 🌊 off-meta tidepool; proof is 🌊 off-meta tidepool and patch quality is 🌊 off-meta tidepool.
Evidence reviewed

PR surface:

Source +16, Tests +84. Total +100 across 4 files.

View PR surface stats
Area Files Added Removed Net
Source 3 17 1 +16
Tests 1 84 0 +84
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 4 101 1 +100

What I checked:

  • failure reason: retryable codex transport failure.
  • codex failure detail: Codex review failed for this PR with exit 1.
  • codex stderr: resolveHeartbeatReplyPayload(replyResult, {\n+ includeReasoning,\n+ });\n const reasoningPayloads = includeReasoning\n ? resolveHeartbeatReasoningPayloads(replyResult).filter((payload) => payload !== replyPayload)\n : [];".
  • codex stdout: No stdout captured.

Likely related people:

  • unknown: Codex failed before it could trace repository history. (role: review did not complete; confidence: low)
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.

@clawsweeper clawsweeper Bot added the rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. label Jun 11, 2026
@liuhao1024

liuhao1024 commented Jun 11, 2026

Copy link
Copy Markdown
Contributor Author

Closing this PR — received off-meta tidepool rating. The heartbeat reasoning payload fix requires real-environment validation (live agent sessions with reasoning models) that cannot be performed in an automated cron context. The fix direction (skip reasoning payloads when includeReasoning is false) is likely correct, but without live environment proof this PR cannot be validated per ClawSweeper review standards.

@liuhao1024 liuhao1024 closed this Jun 11, 2026
@harjothkhara harjothkhara mentioned this pull request Jun 11, 2026
Closed
This was referenced Jun 20, 2026
Merged
Closed
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

proof: supplied External PR includes structured after-fix real behavior proof. rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. size: S

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: Heartbeat delivers reasoning payload as main reply when includeReasoning is false (resolveHeartbeatReplyPayload ignores isReasoning)

1 participant

@liuhao1024