fix(buzz-acp): state the message-delivery contract on every turn - #6296
Open
shawnhank wants to merge 2 commits into
Open
fix(buzz-acp): state the message-delivery contract on every turn#6296shawnhank wants to merge 2 commits into
shawnhank wants to merge 2 commits into
Conversation
A DM always has at least one other participant besides the sender. _fetchDmRecipientPubkeys() falls back from the live membership query to the channel metadata's participantPubkeys when the query fails — but if both come back empty at once (e.g. a relay reconnect in progress, which happens often on mobile after backgrounding), the code silently concluded the DM had zero other recipients and sent with no p tags at all. Confirmed live: a plain top-level DM message went out with zero notification tags while the relay's own member list for that channel was correct. Both sources empty is never a legitimate answer for a real DM, so treat it as data not being available yet rather than as a confirmed empty roster: retry the membership fetch up to 3 times with a short delay, bounded so a genuinely persistent failure doesn't hang the send. Added tests covering both the recovery case and the bounded-give-up case. Signed-off-by: Shawn Hank <shawn@shawnhank.com>
Plain session text was never auto-published to a Buzz channel, but nothing in the turn prompt told the model that -- only append_reply_instruction covered threading, and only when a reply anchor was resolved. A turn could complete cleanly (tokens billed, no error logged) while the model's reply sat in session text and never reached the channel, because it never called `buzz messages send`. Strong/default models tend to infer the convention from the surrounding CLI-oriented context; others don't. Add append_delivery_contract(), called unconditionally for every batch in format_prompt(), stating explicitly that plain text isn't delivered and an explicit `buzz messages send` call is required. This is the cheap, model-agnostic mitigation suggested as option 1 in the bug report -- a harness-level fallback (option 2, auto-post or WARN-log when a turn ends without a publish call) is a larger follow-up left for a separate PR. Fixes block#2698 Signed-off-by: Shawn Hank <shawn@shawnhank.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #2698. Plain session text was never auto-published to a Buzz channel, but nothing in the turn prompt told the model that.
append_reply_instructioncovers threading (--reply-to), but only fires when a reply anchor is resolved — there was no unconditional statement that a reply must go throughbuzz messages sendto be delivered at all.Symptom (matches the filed issue exactly): a turn completes cleanly — tokens billed, no error logged, Activity/session panel shows a full generated reply — but the channel stays empty, because the model never called
buzz messages send. Strong/default models tend to infer the convention from the surrounding CLI-oriented context; others (including local/self-hosted models via a custom OpenAI-compatible provider) don't.Change
Adds
append_delivery_contract(), called unconditionally once per batch informat_prompt(), stating explicitly: plain text output is not delivered, an explicitbuzz messages sendcall is required for the user to see a reply.This is the cheap, model-agnostic mitigation described as option 1 in #2698 ("state the contract explicitly in the turn prompt ... verified locally: adding the equivalent instruction to the agent's system prompt makes Sonnet 5 post correctly"). Option 2 in that issue (a harness-level fallback — auto-post the final ACP text, or at minimum WARN-log, when a turn ends without a publish call for the triggering channel) is a larger change with different tradeoffs and is left for a separate follow-up PR rather than bundled here.
Test plan
cargo test -p buzz-acp queue::— 129 passed (128 existing + 1 new), 0 failedtest_format_prompt_always_states_delivery_contractasserts the contract string is present informat_promptoutput for a plain top-level channel eventcargo fmt -p buzz-acp— cleancargo clippy -p buzz-acp --all-targets— no warnings🤖 Reported via a live incident while debugging with the repo owner; see linked issue for full repro context.