Skip to content

fix(buzz-acp): exactly-once fallback publish for agents that don't self-publish - #5579

Open
FvanW wants to merge 6 commits into
block:mainfrom
FvanW:fix/hermes-publish-tap
Open

fix(buzz-acp): exactly-once fallback publish for agents that don't self-publish#5579
FvanW wants to merge 6 commits into
block:mainfrom
FvanW:fix/hermes-publish-tap

Conversation

@FvanW

@FvanW FvanW commented Aug 11, 2026

Copy link
Copy Markdown

Summary

buzz-acp harnesses require the wrapped ACP agent to self-publish its reply
via a buzz messages send tool call — the harness itself never
auto-publishes (per base_prompt.md: "your reasoning is invisible... exists
only if you published it"). Well-behaved backends do this reliably, but a
less rigorous backend can intermittently end its turn with a full text
answer via agent_message_chunk updates while never emitting the publish
tool call, silently dropping the reply.

This adds an opt-in, default-off --publish-final-if-unsent flag
(BUZZ_ACP_PUBLISH_FINAL_IF_UNSENT). When armed for a given backend:

  • Buffers agent_message_chunk text per turn (bounded at 64 KiB, matching
    buzz_sdk::builders::build_message's own content-size limit; only
    buffered at all when the flag is armed, so this is a zero-cost no-op for
    backends that never set it).
  • Tracks whether the turn's tool calls included an actually-successful
    buzz messages send (checks rawOutput.isError, the same success/failure
    convention this repo's own buzz-agent reference implementation already
    uses — not just tool-call lifecycle status) to a matching --channel.
  • On a clean EndTurn (never Refusal/MaxTokens/MaxTurnRequests/Cancelled —
    those are distinct terminal outcomes this must not paper over) for a
    channel-sourced turn, if no successful publish was detected and the
    buffered text is non-blank, publishes it via the same in-process
    signing/submit path post_failure_notice already uses, threaded to the
    turn's own --reply-to anchor (now shared with format_prompt via one
    extracted helper instead of two hand-synced copies).

This is a best-effort fallback with duplicate suppression on a heuristic
detection signal, not a delivery guarantee — documented as such rather than
"exactly-once."

Left default-off, and currently only enabled for one backend in our own
deployment config — arming it for a backend that already self-publishes
reliably would risk a double-post, so this should stay opt-in per-backend.

Testing

  • cargo fmt --all -- --check clean.
  • cargo build --release -p buzz-acp succeeds.
  • cargo test -p buzz-acp: 736 tests pass, 0 failures, including ~40 new
    tests covering: clean-EndTurn-fires, detected-publish-suppresses (both
    success- and failure-shaped tool results), every non-EndTurn stop reason
    excluded, heartbeat-sourced turns excluded, blank-text excluded, buffer
    armed/disarmed and capped correctly, heuristic positive/adversarial/
    wrong-channel cases, and reply-anchor threading (DM/top-level/threaded/
    agent-to-agent) — the last of which also confirms the anchor-resolution
    refactor preserves format_prompt's pre-existing test suite unchanged.
  • Live-verified against a running relay: 3 consecutive turns each produced
    exactly one correctly-threaded reply, including one turn with a genuine
    mid-turn steer signal that still resulted in exactly one reply with no
    duplicate. The fallback path itself has not yet been observed firing live
    (the bug it targets is intermittent) — the self-publish path was
    exercised, not yet the fallback path itself in production.

Known residual limitations (documented in code)

  • The rawOutput.isError success signal is only guaranteed for
    buzz-agent-shaped tool-call payloads; other backends may report
    status: "completed" with no rawOutput, in which case this falls back
    to trusting lifecycle status alone (pre-existing behavior) rather than
    guessing.
  • The self-publish detector is a token/substring scan of the tool call's
    structured command field, not a real shell parser — a command built at
    runtime or hidden behind a wrapper script can still be missed.

Related issue

None found — searched open issues/PRs for related discussion, didn't find one.

FvanW added 6 commits August 11, 2026 11:27
@FvanW
fix(buzz-acp): exactly-once fallback publish for agents that don't se…
550c339 
…lf-publish

buzz-acp harnesses require the wrapped ACP agent to self-publish its
reply via a `buzz messages send` tool call — the harness itself never
auto-publishes. Well-behaved backends (claude-agent-acp, codex-acp) do
this reliably. Less rigorous backends (e.g. a custom `hermes` ACP
command) can intermittently end a turn with a full text answer via
`agent_message_chunk` updates but never emit the publish tool call,
silently dropping the reply.

Adds an opt-in, default-off `--publish-final-if-unsent` flag
(env `BUZZ_ACP_PUBLISH_FINAL_IF_UNSENT`). When enabled for a backend:

- Buffers `agent_message_chunk` text per turn (previously logged and
  discarded).
