fix(acp): follow threads the agent participates in (#2270)

[brocoppler]

Summary

A mention-gated agent (SubscribeMode::Mentions, the default) goes deaf in threads it has joined: once it replies, untagged follow-ups like "yes, go ahead" are never seen unless the human re-tags the agent on every message. The gate exists at two levels — the relay REQ's #p clause never delivers the reply, and match_event's mention re-check would drop it anyway.

Fix: participation opens a thread; inactivity closes it.

• thread_follow.rs (new): per-channel set of followed NIP-10 thread roots with a sliding inactivity TTL (default 24h, --thread-follow-ttl-secs / BUZZ_ACP_THREAD_FOLLOW_TTL_SECS) and a hard per-channel cap (64, least-recently-active evicted). Participation is recorded at dispatch time: the batch's thread root, or the triggering event id when the agent's reply is about to start a new thread (mirrors resolve_reply_anchor — this is the exact repro case in [Bug] buzz-acp agents go deaf in threads they've joined unless re-mentioned. #2270). Admitted followed-thread events refresh the TTL, so live conversations keep sliding.
• Relay gate: REQ construction is extracted into build_req_filters(), which emits a second filter clause — same #h/kinds/since, plus #e limited to followed roots — only when the base clause is mention-gated. The mention gate stays intact for everything else; no channel firehose. Subscription updates reuse the existing per-channel sub-id replacement path and replay from last_seen minus skew, so replies landing during the update window are recovered. Resubscribes fire only on thread join/expiry, not per message, staying inside the pacing envelope from fix(acp): honor relay rate limits and pace resubscribes on bad links #2199/fix(acp): pace relay observer frames (6/s + 90/min, zero burst) #2217.
• Local gate: match_event takes the followed set and admits an unmentioned event only when its thread root is followed. Channel scope, kinds, and evalexpr filters still apply — following relaxes only the mention check.
• Lifecycle: channel unsubscribe clears its follow state; --no-follow-threads (BUZZ_ACP_NO_FOLLOW_THREADS) restores the previous strict-mention behavior. State is in-memory, matching the harness's existing recovery semantics (after restart, a fresh mention re-joins a thread).

Related issue

Fixes #2270 (independently re-reported as #2332). The patch is confirmed by two affected users on the issue: oveddan (owner-replied-in-thread repro) and dmnyc (agent-perspective confirmation).

Scope narrowing (explicit). #2270 sketches per-rule follow_participating_threads / follow_thread_authors; this PR implements them as global CLI/env settings instead, and does not claim the rule-level contract. Reason: the followed-thread REQ clause is channel-scoped, so per-rule control needs union-at-REQ plus per-rule enforcement in match_event, and two rules disagreeing on the same channel have no coherent subscription answer. That is a subscription-design change rather than part of this fix. It is implementable on top of this (the REQ clause is a delivery optimization; correctness already lives in match_event) — happy to do it as a follow-up if the rule-level contract is wanted.

Testing

• Live-relay integration test (crates/buzz-test-client/tests/e2e_acp_thread_follow.rs): drives the real buzz-acp binary with a stub ACP agent against a live relay — asserts a mention opens the thread, an untagged in-thread follow-up is admitted, untagged replies in unfollowed threads stay suppressed, and agent-authored unmentioned replies stay suppressed under --respond-to anyone.
• 13 unit tests on the new surface: follow-state lifecycle (record/touch/TTL/cap/eviction/dirty-tracking/channel isolation), match_event followed-thread admission (admits followed, ignores unfollowed and top-level, relaxes only the mention gate), and build_req_filters clause construction (base-only, roots-ignored-without-mention-gate, two-clause shape).
• Full cargo test -p buzz-acp: 643 passed, 0 failed. cargo clippy -p buzz-acp --all-targets and cargo fmt clean.
• Manual end-to-end (Claude Code behind claude-code-acp, live local relay): mention answered in-thread; untagged human follow-up answered (~15s, p tags empty); untagged replies in unfollowed threads silent; agent-authored unmentioned replies suppressed under both owner-only and --respond-to anyone (transcripts in the review thread).
• Head 2af74400: 647 unit tests pass (638 + 9), cargo clippy -p buzz-acp --all-targets -- -D warnings clean, cargo fmt clean, and the live-relay e2e passes against a local relay (~31s).
• The anti-loop boundary now fails closed: only the configured owner or an author positively resolved through the channel's kind:39002 member roles counts as human. A cryptographically verified NIP-OA attestation or the bot role proves an agent. Missing profiles, unresolved membership, malformed events, and relay timeouts stay Unknown and still require an explicit mention under the default humans policy.
• The live-relay test now asserts exact per-message turn deltas (never >=), publishes a visible agent reply so the human follow-up replies to a real message, and covers a non-owner human member — the case that exercises member-role resolution rather than the configured-owner shortcut.
• Operator settings are documented in the buzz-acp README and .env.example.

Follow-up candidates (out of scope here)

• Surfacing followed-thread state to the desktop (ties into feat(desktop): auto-continue agent loop on untagged thread reply #1654's auto-continue UX).
• Optionally persisting follow state across harness restarts via the pending-turn ledger machinery from feat(acp,desktop): resume interrupted turns after app restart #2177.

🤖 Generated with Claude Code

[BradGroux]

Blocking evidence against current head 930c0ef33262d6f15772ff9d253e81b3853730e7: the implementation does not preserve #2270's stated anti-loop invariant.

Control-flow evidence

The current head does these three things in sequence:

1. author_allowed explicitly allows same-owner sibling agents in both OwnerOnly and Allowlist modes.
2. The inbound path applies that author gate, then passes the channel's followed roots into match_event (lib.rs).
3. match_event bypasses require_mention whenever the event's root is followed. It does not distinguish a human author from an agent author on that followed-only path.

Once sibling agents A and B have both participated in one thread, an unmentioned reply from B can therefore wake A. A's reply can wake B through the same rule. That is the agent-to-agent loop #2270 explicitly says must remain impossible.

This is not covered by the new tests. They prove that a generic unmentioned event in a followed root is admitted, but do not exercise author identity or two participating agents.

Required correction

Please keep explicit agent mentions working, but reject agent-authored, unmentioned events that are admitted only because their root is followed. If the proposed follow_thread_authors = "humans" | "all" policy is retained, the safe default needs to be humans.

Regression coverage should include:

• human unmentioned reply in a followed root → admitted;
• sibling-agent unmentioned reply in that root → dropped;
• external agent unmentioned reply in RespondTo::Anyone → dropped;
• explicitly mentioned agent-authored reply → existing configured behavior preserved;
• two agents following the same root cannot auto-continue each other.

The PR description also states that this has not been exercised against a live relay. Please add the end-to-end relay case before treating #2270/#2332 as resolved: mention agent → agent replies → human sends an untagged thread reply → harness receives exactly one new turn.

Separate timing evidence for successful explicit mentions is now tracked in #2386; this comment is about correctness and loop safety, not model latency.

[brocoppler]

The loop was real: ignore_self blocks self-loops and the author gate blocks strangers, but same-owner siblings pass the gate by design, and the follow bypass in match_event didn't check author kind. Fixed in 6318ba4:

• match_event reports admission basis (MatchedRule.admitted_via_follow) — follow-bypass admissions are distinguishable from mention matches at the call site.
• New --follow-thread-authors <humans|all> (BUZZ_ACP_FOLLOW_THREAD_AUTHORS), default humans: follow-only admissions from agent authors are suppressed. Explicit mentions never take this path. Suppression runs before the TTL refresh, so agent chatter can't keep a thread alive.
• Agenthood classification reuses the existing NIP-OA heuristic (profile_event_is_agent, now pub(crate)) via a cached kind:0 lookup that runs only on the follow-admitted path. It classifies by agenthood, not gate mode, so it covers external agents under RespondTo::Anyone. Unknown/unfetchable identities classify as human (fail open, matching turn_is_human_facing); the security gate remains author_allowed.

Regression coverage per your list: human unmentioned reply in a followed root admitted; agent-authored unmentioned reply dropped (siblings and external-under-Anyone, both via the agenthood classification); explicitly mentioned agent-authored replies unaffected; all as explicit opt-in; two-agent auto-continue cannot start from either side at the decision layer. 589 tests pass, clippy/fmt clean.

Live-relay run (local relay, this branch, Claude Code behind claude-code-acp, second identity carrying the NIP-OA auth-tag shape):

1. Mention → in-thread reply → untagged human follow-up (p tags []) → exactly one new turn, ~15s. Untagged replies in unfollowed threads: no turn.
2. Agent-authored unmentioned reply in a followed root → no turn. Under default owner-only the author gate drops it first (its attestation fails signature verification). Under --respond-to anyone — author gate admits everyone, this guard is the only defense — the harness logs followed-thread admission suppressed for agent author and fires no turn.

human:  @Scout final guard test: 7+7? Just the number.      ← dispatched, thread followed
agent2: unmentioned agent reply under respond-to anyone     ← suppressed, no turn

human: @Scout round 3: what is 2+2? …    → agent: "4"
human: thanks - now 3+3? (not tagging)   → agent: "6"   (p tags: [])

Observed turn latency (~15–30s) matches #2386. The PR description's "not yet exercised against a live relay" line is updated. Can add these scenarios as a buzz-test-client integration test if wanted.

[brocoppler]

Integration test added in b9700ca: e2e_acp_thread_follow.rs drives the real buzz-acp binary with a stub ACP agent against a live relay and asserts all four review cases (mention → turn, untagged human follow-up → turn, untagged agent-authored reply → suppressed with the log line asserted, untagged top-level → gated). Passes in ~23s locally; #[ignore]-gated like the rest of the e2e suite.

[brocoppler]

Status roll-up since head b9700ca:

• [Bug] buzz-acp agents go deaf in threads they've joined unless re-mentioned. #2270 picked up two independent confirmations today (oveddan; an agent-perspective report via dmnyc), both matching the two-gate mechanics and one flagging that the drop is invisible to operators — analysis on the issue.
• Sustained live use of this branch tonight: local relay, the desktop app, and multiple managed agents (claude-code-acp and the desktop's claude-agent-acp shim). Mention → threaded reply → untagged follow-ups produced exactly one turn each, repeatedly, across two different agents and several threads, in multi-agent channels. The loop guard held throughout — agent-authored unmentioned replies stayed suppressed.
• On the branch: the two-gate fix (REQ #e clause for followed roots + match-layer admission), --follow-thread-authors <humans|all> default humans (added after @BradGroux's loop evidence; his regression list is covered in e2e_acp_thread_follow.rs, which drives the real binary against a live relay), buzz-acp suite green, fmt/clippy clean.

@wpfleger96 @tlongwell-block — needs the first-contributor CI approval click to run in-repo. Can split the e2e harness (stub agent + test-client wiring) into its own PR if that makes review easier.

[brocoppler]

Rebased onto today's main (head now 868b3bf, post-#2974 runtime refactor) — clean, no conflicts; unit suite (643 passed) + the e2e_acp_thread_follow live-relay integration test re-verified green on the rebased head.

Description updated to match the new PR guide from #3140: verification evidence is now inline, including the two independent user confirmations on #2270 (one from the owner-repro side, one agent-perspective). Review-ready as it stands; happy to split or adjust scope if a different shape merges easier.

[BradGroux]

Thanks for the careful revision, the rebase, and the live-relay work. I re-reviewed current head 868b3bf1 against the original blocking comment rather than accepting the roll-up wholesale.

Several parts of the response are valid and address the original finding:

• MatchedRule::admitted_via_follow cleanly separates follow-only admissions from explicit mentions.
• The default humans policy suppresses a discoverable agent author before the followed-thread TTL is refreshed.
• The external-agent path is exercised under respond-to=anyone, so the author gate is not masking that test.
• The package suite passes locally at this head: 643 tests, plus Rust formatting and cargo clippy -p buzz-acp --all-targets -- -D warnings.

Two correctness blockers remain.

1. The anti-loop guard still fails open when agenthood cannot be proved. author_is_agent explicitly returns false for a missing profile, malformed author key, REST error, or two-second timeout, and a profile without the heuristic auth shape is cached as human. That lets an unmentioned follow-only event through even though the issue's invariant is that agent-authored replies must require an explicit mention. The new E2E proves suppression only when the relay has a discoverable kind-0 profile with the expected tag shape; it does not cover an absent/stale profile or lookup failure. On the default humans path, unknown cannot mean proven human if this is the loop boundary.

2. The live-relay test does not prove the “exactly one new turn” claim. await_turns returns when the total is >= expected. If the first mention dispatched two turns and the untagged human follow-up dispatched zero, the first wait would pass at 2, the second wait would return immediately at 2, and both later no-new-turn checks would still pass. Please assert a per-message delta of exactly one and keep the count stable for a bounded window. The stub also never publishes the agent reply, so the test covers dispatch-based following but not the exact user-visible chain requested in the original review: mention → agent reply → untagged human reply → exactly one new turn.

There are two contract/documentation gaps as well:

• Issue [Bug] buzz-acp agents go deaf in threads they've joined unless re-mentioned. #2270 asks for per-rule follow_participating_threads and follow_thread_authors; this implementation exposes global CLI/environment settings. I am not asking for the broader subscription design to be rewritten—the precise second #e clause is a reasonable way to avoid a channel firehose—but the rule-level contract should either be implemented or the issue/PR should explicitly narrow that requirement.
• The three new operator settings are absent from the ACP README and environment documentation, despite CONTRIBUTING.md requiring new config variables to be documented.

I compiled the new E2E target successfully. I could not independently execute it because this review environment had no local relay or Docker runtime, so the source-level gaps above matter: the published test currently cannot establish the claims it is being used to support.

The original loop finding was valid, and this revision materially improves the PR, but I do not think it is ready for approval yet. A fail-closed/positively-proven-human boundary, exact per-event turn assertions, and the config contract/docs cleanup would make the re-review straightforward.

[PaulBlackSwan]

I prepared a focused follow-up patch for the remaining review blockers: https://github.com/brocoppler/buzz/pull/1\n\nIt makes the humans-only boundary fail closed, verifies NIP-OA attestations cryptographically, falls back to NIP-29 member roles, strengthens the live-relay test to cover a visible agent reply followed by an unmentioned human reply with exact turn counts, and documents the three settings. The buzz-acp suite (637 + 9 tests), clippy, formatting, and E2E compilation pass. The live relay run is still pending because local Docker image pulls failed with an unexpected EOF.

[brocoppler]

Both blockers are addressed at head 2af74400, along with the two contract/docs gaps. @PaulBlackSwan opened a patch against my fork for the first three; I reviewed it line by line, verified its relay-side assumptions against this repo rather than taking them on faith, took it with attribution, and then fixed two problems in it.

1. The guard now fails closed. author_is_agent is replaced by classify_follow_author, returning Human / Agent / Unknown, and follow-only admission requires a positively proven Human. Owner match, or a member role resolved from the channel's relay-materialized kind:39002 event, proves human. A cryptographically verified NIP-OA attestation (verify_auth_tag, not the old tag-shape heuristic) or the bot role proves agent. Missing profile, unresolved membership, malformed event, and relay timeout all stay Unknown and remain mention-gated. Unknown-means-suppressed is unit-tested directly.

2. The test proves exactly one turn. await_turns(>= expected) is gone. await_exact_turn_delta asserts the count never exceeds the expected value, requires exact equality, then holds it stable for a bounded window; assert_no_new_turns is now assert_eq rather than <=. The chain you asked for is the chain under test: mention → agent publishes a visible reply → untagged human reply to that reply → exactly one new turn.

I added a case beyond the patch: a non-owner human member. Every human in the original test was the configured owner, which short-circuits at the top of the classifier — so a wrong assumption about the member-role tag layout would have suppressed every real human member while the suite stayed green. The added case forces resolution through kind:39002. (I verified the layout independently: the relay writes ["p", pubkey, "", role] with d = channel id.) I also anchored the trailing no-new-turn assertions to the observed count instead of the literal 2, which caught a real off-by-one when I inserted that case.

Fixed in the patch: it resolved humans through two uncached relay round-trips on every follow-admitted event — the common case this feature exists to serve, on the dispatch path, at up to 2s each. Proven classes are now cached, with the NIP-OA result global (identity-level, immutable) and the member-role result channel-scoped, since one identity can be a plain member in one channel and a bot in another; a single author-keyed cache would have leaked a role across channels. Unknown is never cached, so a transient relay failure can't pin an author as suppressed for the session. Both properties are unit-tested.

3. Config docs. The three settings are documented in the buzz-acp README (flag / env / default / description) and .env.example.

4. Rule-level contract — narrowed explicitly, per your first option, and written into the PR description rather than left implicit. This PR ships global CLI/env settings and does not claim per-rule follow_participating_threads / follow_thread_authors. The reason is structural: the followed-thread REQ clause is channel-scoped, so per-rule control needs union-at-REQ plus per-rule enforcement in match_event, and two rules disagreeing on one channel have no coherent subscription answer. It is implementable on top of this — the REQ clause is a delivery optimization and correctness already lives in match_event — and I'll do it as a follow-up if you want the rule-level contract rather than the narrowing.

Head 2af74400: 647 unit tests (638 + 9), clippy -D warnings clean, fmt clean, and the live-relay e2e passes against a local relay in ~31s. Your point about the published test not being able to establish its own claims was the right one to press — the suppression assertions were weaker than they read.