Skip to content

fix(agent): suppress heartbeat tool feedback#1937

Merged
alexhoshina merged 1 commit intosipeed:mainfrom
xiwuqi:wuxi/fix-heartbeat-tool-feedback
Mar 25, 2026
Merged

fix(agent): suppress heartbeat tool feedback#1937
alexhoshina merged 1 commit intosipeed:mainfrom
xiwuqi:wuxi/fix-heartbeat-tool-feedback

Conversation

@xiwuqi
Copy link
Copy Markdown
Contributor

@xiwuqi xiwuqi commented Mar 23, 2026

📝 Description

Fixes the heartbeat tool-feedback leak behind #1824: when PicoClaw restarts and the heartbeat runs from HEARTBEAT.md, the agent can publish inline tool feedback messages back to the last user channel.

Problem summary:

  • Heartbeat turns run against the last active channel/chat so they have user context.
  • ProcessHeartbeat() disables the final user response, but tool-call previews were still published during the turn.
  • On restart, users can receive 🔧 read_file, 🔧 exec, and similar heartbeat-internal feedback even though no direct reply should be sent.

Root cause:

  • runTurn() publishes inline tool feedback whenever tool feedback is enabled and a channel is present.
  • Heartbeat turns pass the real channel/chat into the loop, so they matched the same branch as normal chat turns.
  • There was no heartbeat-specific suppression for these intermediate messages.

What changed:

  • Add a focused SuppressToolFeedback option to processOptions.
  • Enable that option for ProcessHeartbeat() so heartbeat turns keep their execution context without leaking inline tool feedback.
  • Add a regression test that reproduces the heartbeat tool-call path and verifies no outbound feedback is published.
  • Add a protection test that confirms regular user messages still publish tool feedback when the setting is enabled.

Why this fix:

  • It is the smallest change that directly addresses #1824.
  • It preserves the existing heartbeat flow, last-channel context, and normal chat tool-feedback behavior.
  • It avoids broader changes to heartbeat routing or outbound delivery semantics.

Risk / compatibility:

  • Low risk. The only behavioral change is that heartbeat turns no longer emit inline tool-call previews to the user channel.
  • Regular chat turns continue to publish tool feedback unchanged.

🗣️ Type of Change

  • 🐞 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 📖 Documentation update
  • ⚡ Code refactoring (no functional changes, no api changes)

🤖 AI Code Generation

  • 🤖 Fully AI-generated (100% AI, 0% Human)
  • 🛠️ Mostly AI-generated (AI draft, Human verified/modified)
  • 👨‍💻 Mostly Human-written (Human lead, AI assisted or none)

🔗 Related Issue

Fixes #1824

📚 Technical Context (Skip for Docs)

  • Reference URL: [BUG] Agent run heartbeat from example in HEARTBEAT.md #1824
  • Reasoning: The heartbeat service already treats final heartbeat results as silent in the normal HEARTBEAT_OK case. The user-visible leak came from the generic inline tool-feedback path in runTurn(), so the least risky fix is to suppress that path specifically for heartbeat turns.

🧪 Test Environment

  • Hardware: PC
  • OS: Windows (local development environment)
  • Model/Provider: N/A (unit tests with mock providers)
  • Channels: Telegram (simulated unit tests), heartbeat service

📸 Evidence (Optional)

Click to view Logs/Screenshots

Passed:

  • go test ./pkg/agent -run TestProcessHeartbeat_DoesNotPublishToolFeedback -count=1
  • go test ./pkg/agent -run "TestProcessHeartbeat_DoesNotPublishToolFeedback|TestProcessMessage_PublishesToolFeedbackWhenEnabled|TestProcessMessage_PublishesReasoningContentToReasoningChannel" -count=1
  • go test ./pkg/heartbeat -count=1

Baseline reproduction:

  • The new heartbeat regression test fails on clean upstream/main before this fix because a 🔧 read_file feedback message is published to the outbound bus during heartbeat execution.