- Tracks whether the turn's tool calls included an accepted
  `buzz messages send` invocation (best-effort title/rawInput match —
  there's no typed ACP tool-call schema or dedicated "published to
  channel" signal in this crate to key off instead).
- On a clean `EndTurn` (never Refusal/MaxTokens/MaxTurnRequests/
  Cancelled — those are distinct terminal outcomes this must not
  paper over) for a channel-sourced turn, if no publish was detected
  and the buffered text is non-blank, publishes it via the same
  in-process signing/submit path `post_failure_notice` already uses,
  threaded to the same `--reply-to` anchor the turn's own prompt was
  given.

Exactly-once by construction: the buffer is reset at the top of every
turn, and the fallback never fires when a publish was already
detected, so a backend that already self-publishes reliably is
unaffected even with the flag on. Left default-off and unset for
claude-agent-acp/codex-acp to avoid any behavior change for backends
that already work correctly.

Verified live against a running Trinity relay: 3/3 consecutive turns
produced exactly one correctly-threaded reply each, including one
genuine mid-turn steer signal that still resulted in exactly one
reply with no duplicate.

Signed-off-by: Frederick <fred.vanwagenen@gmail.com>
@FvanW
style(buzz-acp): cargo fmt --all
50fdf98 
Formatter reflow only, no logic change. Run first so subsequent diffs
in this branch's follow-up work aren't fighting formatter noise.

Signed-off-by: Frederick <fred.vanwagenen@gmail.com>
@FvanW
fix(buzz-acp): harden publish-final-if-unsent detection and buffering
fdf15ad 
Three related fixes to the fallback-publish gate from e85996a, found in
team review (Hermes + Codex):

1. Publish detection only checked tool-call lifecycle status
   ("completed"), never whether the underlying `buzz messages send`
   actually succeeded. A tool call that completes its ACP lifecycle
   while reporting a command/relay error would still suppress the
   fallback, silently recreating the exact silent-drop bug this
   feature exists to prevent. Adds `tool_call_update_succeeded`, which
   reuses this codebase's own success/failure convention
   (`buzz-agent::agent::{emit_completed,emit_failed}`:
   `rawOutput.isError: true` marks a "completed" lifecycle status as
   an actual failure). Best-effort: only `buzz-agent` is guaranteed to
   follow this shape, so an absent `rawOutput` still falls back to
   trusting `status: "completed"` alone, since treating an unknown
   shape as failure would make the gate MORE likely to double-publish.

2. `final_text_buffer` grew unconditionally on every backend
   (claude-agent-acp, codex-acp, hermes) for every `agent_message_chunk`,
   even though the feature itself is default-off and only enabled for
   `hermes`. Threads `Config::publish_final_if_unsent` into `AcpClient`
   via a new `set_publish_final_if_unsent` setter (called once at spawn
   time, alongside the existing `set_observer` pattern) so the buffer
   append is skipped entirely — not even a no-op push — for every
   disarmed client. Also caps the buffer at 64 KiB
   (`FALLBACK_PUBLISH_MAX_BYTES`), matching the max message content size
   `buzz_sdk::builders::build_message` itself enforces — buffering past
   that point is pointless even before considering memory growth, since
   a larger fallback publish would be rejected by that validation and
   never send anyway. Truncates on a UTF-8 char boundary rather than
   splitting a multi-byte sequence.

3. `looks_like_buzz_messages_send`'s "all three words anywhere in any
   order" fallback branch was broad enough for an unrelated tool call
   (a search or file op whose title/args happened to mention "buzz",
   "messages", and "send") to false-positive and wrongly suppress a
   reply that was never published. Tightened to prefer the structured
   `rawInput.command` field (the shape a real shell-tool invocation
   uses, and the same field `buzz-agent::agent::is_reply_shaped`
   already keys off for an analogous problem) and require an ordered,
   adjacent "messages send" substring with "buzz" appearing before it,
   rather than three words anywhere. Also cross-checks a `--channel
   <id>` argument, when present, against the channel this turn is
   processing (threaded in via a new `set_pending_channel_id`,
   mirroring `set_pending_reply_anchor`) — a detected send to a
   DIFFERENT channel no longer credits this turn's publish detection.

No behavior change for the default-off path or for claude-agent-acp/
codex-acp, which never set publish_final_if_unsent.

Signed-off-by: Frederick <fred.vanwagenen@gmail.com>
@FvanW
refactor(buzz-acp): share reply-anchor resolution between format_prom…
7f40cf3 
…pt and reply_anchor_for_batch

reply_anchor_for_batch's own doc comment flagged itself as a
hand-maintained duplicate of the anchor computation format_prompt
performs internally for its [Context] reply instruction — real
maintenance debt per both team-review reviewers (Hermes + Codex): two
implementations of the same logic will drift.

Extracts resolve_turn_reply_anchor as the single shared implementation
of the DM/non-DM anchor rule (DM: anchor only when replying in an
existing thread; non-DM: delegate to resolve_reply_anchor, which
anchors human-facing turns to the thread root or triggering event and
leaves agent-to-agent turns unanchored). Both format_prompt and
reply_anchor_for_batch now call it instead of hand-syncing copies.

format_prompt's existing test suite (112 tests in queue::tests,
including test_format_prompt_dm_scope, test_format_prompt_thread_scope,
test_human_thread_reply_anchors_to_root_not_triggering_or_parent, and
the full test_reply_instruction_* family) passes unchanged after the
refactor — confirms the extraction preserves format_prompt's existing
behavior exactly.

Signed-off-by: Frederick <fred.vanwagenen@gmail.com>
@FvanW
test(buzz-acp): behavioral coverage for the publish-final-if-unsent gate
7041060 
No test previously exercised the fallback-publish gate's actual logic —
the original commit only threaded publish_final_if_unsent: false through
existing fixtures. Adds coverage for every case both team-review
reviewers enumerated:

lib.rs (fallback_publish_target_tests, testing the newly-extracted pure
predicate `fallback_publish_target`):
  - fires_on_armed_end_turn_channel_with_pending_text: the happy path
  - does_not_fire_when_disarmed
  - does_not_fire_on_non_end_turn_stop_reasons: Refusal/MaxTokens/
    MaxTurnRequests/Cancelled, table-driven over all four
  - does_not_fire_for_heartbeat_source
  - does_not_fire_when_no_pending_final_text (covers both "already
    published" and "blank buffer", which collapse to None upstream)
  - threads_the_reply_anchor_through_unchanged / no_anchor_still_fires_with_none_anchor

acp.rs (tool_call_update_succeeded — item 2):
  - completed+no rawOutput → success (preserves old behavior)
  - completed+isError:false → success, completed+isError:true → failure
  - status:failed → failure regardless of rawOutput
  - in_progress / missing status → not success

acp.rs (extract_channel_flag, looks_like_buzz_messages_send — item 4):
  - matches buzz-agent's own documented send shapes (shared fixture
    with crates/buzz-agent/src/agent.rs's reply_shape_matches_documented_send_forms)
  - title fallback still matches
  - adversarial decoys (words present but out of order / not adjacent /
    no buzz prefix) do NOT match — the false-positive this item fixes
  - --channel cross-check: matching channel credits detection,
    mismatched channel does not, absent --channel still credits

acp.rs (buffer/detection integration, via spawn_inert_client +
handle_session_update — items 2/3/4 working together):
  - disarmed client never buffers agent_message_chunk text
  - armed client accumulates across chunks
  - whitespace-only buffered text does not count as pending
  - buffer stops exactly at FALLBACK_PUBLISH_MAX_BYTES, preserving
    content up to the cap rather than dropping it
  - a detected send (via tool_call, and via tool_call → tool_call_update)
    suppresses the pending text
  - a "completed" tool_call reporting rawOutput.isError:true does NOT
    suppress the fallback
  - a send to a DIFFERENT channel does NOT suppress this turn's fallback
  - pending text carries the reply anchor set via set_pending_reply_anchor

queue.rs (resolve_turn_reply_anchor, reply_anchor_for_batch — item 6):
  - DM reply vs. DM non-reply
  - non-DM top-level human, threaded human, and agent-to-agent cases,
    each cross-checked against resolve_reply_anchor's existing behavior
  - reply_anchor_for_batch agrees with the anchor format_prompt embeds
    in its own [Context] reply instruction for the identical batch —
    the actual guarantee item 6's extraction is meant to provide

727 tests pass (686 baseline + this branch's prior fixes' assertions +
these new tests), including all pre-existing tests unchanged. The one
pre-existing flaky test (keepalive_resets_idle_past_deadline, on the
documented flaky list) fails intermittently under load, unrelated to
these changes.

Signed-off-by: Frederick <fred.vanwagenen@gmail.com>
@FvanW
docs(buzz-acp): stop calling the fallback-publish gate "exactly-once"
8955a1b 
Codex flagged this in team review: the gate is fire-and-forget with no
retry, and publish detection is heuristic (title/rawInput substring
matching), not a protocol-level guarantee. "Exactly-once" overstates
what it actually provides. Retitles the doc comments in config.rs
(CliArgs::publish_final_if_unsent, Config::publish_final_if_unsent) and
the call site in lib.rs's handle_prompt_result as "best-effort... with
duplicate suppression on a best-effort detection signal" instead, and
spells out the two failure modes (miss a real publish → possible
duplicate; false-positive a detection → a genuinely dropped reply stays
dropped) that "exactly-once" was papering over. No behavior change.

Signed-off-by: Frederick <fred.vanwagenen@gmail.com>
@FvanW
FvanW requested a review from a team as a code owner August 11, 2026 16:28
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

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@FvanW