Skip to content

fix(integrations): stop posting the model's closing text after band_no_reply or band_send_message - #725

Merged
AlexanderZ-Band merged 6 commits into
mainfrom
fix/acp-tool-settled-reply
Oct 6, 2026
Merged

AlexanderZ-Band merged 6 commits into
mainfrom
fix/acp-tool-settled-reply

Conversation

@bandzalkin

@bandzalkin bandzalkin commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Problem

RoomTurnEmitter relays the model's closing text unless turn_replied_in_room() sees a completed band_send_message/band_no_reply in the ACP stream — matched by the tool_call title (ACPToolCall.from_acp reads title first).

On OMP that never matches:

  • OMP fills the title with the model's i intent phrase (buildToolTitle in modes/acp/acp-event-mapper.ts), e.g. "Ending the turn silently".
  • Without an intent, OMP names MCP tools mcp__band_<tool>, a spelling _resolve_mcp_tool_name does not accept.

So every OMP turn's closing narration was posted to the room as a reply to the sender being answered. Observed live (band-agent, SDK 3.3.0): two OMP agents looped for ~10 minutes, one posting "X's message asks nothing new of me, so I ended the turn without replying. Task 2 is at …" every ~10 s — the runner log shows no reply this turn at the same timestamps — each post starting the other agent's next turn. Real band_send_message calls were also duplicated by their narration ("I told the architect…").

Fix

  • engine.py / backends.py: the http/sse local MCP backend now honors tool_result_hook (the Claude backend already had one; the http path silently dropped it). The hook fires only after a successful built-in or custom call, with (tool_name, chat_id, result).
  • client_adapter.py: keeps the active RoomTurnEmitter per room and passes a hook that calls mark_reply_settled() when settles_turn_reply(tool_name) — the canonical name is known in-process regardless of what the ACP title says.
  • room_emitter.py: __aexit__ relays held text only if neither the in-process mark nor the stream-based detection settled the reply. The stream path stays for remote band-mcp servers the SDK never executes.

Tests

tests/integrations/acp/test_client_adapter_behavior.py drives the real local MCP server through the fake ACP agent with a title that differs from the tool name (will_call_mcp_tool(..., title=)):

  • intent-titled band_no_reply → closing text not relayed (red before: ["Huginn's me…ed the turn."] == [])
  • intent-titled band_send_message → posts once, no text duplicate (red before)
  • intent-titled observing tool (band_get_participants) → text still relayed

tests/mcp/test_engine.py: hook observes successful built-in and custom calls with the room id; skipped when the call fails.

pytest tests/mcp/test_engine.py tests/integrations/acp tests/integrations/test_mcp_backends.py tests/integrations/mcp: 476 passed, 10 skipped. pre-commit (ruff, pyrefly, secrets) clean.

Not in this PR

_resolve_mcp_tool_name still rejects mcp__band_<tool>; irrelevant once execution-time settlement exists, and the narrated tool_call event names on OMP remain the model's intent phrase (cosmetic).

Testing

  • Unit tests pass (uv run pytest tests/ --ignore=tests/integration/): 6289 passed, 658 skipped
  • Pre-commit checks pass (ruff check/format, pyrefly, secrets) on the changed files
  • Integration tests: not run (no live ACP harness in this environment); the behavior tests above exercise the real local MCP server over a real ACP wire against a fake agent

Checklist

  • PR title follows Conventional Commits format
  • Code follows project style guidelines
  • Tests added/updated as needed
  • Documentation updated as needed (docstrings on turn_replied_in_room, mark_reply_settled, ToolResultHook; changelog is release-please generated from the commit)

@bandzalkin
bandzalkin requested a review from a team October 2, 2026 16:59
@bandzalkin
bandzalkin force-pushed the fix/acp-tool-settled-reply branch from d8b08f6 to 78de6a7 Compare October 2, 2026 17:00
@bandzalkin bandzalkin changed the title fix(acp): settle the turn when a Band tool runs in-process, not only by ACP title fix(acp): stop posting the model's closing text after band_no_reply or band_send_message Oct 2, 2026
…o_reply or band_send_message

The ACP bridge decided whether to relay the model's closing text by
reading Band tool names off the ACP tool_call title. OMP writes the
model's intent phrase into that title ("Ending the turn silently"), and
names MCP tools mcp__band_<tool>, which the resolver does not accept
either; so on OMP no band_no_reply or band_send_message was ever
recognized and every turn's closing narration was relayed to the room as
a reply to the sender being answered. In a live room that produced a
ten-minute loop between two agents, each one's "I ended the turn without
replying…" text starting the other's next turn.

The http/sse local MCP backend now honors the tool_result_hook the Claude
backend already had: the ACP adapter passes a hook that marks the room's
active RoomTurnEmitter as reply-settled when a reply-settling tool
completes in-process, and the emitter honors either that mark or the
stream-based detection. The hook runs only after a successful call, so a
failed post still falls back to text. The stream-based path stays for
remote band-mcp servers the SDK never executes.

Tests drive the real local MCP server through the fake ACP agent with a
title that differs from the tool name: band_no_reply suppresses the
closing text, band_send_message posts once without a text duplicate, and
an observing tool leaves the text reply flowing.
@bandzalkin
bandzalkin force-pushed the fix/acp-tool-settled-reply branch from 78de6a7 to bbb887e Compare October 2, 2026 17:00
@bandzalkin bandzalkin changed the title fix(acp): stop posting the model's closing text after band_no_reply or band_send_message fix(integrations): stop posting the model's closing text after band_no_reply or band_send_message Oct 2, 2026
@AlexanderZ-Band
AlexanderZ-Band added this pull request to the merge queue Oct 6, 2026
Merged via the queue into main with commit 3b00dcc Oct 6, 2026
28 checks passed
@AlexanderZ-Band
AlexanderZ-Band deleted the fix/acp-tool-settled-reply branch October 6, 2026 11:02
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