Additional notes:

  • go test ./pkg/agent -count=1 still fails due to pre-existing TestGlobalSkillFileContentChange; I confirmed that same failure on clean upstream/main, so it is unrelated to this PR.
  • codex review --base origin/main could not run in this environment because the local Codex config rejected approvals_reviewer = never.

☑️ Checklist

  • My code/docs follow the style of this project.
  • I have performed a self-review of my own changes.
  • I have updated the documentation accordingly.

@sipeed-bot sipeed-bot bot added type: bug Something isn't working domain: agent go Pull requests that update go code labels Mar 23, 2026
@xiwuqi
Copy link
Copy Markdown
Contributor Author

xiwuqi commented Mar 24, 2026

@lxowalle @Zhaoyikaiii The CI is green now. Could you please take a look at this agent fix when you have a chance? Thanks.

@xiwuqi xiwuqi force-pushed the wuxi/fix-heartbeat-tool-feedback branch from 7b23626 to f66d7d1 Compare March 24, 2026 15:46
@JokerQyou
Copy link
Copy Markdown

This is a must-have. Currently my agent channels are flooded with tool call messages from multiple heartbeat tasks.

@alexhoshina alexhoshina merged commit 85dfb34 into sipeed:main Mar 25, 2026
4 checks passed
samueltuyizere added a commit to samueltuyizere/picoclaw that referenced this pull request Mar 25, 2026
* update security migration documents

