Skip to content

fix(agent): stop leaking provider retry heartbeats to user channels - #3229

Merged
Re-bin merged 3 commits into
HKUDS:mainfrom
SamZhu19921116:fix-error-message-propagation-bug
Apr 18, 2026
Merged

Re-bin merged 3 commits into
HKUDS:mainfrom
SamZhu19921116:fix-error-message-propagation-bug

Conversation

@SamZhu19921116

@SamZhu19921116 SamZhu19921116 commented Apr 17, 2026 •

Copy link
Copy Markdown
Contributor
  1. 当 LLM 服务出现临时性错误(如网络波动、超时、429限流等)时, base.py 中的 _run_with_retry 方法会启动重试机制。
  2. 在重试等待期间, _sleep_with_heartbeat 方法会周期性调用 on_retry_wait 回调函数,发送类似 'Model request failed, retry in 1s (attempt 1)' 的心跳消息。
  3. 之前 on_retry_wait 参数被错误地绑定到 _bus_progress ,导致这些内部诊断消息被当作普通进度消息发送到飞书客户端。
  4. manager.py 的消息分发器没有过滤这类重试心跳消息。

修复方案:

  1. loop.py - 新增重试等待回调- 新增独立的 _on_retry_wait 回调函数,为重试消息添加 _retry_wait: True 元数据标识- 在 AgentRunSpec 中传入 retry_wait_callback 参数。
  2. runner.py - 支持重试回调参数- 在 AgentRunSpec 数据类中新增 retry_wait_callback 字段- 在 _build_request_kwargs 中将 on_retry_wait 参数从 progress_callback 改为 retry_wait_callback。
  3. manager.py - 过滤重试心跳消息- 在 _dispatch_outbound 方法中新增过滤逻辑,丢弃所有带 _retry_wait 标识的消息,确保重试心跳不会发送到任何客户端。

…ith_retry 方法会启动重试机制。2. 在重试等待期间, _sleep_with_heartbeat 方法会周期性调用 on_retry_wait 回调函数,发送类似 'Model request failed, retry in 1s (attempt 1)' 的心跳消息。3. 之前 on_retry_wait 参数被错误地绑定到 _bus_progress ,导致这些内部诊断消息被当作普通进度消息发送到飞书客户端。4. manager.py 的消息分发器没有过滤这类重试心跳消息。 修复方案:1. loop.py - 新增重试等待回调- 新增独立的 _on_retry_wait 回调函数,为重试消息添加 _retry_wait: True 元数据标识- 在 AgentRunSpec 中传入 retry_wait_callback 参数。2. runner.py - 支持重试回调参数- 在 AgentRunSpec 数据类中新增 retry_wait_callback 字段- 在 _build_request_kwargs 中将 on_retry_wait 参数从 progress_callback 改为 retry_wait_callback。3. manager.py - 过滤重试心跳消息- 在 _dispatch_outbound 方法中新增过滤逻辑,丢弃所有带 _retry_wait 标识的消息,确保重试心跳不会发送到任何客户端。
@chengyongru chengyongru changed the title fix: 错误消息流转路径:1. 当 LLM 服务出现临时性错误(如网络波动、超时、429限流等)时, base.py 中的 _run_w… fix: 错误消息流转路径:1. 当 LLM 服务出现临时性错误(如网络波动、超时、429限流等)时, base.py 中的 _run_with_retry Apr 17, 2026
@chengyongru chengyongru changed the title fix: 错误消息流转路径:1. 当 LLM 服务出现临时性错误(如网络波动、超时、429限流等)时, base.py 中的 _run_with_retry fix: 错误消息流转路径 Apr 17, 2026
Re-bin added 2 commits April 18, 2026 05:42
… channels

Add two focused regression tests for the retry-wait leak this PR fixes:

- tests/agent/test_runner.py::test_runner_binds_on_retry_wait_to_retry_callback_not_progress
  locks in that `AgentRunSpec.retry_wait_callback` (not `progress_callback`) is
  what `_build_request_kwargs` forwards to the provider as `on_retry_wait`.

- tests/channels/test_channel_manager_delta_coalescing.py::TestRetryWaitFiltering
  runs `_dispatch_outbound` end-to-end and asserts that `_retry_wait: True`
  messages never reach channel send.

Both tests fail on origin/main and pass with this PR's fix applied.

Made-with: Cursor

@Re-bin Re-bin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the right kind of fix.

The regression comes from fbedf7ad ("feat: harden agent runtime for long-running tasks"), where AgentRunSpec.progress_callback ended up wired into the provider's on_retry_wait parameter. That meant every transient-retry heartbeat ("Model request failed, retry in 1s (attempt 1).") got published as a normal progress message and delivered to end-user channels — exactly the kind of internal diagnostic a user should never see.

The fix splits the two signals cleanly:

  • AgentRunSpec grows a dedicated retry_wait_callback, and _build_request_kwargs forwards on_retry_wait to it instead of progress_callback.
  • AgentLoop._process_message builds a distinct _on_retry_wait closure that tags outbound messages with _retry_wait: True.
  • ChannelManager._dispatch_outbound drops any message carrying that tag before it reaches a channel.

I like that retry-wait events still enter the bus instead of being silently dropped at source — future diagnostics/metrics subscribers can observe them without any further plumbing, and only the end-user delivery path is gated.

What the PR was missing was a regression test; given this is exactly the "silently re-bindable callback" class of bug, I pushed two focused ones on top of the existing changes:

  • tests/agent/test_runner.py::test_runner_binds_on_retry_wait_to_retry_callback_not_progress
    pins _build_request_kwargs to forward retry_wait_callback (not progress_callback) as on_retry_wait.
  • tests/channels/test_channel_manager_delta_coalescing.py::TestRetryWaitFiltering::test_retry_wait_message_dropped
    exercises _dispatch_outbound end-to-end and asserts _retry_wait: True messages never reach channel.send.

Both tests fail on origin/main and pass with this PR's fix. Full suite is green locally (2032 passed) and CI is already green across 3.11/3.12/3.13.

From my side this is clean, narrow, and ready to merge.

@Re-bin Re-bin changed the title fix: 错误消息流转路径 fix(agent): stop leaking provider retry heartbeats to user channels Apr 18, 2026
@Re-bin
Re-bin merged commit 70a1279 into HKUDS:main Apr 18, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants