Deduplicate thread context by ACP session delivery

[salman1993]

Summary

Buzz can repeat a thread reply in later prompts after the provider already received that event as context. The old fix treated every message signed by the agent key as retained by the current provider session. That could hide an unseen heartbeat reply because independent sessions share the same signing key.

This PR now removes only exact event IDs already delivered to the live ACP session. It uses bounded overfetch so removed events do not shrink the new-context window, and it preserves unseen same-key replies from other sessions.

The context header reports truncated=true whenever session history was omitted. The harness calls /count only when the relay query finds a sentinel beyond its fetch window.

This deliberately does not parse event IDs from shell output. A reply that the agent just published may appear once in the next context delta; after that event reaches the provider as context, later prompts omit it.

Related issue

N/A; reported from an exported Pi session.

Testing

No manual testing. CI covers this harness-only prompt-context change, including concurrent provider sessions that share one agent key.

Generated with Codex

[github-actions]

🔐 Codex Security Review

Status: review required for the current range.

The current range is 4cd82f513214aad11c2b742ce7cc7c681e8e32a0...1d6322fc45b090831085fb60b126aa6e6b2224e0.
A new review must complete for this exact range. When manual authorization
is required, a Block organization member must comment exactly
@buzz-security-review 1d6322fc45b090831085fb60b126aa6e6b2224e0 to authorize a new review.
Any previous review applies only to its recorded range.

[salman1993]

🤖 Automated review of the hydrated-thread-context change. Six findings, all centered on one invariant: hydrated_thread_roots is treated as proof that "the current provider session already contains this agent's replies for this thread", but several paths set it without establishing that. When the invariant is violated the agent silently loses its own prior replies from thread context — the exact failure the agent-reply pin exists to prevent (and hydration never expires within a session, so the loss persists until rotation/invalidation).

Inline comments below, roughly in severity order.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-14T14:18:29.907032Z	4f74327	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[salman1993]

Re-review at 4f74327 — all 6 prior findings resolved

Verified each finding from review 5191427869 against the current HEAD. cargo test -p buzz-acp --lib (939 passed) and cargo clippy -p buzz-acp --all-targets are clean.

Prior findings

#	Prior finding	Status
1	thread_roots_to_hydrate hydrated every thread in a merged batch	Fixed
2	Hydration committed above the context_message_limit > 0 gate and independent of fetch success	Fixed
3	handle_prompt_result hydrated regardless of result.outcome	Fixed
4	Steer hydration missing is_dm check + un-lowercased root	Fixed
5	conversation_context_has_author could never match (agent replies stripped upstream)	Fixed
6	total = fetched_total broke included == total when !truncated	Fixed (one narrow residual, below)

1. thread_roots_to_hydrate is gone, replaced by fetched_thread_root_to_hydrate (pool.rs:4033) which returns at most the single ContextTarget::Thread root that was actually fetched. A merged channel-policy batch no longer marks sibling/cancelled threads hydrated. merged_batch_hydrates_only_successfully_fetched_non_dm_target covers it, and channel_prompt_commits_delivery_state_only_after_acp_success now asserts hydrated_thread_roots.is_empty() for a top-level turn.

2. The insert moved below the fetch (pool.rs:2899) and is gated on Some(ConversationContext::Thread { .. }). limit == 0, a failed /query, and a timed-out /query all yield None → no hydration. Commit still happens only via record_scope_delivery_success on ACP success.

3./4. Resolved by deletion rather than repair: hydrated_thread_root is gone from SteerAckEvent and SuccessfulSteerDelivery, and record_successful_steer / mark_scope_delivery_success now pass []. Steering never hydrates, so the outcome-independence, DM, and case-normalization problems are all moot. lib.rs is back to a one-line signature change.

5. parse_nostr_thread_response_with_meta no longer does retain(!is_agent) upstream; agent replies survive into conversation_context_has_author, which now genuinely matches on a hydrated turn, so conversation_context_had_session_events becomes true and Complete is correctly suppressed. Confirmed end-to-end by hydrated_thread_prompt_omits_agent_reply_but_keeps_new_human_context, which asserts buzz messages thread appears on the wire.

6. truncated now folds in has_agent_replies and human_reply_count > limit for the hydrated path, so included == total holds whenever truncated == false. Also good: resolve_context_target now lowercases both root sources, so the ledger key and the lookup key agree and an uppercase e tag can't produce an unmatchable entry.

---

Residual / new observations (all low — none blocking)

a) pool.rs:4607 — has_agent_replies scans replies only, so an agent-authored thread root still slips past the truncated guard.

conversation_context_delta's excluded_author filter removes any agent-authored message including the root, but has_agent_replies is computed from reply_msgs, which excludes the root. Concrete case: an agent-posted announcement/workflow message becomes a thread root, humans reply in-thread (h <= limit, no agent replies, no relay truncation). Then truncated == false, total == 1 + h, but the delta drops the root → the prompt renders included="h" total="h+1" truncated="false" — the same invariant break finding 6 was about. Impact is cosmetic here: conversation_context_has_author does scan the root, so status is Included (not Complete) and the retrieval hint is still emitted. Folding the root's authorship into the truncated computation would close it.

b) pool.rs:4033 — the hydration ledger is per-root, but the evidence is per-window.

fetched_thread_root_to_hydrate marks the whole root hydrated even when that turn's fetch was itself truncated. Thread with 30 replies, limit = 12: turn 1 (pin = true) delivers only the newest 12, then hydrates the root. Turn 2 (pin = false) fetches the newest 25 and strips every agent reply in that window — including agent replies at positions 13–25 that this provider session never received. Those would have been rendered on main. Mitigated by truncated = true + accurate /count total + the buzz messages thread hint, so the agent can recover them, but the "the provider already retains those replies" premise is only true for the first turn's window. Worth a comment noting the narrower guarantee, or fencing hydration on truncated == false.

c) pool.rs:4316 — /count now fires on essentially every hydrated turn.

Because truncated is forced true whenever the window contains any agent reply, the best-effort /count (CONTEXT_COUNT_TIMEOUT, 500 ms) that previously ran only on genuinely truncated windows now runs on nearly every follow-up turn in a hydrated thread. Bounded and single-attempt, so it's a latency nit rather than a defect — just flagging that it's a new per-turn relay round-trip on the prompt hot path.

d) pool.rs:4601 vs pool.rs:4266 — reply_fetch_limit is computed twice with no shared source.

fetch_thread_context_with builds the relay filter with if pin { limit } else { limit * 2 } + 1, and parse_nostr_thread_response_with_meta independently recomputes the same expression to derive relay_window_truncated. They agree today; if either is tuned alone, relay_window_truncated silently reports the wrong answer (under-reporting truncation is the dangerous direction — it can restore a false Complete). Passing the effective fetch limit into the parser would make the coupling explicit.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4f74327654

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Mark omitted agent-authored roots as truncated

When a human continues a thread whose root was authored by the agent, hydrated-session filtering later removes that root, but has_agent_replies only examines reply_msgs. This branch can therefore leave truncated=false while included < total, causing the prompt to emit contradictory <thread-context> metadata despite omitting the root; include the root author in this truncation calculation and cover that case.

Useful? React with 👍 / 👎.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes requested

[P2] Do not treat a shared signing identity as proof of provider-session retention.

At pool.rs:3908-3914, hydration now excludes every matching-author message, including event IDs this provider session has never received or emitted. A successful turn only establishes that this session fetched one context window; it does not establish ownership of future messages signed by the same agent identity.

A supported counterexample is the built-in heartbeat: it has its own heartbeat_session (pool.rs:2574-2601), and its default instructions explicitly allow answering actionable feed items with buzz messages send --reply-to (lib.rs:5326-5341). The heartbeat and channel sessions use the same signing identity, but do not share provider conversation history.

Source-traced reproduction:

1. With heartbeats enabled, complete a successful turn in a channel thread so its root is hydrated.
2. Have the separate heartbeat session publish a reply into that thread, then let it finish without rotating the original channel session.
3. Send a human follow-up into the original thread. Keep the new heartbeat reply inside the fetched window.

The self-event admission gate drops that reply as a trigger (lib.rs:3342-3344), and this new author filter also removes it from relay context on the human follow-up (pool.rs:2917-2931). The target session therefore never receives a reply it did not retain. It can repeat work or contradict the answer the human just saw. On the base commit, the unseen event survives delivery-ID deduplication.

Please retain replies whose ownership is unknown; suppress only messages demonstrably delivered to or emitted by this exact provider session, rather than all messages with its pubkey. Add a two-provider-session regression where the same key signs a reply outside the hydrated target session and verify that reply reaches the target's next prompt. Existing successful-turn/reset and same-session deduplication behavior should remain intact.

Scope and validation

Reviewed exact head 4f743276541995cc570ed7f0a02e34c31cb7253a against base 6c35e82bd50f4ad6587554eeb429e7378d474ba7. The compatibility contract is to remove redundant context without discarding unseen conversation history. Integrated independent lifecycle and selection/presentation reviews across channel/thread policy, DM exclusion, merged batches, steering, failure/cancellation, rotation/restart, author normalization, and bounded fetching. No additional material blocker found in those paths. This is a harness-only change; desktop/mobile rendering and wire schemas were not changed or exercised.

Source/metadata-only review on Wes's Mac Studio. No checkout, PR-code execution, build, or tests were run; the reproduction above is established by source tracing, not a runtime test.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Changes requested

The previous shared-key/session-retention blocker is resolved: exact delivered event IDs now determine omission, so an unseen reply from a separate heartbeat/provider session is no longer suppressed merely because it uses the same signing key.

[P2] Keep trigger-only deduplication from declaring complete history truncated.

At pool.rs:3881-3907, omitted combines previously delivered IDs with the current batch’s trigger IDs, and either reason now sets truncated=true (also in the DM branch at lines 3915-3919). Trigger IDs are not missing history: they are rendered separately in this same prompt.

Source-traced reproduction:

1. Start a fresh session on a two-message thread: root R and triggering reply T, with the context limit above two and no previously delivered IDs. /query returns R and T, a complete window.
2. run_prompt_task puts T in rendered_batch_ids and passes that set to conversation_context_delta (pool.rs:2888-2925). The delta removes T, retains R, and now sets truncated=true even though both messages reach the provider.
3. conversation_context_status therefore returns Included, not Complete (queue.rs:1781-1800), and the prompt says to run buzz messages thread for full history if truncated (queue.rs:1674-1681). A fresh DM with one earlier message and its triggering message follows the equivalent buzz messages get path (queue.rs:1634-1645).

This regresses the complete-context hint on ordinary fresh conversations and encourages redundant retrieval of the very context this change aims to deduplicate. Before this change, trigger-only filtering preserved the complete-window flag.

Please distinguish trigger-only removal from omitted prior-session history when setting this flag. Preserve the intentional truncated=true signaling for omitted previously delivered history and real window truncation; no broader metadata redesign is required. Add fresh thread-reply and DM regression cases through fetch/delta/formatting with an empty delivery ledger. The existing queue completeness test bypasses delta, while the new delta assertion mixes both omission reasons.

Scope and validation

Reviewed head e83c285b442346bec1b646dc81f92f0817eed513 against base 6c35e82bd50f4ad6587554eeb429e7378d474ba7, with focused comparison to the previously reviewed head 4f743276541995cc570ed7f0a02e34c31cb7253a. The product contract is session-local context deduplication without dropping unseen conversation history or falsely soliciting a full-history fetch.

Integrated independent delivery-lifecycle and presentation reviews with my fetch-selection/relay-contract trace: channel/thread policy, DMs, shared-key sessions, steering, failed/cancelled turns, reset/rotation, bounded overfetch, pinning, and sentinel/count handling. No additional material blocker found in those reviewed paths. Desktop/mobile UI and wire formats are unchanged and were not exercised.

Source/metadata-only review on Wes’s Mac Studio; no checkout, build, tests, or PR-code execution. Reproduction above is source-traced, not runtime-verified. CI was not assessed: the installed Agent Tools availability check did not expose the required CI-results command.

[wesbillman]

Carl, an automated reviewer, commenting via Wes’s GitHub account.

Re-review: no remaining code blockers found

Reviewed head 1d6322fc45b090831085fb60b126aa6e6b2224e0 against base 4cd82f513214aad11c2b742ce7cc7c681e8e32a0, including the repair since e83c285b442346bec1b646dc81f92f0817eed513 and the current integration.

• Both earlier findings are resolved. Context deduplication uses exact live-session delivery IDs, preserving unseen replies from a different session sharing the agent key. Trigger-only removal no longer marks a fresh complete thread or DM as truncated; the trigger is rendered separately in the same prompt. Genuine window truncation and intentional prior-session omission still retain their history hints.
• Lifecycle and regression coverage: delivery IDs/hydration commit on successful turns and clear with session replacement/invalidation; steering receipts remain session-ID-fenced. The added fresh-thread/DM regressions cross the production fetch/parser, delta and formatter seams. The CI timeout and lockfile changes introduce no additional blocker found in this review.
• Validation and limits: hosted CI run 34888913752 completed successfully for this exact head, including Rust unit tests/lint and integration lanes. This was a source-and-hosted-CI review; no local suite or live provider/relay workflow was executed.

This comment supersedes the code findings in my earlier changes-requested reviews, but is not a GitHub approval. The earlier blocking review state may still require approval/dismissal, and the security bot still reports that an exact-range security review is required. Those are separate from this no-code-blockers verdict.