* feat(config): add command pattern detection tool in exec settings (sipeed#1971)

* Add command pattern testing endpoint and UI tool

Adds a new API endpoint `/api/config/test-command-patterns` that tests a
command against configured whitelist and blacklist patterns, along with
a frontend UI component to interactively test patterns.

* Only process deny patterns when enableDenyPatterns is true

* feat(models): add extra_body config field in model add/edit UI (sipeed#1969)

* Add extraBody field to model configuration forms

This adds a new field allowing users to specify additional JSON fields
to inject into the request body when configuring models.

* Handle ExtraBody clearing when frontend sends empty object

The backend now interprets an empty object sent from the frontend as a
signal to clear the ExtraBody field, while nil/undefined preserves the
existing value. Frontend changed to send {} instead of undefined when
the field is empty.

* refactor(web): clean up systray platform build files

Separate embedded tray icons into platform-specific files, rename the
no-cgo systray stub for consistency, and add the app version to the
launcher startup log.

* fix(agent): suppress heartbeat tool feedback (sipeed#1937)

* fix(lint): remove CGO_ENABLED=0 for lint and fix (sipeed#1989)

* fix(lint): remove CGO_ENABLED=0 for lint and fix

* fix makefile

* config: add baidu_search example to config.example.json (sipeed#1990)

Add Baidu Qianfan AI Search configuration block after glm_search,
matching the BaiduSearchConfig struct defaults (enabled: false,
max_results: 10).

Co-authored-by: BeaconCat <BeaconCat@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

* Fix security config precedence during migration (sipeed#1984)

* Fix security config precedence during migration

* add doc

* fix ci

* add baidu search

* feat(web): add WeCom QR binding flow to channel settings (sipeed#1994)

- add backend WeCom QR flow endpoints and in-memory flow state management
- add frontend WeCom binding UI with QR polling and channel enable toggle
- update channel config behavior and i18n strings for WeCom and WeChat
- apply minor formatting cleanup in model-related components

* chore(tui): add build target for picoclaw-launcher TUI and create README for TUI launcher (sipeed#1995)

* fix(build): disable Matrix gateway import on freebsd/arm

Exclude the Matrix gateway shim from freebsd/arm builds because
modernc.org/libc currently fails to compile on that target.
Document the upstream 32-bit FreeBSD codegen mismatch as well.

* fix(release): ignore nightly tags in goreleaser changelog (sipeed#1999)

GoReleaser was picking nightly tags as the "previous tag" when
generating changelogs, causing release changelogs to be incomplete.
Add git.ignore_tags to skip nightly tags.

* feat(tools): add exec tool enhancement with background execution and PTY support (sipeed#1752)

- Unified exec tool with actions: run/list/poll/read/write/send-keys/kill
- PTY support using creack/pty library
- Process session management with background execution
- Process group kill for cleaning up child processes
- Session cleanup: 30-minute TTL for old sessions
- Output buffer: 100MB limit with truncation

Actions:
- run: execute command (sync or background)
- list: list all sessions
- poll: check session status
- read: read session output
- write: send input to session stdin
- send-keys: send special keys (up, down, ctrl-c, enter, etc.)
- kill: terminate session

Tests:
- PTY: allowed commands, write/read, poll, kill, process group kill
- Non-PTY: background execution, list, read, write, poll, kill, process group kill
- Session management: add/get/remove/list/cleanup

* docs: update WeChat community QR code (sipeed#2003)

Co-authored-by: BeaconCat <BeaconCat@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

* feat(logger): add PICOCLAW_LOG_FILE env var for file-only logging

* Feature/add mimo provider (sipeed#1987)

* feat: add Xiaomi MiMo provider support

- Add 'mimo' protocol prefix support in factory_provider.go
- Add default API base URL for MiMo: https://api.xiaomimimo.com/v1
- Update provider-label.ts to include Xiaomi MiMo label
- Add MiMo to provider tables in both English and Chinese documentation
- Add comprehensive unit tests for MiMo provider

MiMo API is compatible with OpenAI API format, making it easy to integrate
with the existing HTTPProvider infrastructure.

Users can now use MiMo by configuring:
{
  "model_name": "mimo",
  "model": "mimo/mimo-v2-pro",
  "api_key": "your-mimo-api-key"
}

* hassas dosyaları kaldırma

* Add .security.yml and onboard to .gitignore

* build(deps): upgrade pty and reorganize sqlite dependencies (sipeed#2012)

- Upgrade github.com/creack/pty from v1.1.9 to v1.1.24
- Move github.com/mattn/go-sqlite3 to indirect dependency
- Move rsc.io/qr from indirect to direct dependency

* feat(channels): support multi-message sending via split marker (sipeed#2008)

* Add multi-message sending via split marker

* Add marker and length split integration tests

Tests that SplitByMarker and SplitMessage work together correctly, and
that code block boundaries are preserved during marker splitting.

* Simplify message chunking logic in channel worker

Extract splitByLength helper function and remove goto-based control
flow.
The logic now flows more naturally - try marker splitting first, then
fall
back to length-based splitting.

* Update multi-message output instructions in agent context

* Add split_on_marker to config defaults

* Add split_on_marker config option

* Rename 'Multi-Message Sending' setting to 'Chatty Mode'

* Add SplitOnMarker config option

---------

Co-authored-by: Cytown <cytown@gmail.com>
Co-authored-by: 柚子 <40852301+uiYzzi@users.noreply.github.com>
Co-authored-by: wenjie <meetwenjie@gmail.com>
Co-authored-by: daming大铭 <yinwm@outlook.com>
Co-authored-by: xiwuqi <64734786+xiwuqi@users.noreply.github.com>
Co-authored-by: taorye <taorye@outlook.com>
Co-authored-by: Luo Peng <luopeng.he@gmail.com>
Co-authored-by: BeaconCat <111232138+BeaconCat@users.noreply.github.com>
Co-authored-by: BeaconCat <BeaconCat@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: lxowalle <83055338+lxowalle@users.noreply.github.com>
Co-authored-by: Guoguo <16666742+imguoguo@users.noreply.github.com>
Co-authored-by: Liu Yuan <namei.unix@gmail.com>
Co-authored-by: 肆月 <2835601846@qq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

domain: agent go Pull requests that update go code type: bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Agent run heartbeat from example in HEARTBEAT.md

3 participants