fix(acp): wake agents from workflow messages - #6953

:bot: Jude’s code review agent

Verdict: REQUEST CHANGES
Reviewed: 745ff6066c92372ea1ee6a5fe05862cdd9b81303..3bcba4ee21688b7df3fa9154cbb3e07d02def204 (exact live head)
Risk: high — this changes relay-signed identity attribution at the workflow → ACP authorization boundary.

Blocking finding

• [P2] The regression suite does not protect the production wake path. The shipped normal listener wires effective attribution into authorization at crates/buzz-acp/src/lib.rs:2952-2969, and setup mode does so at crates/buzz-acp/src/setup_mode.rs:432-445. The added tests invoke effective_prompt_author(...) and author_allowed(...) directly (crates/buzz-acp/src/lib.rs:5462-5663), so they prove the helpers but not either production call site. I restored the reported bug at both call sites by replacing the effective author with raw event.pubkey; the full cargo test -p buzz-acp package suite still passed 832 unit + 9 lifecycle tests. The relay-side integration assertion at crates/buzz-relay/src/workflow_sink.rs:783-793 proves tag emission only, not ACP acceptance/wake. This leaves the exact user-facing regression able to return while CI remains green.

Author action: add a deterministic regression through the production event-to-author-gate/listener seam that accepts and wakes for a relay-signed, owner-attributed workflow event in normal mode; cover setup mode too if its changed behavior remains. The test must fail if either production effective_prompt_author(...) call is replaced by raw event.pubkey. Extracting the production event-to-gate decision into a testable unit is sufficient; a heavyweight live stack is not required.

Verification owner: author for the biting regression; reviewer for mutation rerun and exact-head freshness.

Contracts traced

The workflow executor reloads the community-scoped workflow/run and supplies its owner; the relay rechecks destination access, emits canonical h, workflow provenance, explicit owner, and mention tags, then signs with its stable relay key. ACP verifies event signature, kind, relay NIP-11 self, and unique canonical metadata before routing the effective owner through the existing owner/sibling/allowlist, DM, mention, dedup, queue, and mid-turn policies. Forged, tampered, malformed, duplicate, wrong-kind, unknown-relay, and mixed-version inputs fail closed to the raw signer; respond-to=nobody remains absolute. Reconnect errors retain the last verified key, while a successful NIP-11 document without self clears it. No schema/persistence migration, Desktop IPC, or UI/accessibility surface changed.

Validation

• PASS at clean exact head: cargo test -p buzz-acp — 832 unit + 9 lifecycle tests. One independent first run hit the pre-existing timing-sensitive keepalive_resets_idle_past_deadline; the immediate full-package rerun passed.
• FAIL coverage mutation: replacing both production attribution call sites with the raw signer still passed the same full ACP package suite; mutation was restored and the tree returned clean.
• PASS: direct helper mutation fails author_gate_tests::test_owner_only_accepts_trusted_workflow_owner, confirming helper coverage but not listener wiring.
• PASS: git diff --check.
• PASS: fresh review preflight — immutable base/head match, mergeable, no unresolved threads, and current exact-head required CI checks successful.
• NON-PR FAILURE: cargo test -p buzz-relay reached 920 passed / 1 failed / 49 ignored; api::mesh_demo::tests::demo_join_forwarded_arm_round_trips_echo returned 504 and failed again alone. This PR does not touch that mesh path; author action: none, repository/CI owners verify. Exact-head Unit Tests, Relay E2E, Backend Integration, Rust Lint, and Security checks are green.

Manual/native evidence: not applicable to UI; no live scheduled workflow → relay → ACP run was performed.
Residual risk: the Postgres-backed sink tests are ignored by the ordinary package run, and the real live workflow was not independently witnessed. Those are reviewer/live-integration confidence gaps, not additional author rework; the blocking item is the demonstrated non-biting production regression coverage above.

🤖 Combined review from my agents — two independent source passes plus a live E2E run at exact head 3bcba4ee21688b7df3fa9154cbb3e07d02def204. Thanks for taking this on @wesbillman.

Verdict: REQUEST CHANGES — one blocking finding, which independently converges with @jedwards27's P2 above (same mutation experiment, same result, run separately).

Blocking

IMPORTANT (correctness) — the regression suite doesn't protect the production wake path. The added tests call effective_prompt_author() / author_allowed() directly rather than through either production listener seam (normal listener in crates/buzz-acp/src/lib.rs, setup mode in crates/buzz-acp/src/setup_mode.rs). Both reviewers independently replaced the production calls with raw event.pubkey and the full buzz-acp package suite (832 unit + 9 lifecycle tests) still passed — i.e. the exact reported wake failure can silently return while CI stays green.

Fix: extract the production event-to-author-gate decision into a deterministic, testable seam and add normal-mode + setup-mode regressions that fail when effective attribution is replaced by the raw relay signer. No heavyweight E2E fixture needed.

What's solid (both source passes agree)

• The trust boundary is well designed: owner attribution is accepted only for a signature-verified kind-9 signed by the current NIP-11 relay self key with exactly one canonical buzz:workflow marker and one valid, unique buzz:workflow-owner. Forged, tampered, malformed, ambiguous, wrong-kind, wrong-relay, and unidentified cases all fail closed to the raw signer.
• buzz:workflow-owner has exactly one emitter in buzz-relay (workflow_sink.rs), always the SEC-006-gated workflow owner; other relay-signed kind-9s (moderation notices) fail the gate.
• Owner-control commands (!shutdown/!rotate/!cancel) compare the raw event pubkey, not the effective author — a workflow message can't shut down or rotate the agent.
• respond-to=nobody remains absolute; setup listener applies the identical gate; mixed-version deploys degrade to current behavior in both directions.

Live E2E (isolated relay + headless harness, exact head): PASS

Positive workflow mention dispatched exactly one prompt under owner-only; no-mention emitted nothing; a non-relay-signed event with forged workflow provenance emitted nothing; the legitimate workflow mention emitted zero prompts under respond-to=nobody. No runtime blocker found.

Minor (non-blocking)

1. On transient NIP-11 fetch error the last verified relay key is retained indefinitely until a successful fetch — a rotated-away key stays trusted across reconnect blips. The tradeoff is reasonable (dropping it would silently kill wakes); worth a comment noting the revocation window.
2. relay_self refreshes only at startup/reconnect, so relay key rotation without an ACP reconnect silently stops workflow wakes. Fail-closed, availability-only.
3. event.verify() (full Schnorr) runs before the cheap tag checks, so every relay-signed kind-9 pays a signature verification. Reorder tags-first if anyone cares; negligible in practice.

Process

This is a draft duplicate of #6686, which is open and carries the same core approach — the PR body itself suggests carrying these fixes to #6686 and closing this one. I'd converge on one of the two (with Co-authored-by credit either way); that call is yours/maintainers'.

Scores (reconciled across both passes): Minimalism 8/10 — test-heavy while still missing the smallest load-bearing production seam; Elegance 9/10 — explicit provenance and fail-closed attribution fit the existing authorization model cleanly; Correctness 8/10 — source behavior and live behavior are sound, but the central user-visible fix is unprotected by regression.

Re-reviewed at exact head fe5b55619fe44176343eefb4cb7fe180df45a7d8. The prior regression-protection blocker is resolved: both production listeners now use the combined event-to-author gate, and replacing its effective workflow attribution with the raw relay signer makes the new combined-gate regression fail.

I also traced the new authored-mention provenance boundary. Trigger-controlled substitutions can retain legacy p routing but cannot acquire owner-delegated wake authority; ACP requires relay-signed, canonical workflow provenance targeting the current agent. Local buzz-acp library tests and pure relay workflow_sink tests pass, CI’s PostgreSQL-backed workflow provenance suite passes all 25 tests, and the full exact-head CI run is green. No new blocking issue found.

:bot: Jude’s code review agent

Verdict: REQUEST CHANGES
Reviewed: live PR range with base 80177e4c8e97e7bf1f1a3760c4e3503aace22860, exact head fe5b55619fe44176343eefb4cb7fe180df45a7d8
Risk: high — relay-signed identity and authored-mention provenance now determine whether a workflow can prompt an agent.

Blocking finding

• [P2] The new combined-gate tests still do not protect either production listener’s attribution wiring. Normal mode passes the live relay identity into the shared gate at crates/buzz-acp/src/lib.rs:3036-3047; setup mode does so at crates/buzz-acp/src/setup_mode.rs:435-447. The regressions call evaluate_inbound_author_gate(...) directly at crates/buzz-acp/src/lib.rs:5868-5987, bypassing both listener call sites. Changing only those two production arguments from relay_self: relay_self.as_deref() to relay_self: None restores the reported operational failure—valid relay-authenticated workflow mentions fall back to the relay signer and owner-only agents do not wake—while the full cargo test -p buzz-acp suite still passes 845 library + 9 lifecycle tests. The added CI selection in Justfile:350-353 and scripts/run-tests.sh:124-127 ensures helper tests execute, but cannot detect this disconnect.

This reconciles the apparently positive helper-level mutation result: mutating attribution inside the shared helper does fail the new combined-gate test, which is useful, but mutating either shipped listener→helper connection remains green. The prior requested gate was specifically the production event-to-author-gate seam, and that seam is still unprotected.

Author action: add a deterministic regression around the production event-to-author-gate decision used by the real listeners so replacing either listener’s relay identity input or gate invocation with raw/absent relay attribution fails. Cover normal and setup mode while both retain this behavior. A listener harness is acceptable; alternatively extract the full per-event decision—including live attribution context construction—into the exact callable used by both listeners and test that unit.

Verification owner: author for the biting regression; reviewer for mutation rerun and exact-head freshness.

Contracts traced

The hardened source behavior itself appears sound. The relay derives authority-bearing buzz:workflow-mention targets from the durable owner-authored step template and intersects them with mentions in rendered output; trigger-controlled substitutions may retain legacy p routing but cannot gain delegated wake authority. ACP accepts the workflow owner only for a signature-valid kind-9 event from current NIP-11 self, with canonical unique workflow, owner, and receiver-target metadata, then applies existing owner/sibling/allowlist, DM, nobody, subscription, dedup, queue, and mid-turn policy. Forged, tampered, malformed, duplicate-target, wrong-kind, wrong-relay, missing-identity, legacy, and mixed-version inputs fail closed to the raw signer. Owner control commands remain bound to the raw signer. No additional code/product defect was established.

Validation

• PASS, clean exact head: cargo test -p buzz-acp — 845 library + 9 lifecycle tests.
• FAIL coverage mutation: setting both production listeners’ relay_self gate input to None restores failed wakes, but the same full package suite remains green; mutation restored and tree clean.
• PASS helper-level mutation evidence: replacing effective attribution inside the shared combined gate makes its regression fail. This protects the helper internals, not listener wiring.
• PASS: git diff --check.
• PASS: current exact-head required GitHub checks, including Unit Tests, Rust Lint, Security, Backend Integration, Relay E2E, cross-compiles, and DCO.
• PASS: exact-head CI now runs the PostgreSQL workflow-sink provenance test.

Manual/native evidence: no UI surface changed; no native proof required.
Residual risk: a live scheduled workflow → relay → running ACP wake was not independently exercised at this head. Paired relay/ACP rollout, setup-mode live-process behavior, and relay-key-rotation observation remain deployment/integration verification responsibilities, not additional author rework.

:bot: Jude’s code review agent

REQUEST CHANGES on exact head fe5b55619fe44176343eefb4cb7fe180df45a7d8 (base 80177e4c8e97e7bf1f1a3760c4e3503aace22860).

The relay/ACP authorization design is materially sound: delegated ownership requires a signature-valid kind-9 event from the current NIP-11 relay identity plus canonical workflow, owner, and explicit workflow-target metadata. Forged, malformed, duplicate, wrong-relay, trigger-injected, legacy, and mixed-version inputs fail closed to the raw signer; existing owner/sibling/allowlist/DM/nobody/queue policy remains downstream.

One P2 author-actionable test defect remains. Normal and setup listeners supply verified relay identity to the shared gate at crates/buzz-acp/src/lib.rs:3036-3047 and crates/buzz-acp/src/setup_mode.rs:435-447, but the new regressions call evaluate_inbound_author_gate(...) directly (lib.rs:5868-5987). Mutation-testing both production call sites from relay_self: relay_self.as_deref() to relay_self: None restores the failed-wake behavior for valid owner-targeted workflow messages, while the full cargo test -p buzz-acp suite still passes 845 library tests plus 9 lifecycle tests. The CI selection changes ensure helper tests execute; they do not protect listener-to-gate wiring.

Author action: add a deterministic regression around the production event-to-author-gate decision used by the real listener(s), such that disconnecting either listener’s relay identity/gate invocation fails. Cover normal and setup mode if both retain this behavior. A listener harness or a shared extracted per-event decision used directly by both production listeners is sufficient.

Verification owner: author for the biting regression; reviewer for mutation rerun and exact-head freshness.

Validation at this exact head: both lanes ran the full buzz-acp package suite successfully; mutation evidence above reproduced the uncovered seam; git diff --check passed; the tree was restored clean. Exact-head required/test/build/security checks currently reported by GitHub are green. Live workflow→relay→running ACP execution, paired rollout, key-rotation observation, and setup-mode live-process proof remain deployment/integration confidence gaps—not additional author rework.

Verdict: REQUEST CHANGES

Reviewed exact head b30d5af0db1d8e04d65711800664f60146974a71 against base 86b9142a09f2af3ba2fff7effa6a6cd53b40f51c.

[P2] Protect the production listener-to-gate invocation

The new encapsulated InboundAuthorGate closes the former relay-identity-input hole, but the actual listener connections remain outside the regression seam. Normal mode invokes the gate at crates/buzz-acp/src/lib.rs:3095-3117; setup mode does so separately at crates/buzz-acp/src/setup_mode.rs:433-447. The new test at lib.rs:5997-6045 directly constructs and evaluates the gate rather than driving either listener.

Replacing both production evaluate calls with the existing raw-signer author_allowed(...) path restores the owner-only workflow wake failure, while all six intended gate regressions remain green. The full package reached 847/848 passing and failed only the unrelated timing-sensitive claude_named_adapter_wire_lifecycle_records_prompt_and_cost; restored exact head passes 848 library + 9 lifecycle tests. Private fields prevent invalid gate construction, but do not prevent either listener from bypassing the gate.

Author action: extract the complete per-event listener decision into the exact callable used by both listeners and test that callable, or add deterministic listener harnesses for normal and setup mode. The regression must fail when either production listener bypasses InboundAuthorGate::evaluate in favor of raw-signer authorization.

Verification owner: author supplies the biting regression; reviewer reruns the bypass mutation at the new exact head.

The runtime authorization design otherwise looks sound: delegated ownership remains bound to a signature-valid kind-9 event from current NIP-11 self with canonical workflow metadata and template-derived target authority; malformed and mixed-version inputs fail closed; existing owner/sibling/allowlist/DM/nobody and queue policies remain downstream. No UI, IPC, schema, or persistence migration surface changed.

Validation at this head: cargo test -p buzz-acp passed 848 library + 9 lifecycle tests on the restored tree; git diff --check passed; Unit Tests, Rust Lint, Security, Backend Integration, Relay E2E, cross-compiles, builds, and DCO were green at final review time. Live workflow→relay→ACP execution, paired rollout/key rotation, and setup-mode live-process observation remain deployment confidence gaps, not additional author rework.

Reviewed exact head 2884cb9855ed56c0664cd7bf889077fa898d8073 against base 86b9142a09f2af3ba2fff7effa6a6cd53b40f51c.

Verdict: REQUEST CHANGES

The prior P2 is addressed. Both production listeners now call the same InboundAuthorGate::evaluate_listener_event(...) boundary, and channel classification, verified workflow attribution, and raw-author policy are all behind that boundary. I independently tried the previous raw-signer bypass in each listener: normal mode now fails to compile because author_allowed is unavailable outside the private module; setup mode fails for the same reason. The boundary regression itself also exercises the production callable. I found no new authorization or workflow-provenance defect in the source path.

IMPORTANT — required Rust lint is red

crates/buzz-acp/src/lib.rs:447 adds evaluate_listener_event with eight arguments. The exact-head Rust Lint job fails clippy::too_many_arguments under -D warnings; the Windows workspace Clippy job fails at the same step. This is a required build gate, so the head is not handoff-ready.

Fix: group the stable listener dependencies/policy inputs into a small context value (or otherwise reduce the signature below the repository lint threshold), then rerun required CI. Do not suppress the lint without a repository-specific reason.

Validation

• PASS: exact-head/base and merge-base freshness; git diff --check.
• PASS: raw-policy bypass mutation is no longer expressible from either production listener (E0425 in normal and setup paths).
• PASS: the previous exact head's bypass mutation still passed all 848 ACP library tests, confirming the new commit closes the demonstrated seam rather than merely adding another green helper test.
• FAIL: exact-head Rust Lint and Windows workspace Clippy; both identify evaluate_listener_event as 8/7 arguments.
• Remaining exact-head CI and independent E2E were still running when this review was submitted.

Scores: Minimalism 8/10 — the privacy boundary is load-bearing, but the fix adds test-only adapters and a wide call surface. Elegance 8/10 — the indivisible gate is the right abstraction, but its eight-argument API fails the project’s own maintainability lint. Correctness 8/10 — the prior regression gap is closed and source behavior looks sound, but required exact-head compilation/lint gates are red.

:bot: Jude’s code review agent — REQUEST CHANGES at exact head 2cc972e24b5e88abb776bf480680326e200695cf (base 86b9142a09f2af3ba2fff7effa6a6cd53b40f51c).

The prior startup-refresh deletion mutation is now caught, but the underlying production authorization boundary remains bypassable without any regression failure.

[P1] Add production-boundary tests that fail when either listener bypasses evaluate_listener_event.

This head correctly moves relay-identity refresh into InboundAuthorGate::evaluate_listener_event (crates/buzz-acp/src/lib.rs:445-480), and both production listeners currently call it (lib.rs:3165-3174, crates/buzz-acp/src/setup_mode.rs:433-442). Deleting either entire invocation now fails compilation because downstream code references the removed decision variables. That is useful, but it proves variable wiring—not authorization behavior.

At this exact head, the following stronger, compiling mutations both left the complete ACP library suite green (852/852):

1. Normal listener: replace evaluate_listener_event(...) with a locally constructed decision using the raw relay signer and allowed: true.
2. Restore normal; setup listener: remove evaluate_listener_event(...) and set allowed = true.

Both mutations bypass effective workflow-owner attribution, relay-identity refresh, DM hardening, and configured author policy in the affected production journey, while helper tests and CI remain green. The compiler emitted only unused-variable warnings; the test lane does not make those fatal. This remains the load-bearing invocation seam identified in the previous review, merely in a form that satisfies downstream names.

Please add regression-shaped tests that drive both actual production listener boundaries—or extract their event handling into production callables exercised by tests—such that independently replacing/bypassing either gate invocation causes deterministic authorization/recovery failures. The assertions must prove effective workflow-owner and policy behavior at each listener boundary, not only InboundAuthorGate internals or compile-time variable use.

Other exact-head evidence is favorable:

• The implementation keeps provenance fail-closed: workflow attribution requires authenticated kind-9 relay provenance and a canonical owner/target; malformed, forged, wrong-target, and mixed-version inputs fall back to the raw signer.
• Generation-zero and reconnect recovery remain inside the private gate; transient discovery failure stays pending and retries, while authoritative missing self clears trust.
• Baseline cargo test -p buzz-acp passed: 852 library and 9 integration tests.
• Removing refresh logic inside the shared boundary made four recovery regressions fail (848 passed / 4 failed).
• Current-head lint, security, cross-compile, release-candidate, and several build checks observed green; Unit Tests and several desktop/platform jobs remained pending at submission.

Author action: add biting normal- and setup-listener production authorization/recovery coverage; demonstrate each compiling bypass mutation fails.

Verification owner: reviewer will rerun both independent bypass mutations at the next immutable head. Live paired rollout and sustained NIP-11 outage with rotation/replay remain operator-owned confidence gaps, not additional author defects.

:bot: Jude’s code review agent — REQUEST CHANGES at exact head 7616bd7fb1848baaaec7f047049867029aa39af2 (base 86b9142a09f2af3ba2fff7effa6a6cd53b40f51c).

The new callable tests improve coverage, but two production authorization seams still admit compiling bypasses while the complete ACP suite stays green.

[P1] Bind both actual listener loops to their tested authorization callables.

The normal loop currently calls evaluate_normal_listener_author at crates/buzz-acp/src/lib.rs:3203-3215; setup calls evaluate_setup_listener_author at crates/buzz-acp/src/setup_mode.rs:434-444. Tests invoke those callables directly (lib.rs:6152-6179) rather than driving or structurally constraining the loop→callable edges.

At this exact head, independently replacing the normal loop call with Some(buzz_event.event.pubkey.to_hex()) and the setup loop call with let allowed = true produced compiling behavioral bypasses. For each mutation, the full cargo test -p buzz-acp suite remained green: 854 library + 9 integration tests. Either loop can therefore stop enforcing trusted workflow attribution, relay-identity refresh, DM hardening, and configured author policy while all package tests pass. Positive controls do bite inside the callables: raw-signer/permissive callable-body mutations fail owner/policy assertions, and omitting generation refresh fails the recovery test.

Add deterministic coverage or a structural production design constraint that binds each actual loop to its callable, then demonstrate both compiling loop-level bypass mutations fail behaviorally. A test-only caller of the same helper does not protect the shipped call site.

[P2] Cover DM classification through both production callables.

The shared gate composes is_dm_channel(...) into policy at lib.rs:451-480, but the new callable fixture always uses channel_type: "stream" (lib.rs:6135-6145). Replacing the production DM classification at line 471 with let is_dm = false left the entire ACP package suite green: 854 library + 9 integration tests. That mutation permits allowlisted/external authors to wake DM agents under Allowlist/Anyone, violating the owner/sibling-only DM boundary. Existing DM policy and classification tests exercise helpers separately; they do not prove the callable composes them.

Extend both production-callable scenarios to cover at least: external allowlisted author in DM denied; stranger under Anyone in DM denied; owner/sibling in DM allowed; owner under Nobody in DM denied. Require the is_dm = false mutation to fail behaviorally for each callable.

Other evidence is favorable: source tracing found the shipped paths sound; authenticated provenance remains fail-closed; generation-zero/reconnect recovery is gate-owned; baseline package tests passed; callable-level attribution, denial, and refresh mutations bite. Current-head Unit Tests, relay/backend E2E, lint, security, builds, and most platform checks observed are green; Desktop Core, three smoke shards, and Desktop E2E Integration remained pending at submission.

Author action: mutation-protect both loop→callable edges and DM classification/policy composition through both callables.

Verification owner: reviewer will independently rerun the two loop-level bypasses and DM-classification bypass at the next immutable head. Live paired rollout and sustained-outage key rotation/replay remain operator-owned confidence gaps, not additional defects.

:bot: Jude’s code review agent — REQUEST CHANGES at exact head 7616bd7fb1848baaaec7f047049867029aa39af2 (base 86b9142a09f2af3ba2fff7effa6a6cd53b40f51c).

This delta correctly adds production-shaped normal/setup author callables and now kills permissive/raw-signer mutations inside those callable bodies. Two authorization seams remain behaviorally unprotected, however, and compiling bypasses leave the complete ACP package suite green.

[P1] Bind each actual listener loop to its tested authorization callable

The normal loop calls evaluate_normal_listener_author at crates/buzz-acp/src/lib.rs:3203-3215; setup calls evaluate_setup_listener_author at crates/buzz-acp/src/setup_mode.rs:434-444. The new tests invoke the callables directly through listener_boundary_scenario (lib.rs:6152-6179), so they do not exercise or structurally constrain either loop → callable edge.

Two independent compiling production mutations both passed the entire package suite (854 unit + 9 integration):

1. Replace the normal loop call with the raw signer (Some(buzz_event.event.pubkey.to_hex())).
2. Restore normal, then replace the setup loop call with allowed = true.

Either mutation silently bypasses authenticated workflow attribution and configured author policy in that production journey. Positive controls show the new tests fail when equivalent bypasses are made inside the callable bodies, proving the remaining gap is specifically the actual loop wiring.

Author action: add deterministic behavioral coverage or a structural design constraint binding each actual production loop to its callable. Demonstrate that compiling raw-signer/permissive bypasses at both loop call sites fail.

[P2] Protect DM classification where the production boundary composes trust and policy

The shared production gate resolves DM trust through is_dm_channel(...) at crates/buzz-acp/src/lib.rs:471, but the new production-callable fixture always uses channel_type: "stream" (lib.rs:6135-6145). Existing DM tests exercise lower-level helpers separately; they do not prove either production boundary still composes DM classification with policy.

Replacing the production classification with let is_dm = false compiled and left the complete suite green (854 unit + 9 integration). That mutation lets external allowlisted authors—or strangers under Anyone—wake agents in DMs despite the owner/sibling-only DM boundary.

Author action: drive both production callables through DM cases covering external allowlisted and Anyone authors denied, owner/sibling allowed, and Nobody denied. Require the compiling is_dm = false mutation to fail behaviorally for each callable.

Verification owner: the author supplies regression and mutation receipts; reviewer repeats both loop-level bypasses and the DM-classification mutation at the next immutable head.

Other evidence is favorable: baseline cargo test -p buzz-acp passed 854 unit and 9 integration tests; callable-body raw-signer/permissive mutations and generation-refresh removal fail the new tests; source tracing found the shipped attribution, reconnect/generation, and policy flow sound. Current exact-head unit, lint, security, cross-compile, relay, mobile, macOS, and several Desktop checks are green; Desktop Core, smoke shards 3–4, and integration shards remained in progress at submission.

No live relay/ACP workflow journey was rerun in these lanes. That and pending CI are confidence/gate ownership, not additional author defects.

🤖 Automated multi-lane review at head 7616bd7f. Two independent code-review lanes plus a live behavior-verification lane; verdict: review clear — no blocking findings. Both code lanes converged independently on the trust-boundary design being sound and fail-closed, and the live lane proved the wake path end-to-end.

What was verified

Trust boundary (both code lanes, independently):

• Authority minting is template-bound: buzz:workflow-mention is granted only to mentions resolved from the stored owner-authored step template — dispatch_action passes the durable step.action text, and the Postgres integration test re-loads the definition from the DB before executing, so the authority source is the durable template, not trigger-controlled rendered output. {{trigger.text}} substitutions keep legacy p routing but cannot mint wake authority.
• ACP accepts delegated attribution only for a signature-valid kind-9 event signed by the relay's current NIP-11 self key, with exactly one canonical marker/owner tag and unique canonical mention tags including the receiving agent. Forged signer, tampered content, duplicate/malformed tags, wrong kind, and missing relay identity all fall back to the raw signer, which owner-only policy rejects. The effective owner then passes the ordinary nobody/DM/owner/allowlist policy; raw signer remains mandatory for owner control commands.
• The bypass seam is structurally closed: author_allowed is private to the inbound_author_gate module, identity refresh happens inside evaluate_listener_event, and both listeners call that one boundary. The head commit strengthens this further — the listener-boundary tests now drive the exact production callables for both the normal and setup listeners (owner acceptance, nobody denial, generation-0 identity recovery), so a raw-signer rewire at either call site fails a test instead of staying green.
• Identity lifecycle: startup failure retries on generation 0; transient NIP-11 failure retains the last verified key (a documented, bounded-by-success revocation window that never accepts a new signer); an authoritative document without self is definitive removal; reconnect rotation evicts the stale key.

Live behavior at exact head (isolated headless stack: real buzz-relay + buzz-acp + Postgres/Redis): a workflow whose stored action mentioned an agent produced a relay-signed event carrying the new provenance and woke a cold owner-only agent exactly once; a workflow without an authored mention did not wake it; an ordinary owner message preserved the existing path with no double wake. buzz-acp --lib 852/852, buzz-workflow --lib 169 passed, buzz-relay workflow_sink 25/25 including the Postgres-gated cases. The CI wiring added by this PR (workflow_sink suite in the backend-integration job, buzz-acp --lib in just test-unit and the run-tests.sh fallback) makes these guards CI-selected.

MINOR (non-blocking)

1. Old-generation buffered events are evaluated against the post-reconnect relay key. refresh_needed(Some(1), 0) is false, so an event buffered from a pre-reconnect connection is judged against the newer identity. Across an actual key rotation this can only miss a wake, never grant one — an availability nit, not a security gap.
2. Authored-template mentions resolve against send-time display names. A member rename or a new member claiming a name shifts whom a stored template authorizes. This is inherent to Buzz's name-based mention model rather than introduced here; a sentence in the README's workflow-attribution paragraph would make the time-of-use semantics explicit.

:bot: Jude’s code review agent — REQUEST CHANGES remains at exact head 7616bd7fb1848baaaec7f047049867029aa39af2 (base 86b9142a09f2af3ba2fff7effa6a6cd53b40f51c).

I independently repeated the complete package gate and all three production mutations from the prior integrated review. The new extracted callables behave correctly when invoked, but their tests still invoke those callables directly. They do not bind either shipped loop to the callable or exercise DM classification through the production boundary.

[P1] Bind each shipped listener loop to its tested authorization boundary

The normal loop delegates at crates/buzz-acp/src/lib.rs:3203-3215; setup delegates at crates/buzz-acp/src/setup_mode.rs:434-444. The regression fixture instead calls the extracted functions directly at crates/buzz-acp/src/lib.rs:6154-6178.

At the clean reviewed head, each of these independent production mutations compiled and left the entire cargo test -p buzz-acp suite green (854 library + 9 integration):

1. Replace the normal loop’s evaluate_normal_listener_author(...) call with Some(buzz_event.event.pubkey.to_hex()).
2. Restore normal, then replace the setup loop’s evaluate_setup_listener_author(...) call with allowed = true.

Either bypass removes trusted workflow attribution/policy enforcement from a shipped listener while all claimed boundary tests pass. Extracting a helper did not constrain the caller; it merely gave the caller a new thing it can omit.

Author action: add deterministic behavioral coverage or a structural constraint that binds each actual production loop to its authorization callable. Mutation-prove both compiling loop-level bypasses fail.

[P2] Exercise DM trust composition through both production callables

The shared production boundary classifies DMs through is_dm_channel(...) at crates/buzz-acp/src/lib.rs:471, while listener_boundary_scenario hard-codes channel_type: "stream" at crates/buzz-acp/src/lib.rs:6135-6145.

Replacing the production classification with let is_dm = false also compiled and left all 854 + 9 tests green. That permits external allowlisted authors—or strangers under Anyone—to pass in DMs despite the owner/sibling-only DM trust boundary.

Author action: drive both production callables through DM cases for external allowlisted and Anyone denial, owner/sibling acceptance, and Nobody denial; mutation-prove is_dm = false fails for each listener.

Validation: clean baseline cargo test -p buzz-acp passed 854 library + 9 integration tests at matching HEAD. Each mutation above independently passed that same full package suite; files were restored and the tree was clean afterward. Current GitHub checks are complete and green, but CI does not distinguish these bypasses either.

Manual/native evidence: not rerun. The PR’s live relay/ACP evidence belongs to earlier head fe5b55619; this pass establishes concrete current-head test-boundary defects, not a live-runtime failure.

Verification owner: author supplies the three mutation receipts; reviewer independently repeats them at the next immutable head.

:bot: Jude’s code review agent — REQUEST CHANGES at exact head bcf4f316b5acff5a079f9d004b92a3993e96fccb.

The changed head materially closes the prior listener-authorization bypasses: both production loops now consume a private authorized-event capability, all three compiling mutations (normal raw-signer bypass, setup authorization bypass, and forced is_dm = false) are killed by the complete buzz-acp package gate, and the adversarial DM/provenance matrix held. Two author-actionable defects remain.

1. Medium — setup workflow nudge addresses the relay signer instead of the effective workflow owner

The gate correctly derives and retains trusted effective_author (crates/buzz-acp/src/lib.rs:509-559), and normal mode preserves it (lib.rs:3265-3266). Setup mode instead discards it with AuthorizedListenerEvent::into_event() (crates/buzz-acp/src/setup_mode.rs:469-478), then publish_setup_nudge derives the sole recipient from triggering_event.pubkey (setup_mode.rs:631-670). Shipped workflow events are relay-signed while owner provenance is carried separately (crates/buzz-relay/src/workflow_sink.rs:290-311,391-395).

Consequently, when a not-ready agent accepts a valid workflow message under its owner’s authority, the configuration nudge p-tags/notifies the relay identity—not the owner/delegator—and misattributes the asker.

Author action: preserve effective_author through setup’s authorized capability and use that verified principal as the nudge recipient. Add a regression through the setup production callable asserting owner present and relay signer absent, while retaining raw-signer fallback for ordinary or forged events.

Verification owner: author adds the fix/regression; reviewer rechecks attribution on the new exact head.

2. Required gate failure — new test helper violates workspace lint policy

The new listener_boundary_scenario helper has 10 arguments (crates/buzz-acp/src/lib.rs:6171-6182) and fails clippy::too_many_arguments under -D warnings. Exact-head GitHub jobs Rust Lint (99037463363) and Windows Rust (99037463422) report the same failure. Independent clean reproduction:

cargo fmt --all --check -> rc 0
cargo clippy --workspace --all-targets --all-features -- -D warnings
error: this function has too many arguments (10/7) -> rc 101

Author action: consolidate scenario inputs into a fixture/options struct (or otherwise satisfy the existing lint policy), then rerun Rust Lint and Windows Rust.

Verification owner: author/CI for the fix; reviewer confirms exact-head gate state.

Verification at this head

• cargo test -p buzz-acp: 856 library + 9 lifecycle tests passed on a clean tree.
• All three production-boundary mutations failed behaviorally as intended: 855 pass / 1 fail each.
• Adversarial coverage held across normal/setup DM owner, sibling, external allowlist, Anyone, Nobody, forged/malformed provenance, and denial-without-side-effect paths.
• Confidence gap only: no fresh live relay + not-ready Desktop workflow journey was run. This does not create extra author action beyond fixing the directly established attribution defect; reviewer/tooling owns live UX verification afterward.

🤖 Combined review from my agents — two independent source passes at exact head bcf4f316b5acff5a079f9d004b92a3993e96fccb (delta from previously cleared 7616bd7f).

Verdict: REQUEST CHANGES — two blocking findings. Both lanes converged independently on each.

Delta verified as exactly two commits: 9f19e893c (crates/buzz-acp/src/lib.rs + setup_mode.rs) and merge bcf4f316, whose tree is byte-identical to Git's automatic merge of its parents — the merge contributes nothing unique, so this review covers 9f19e893c.

Blocking

IMPORTANT — the capability pattern narrows but does not structurally close the open P1. AuthorizedListenerEvent itself is well built: private fields, no Default/From/deserialize/test constructor, only construction site is after decision.allowed inside the private inbound_author_gate module, and both shipped loops currently use it correctly. But the claimed property — "replacing the call with a raw signer or a local allowed = true no longer type-checks" — does not hold at the loop boundary:

• BuzzEvent is Clone. A loop can clone the raw event before the gate call, invoke authorize_normal_listener_event(...) and consume the returned capability purely to satisfy the spelling, then filter/queue the raw clone with its raw signer. That compiles.
• The new production_listener_loops_consume_authorized_event_capabilities test counts symbol occurrences between comment markers via include_str! — the bypass above retains each counted string exactly once, so the test stays green while denied events proceed. It proves lexical presence, not data/control dependence.
• Normal mode unwraps the capability immediately into raw (BuzzEvent, String), and queue.push(QueuedEvent { ... }) accepts the raw event; setup mode's publish_setup_nudge(...) is likewise still callable with raw event data from its own module. evaluate_listener_event also remains pub(crate) alongside the new authorize_listener_event, so a decision-shaped bypass still compiles outside the marked regions.

Risk: a compiling production-loop bypass of workflow attribution, DM hardening, and configured author policy can still coexist with a green suite — the exact defect class the open P1 requires eliminating. Corrective direction: make the downstream ingress capability-only — normal-mode filter/queue/steer ingress consumes an authorized type without handing raw event data back to the loop; setup-mode filter/dedup/publish reachable only through a capability-consuming sink. Then the source-text symbol-count test can be dropped in favor of type-enforced data flow.

IMPORTANT — both required Rust CI jobs fail at head on this delta. Rust Lint and Windows Rust (x86_64-pc-windows-msvc) fail deterministically at compile stage: clippy::too_many_arguments (10/7, denied under -D warnings) on the delta-expanded test helper listener_boundary_scenario at crates/buzz-acp/src/lib.rs:6171 — this commit grew its signature from 7 to 10 args. Introduced by this delta, not main. Group the scenario inputs into an options/fixture struct (which would also make the DM cases more readable), or carry an explicitly justified test-only allowance.

What is good in this delta

• The capability type and its construction discipline are the right shape; both listeners were converted symmetrically and the setup-mode refactor is behavior-preserving (gate → filter → dedup ordering, deny logging, DM semantics, generation-aware NIP-11 refresh all unchanged).
• production_listener_boundaries_enforce_dm_author_policy now drives DM owner/sibling/external-allowlist/Anyone-stranger/Nobody policy through BOTH production boundary callables — this closes the previously flagged DM behavioral-coverage gap.

No additional policy or ordering regression found; current production paths are behaviorally sound as shipped.

🤖 Combined review from my agents — re-review at exact head 22ba7454a603f1e47950737b37a03ee38abc6da7 (delta from bcf4f316: single commit 22ba7454, base 8dbc65d9e), following my prior REQUEST_CHANGES (5056506006).

Verdict: REVIEW CLEAR — all blocking findings from the prior round are addressed at this head.

Prior-blocker adjudication

1. Capability-only downstream ingress — addressed. The normal loop now receives event data exclusively through the authorized pipeline: AuthorizedNormalListenerEvent::match_subscription → NormalListenerIngress::push → QueuedNormalListenerEvent::{mark_seen, steer_or_interrupt}. The loop never recovers raw (BuzzEvent, String) parts, and the verified effective_author principal flows into the mode gate through the queued capability. The include_str! symbol-count test and its comment markers were deleted in favor of this type-enforced data flow — exactly the corrective direction from the prior review. Any single-point rewire (raw signer, local allowed = true, dropped gate call) no longer type-checks.
2. Required Rust CI red — addressed. listener_boundary_scenario now takes one ListenerBoundaryScenario struct. Rust Lint and Windows Rust (x86_64-pc-windows-msvc) are green at this exact head; all required checks pass (run 33257882588).
3. Setup nudge misattribution (jedwards27's Medium, 5056456206) — addressed. nudge_authorized_event now consumes into_parts() and threads effective_author into publish_setup_nudge(..., recipient_hex, ...), so the nudge p-tags the verified workflow owner rather than the relay signer. New behavioral test authorized_workflow_nudge_mentions_effective_owner_not_relay_signer drives the production callable and asserts owner present / relay signer absent. Ordinary and forged events retain raw-signer attribution via the shared fallback in effective_prompt_author. Live E2E at this head confirmed the shipped behavior: a relay-signed workflow event to a setup-mode agent produced a nudge p-tagging exactly the effective owner (relay signer absent), threaded flat to the conversation root; the normal loop's queue/👀/interrupt path was also re-verified live.

Behavior equivalence

The loop refactor preserves ordering and conditions: subscription match precedes queue insertion; accepted gates both the 👀 reaction and mid-turn signaling (so DedupMode::Drop produces neither); in-flight check follows push; native steer attempted only for Steer with the same universal cancel+merge fallback. No regression found by either source pass.

Non-blocking (recorded for follow-up)

MINOR — the shipped loops themselves are still not behaviorally test-exercised. tokio_main and run_setup_listener are reachable only from main/startup; tests exercise the authorize/nudge callables directly. Since BuzzEvent remains Clone and the loops share modules with the ingress types, a deliberate multi-point adversarial rewrite (clone raw pre-gate, hand-construct QueuedEvent / call publish_setup_nudge directly) would still compile with a green suite. One of my two source lanes weighs this as blocking; I am recording it as a follow-up rather than re-blocking because the accidental-regression class behind the original P1 is now closed by construction, and the remaining seam requires a behavioral injection harness around each event pump — architecture-scale work beyond this PR. Recommended follow-up: a harness that injects relay events into each shipped loop and asserts denied events produce no queue/publish side effects and authorized workflow events preserve owner attribution.

Verdict: APPROVE
Reviewed: 8dbc65d9e2c80d9d8516e17b751c46e0568100e6..22ba7454a603f1e47950737b37a03ee38abc6da7 (exact head 22ba7454a603f1e47950737b37a03ee38abc6da7)
Risk: high — relay-signed workflow provenance crosses relay → ACP authorization and setup/normal listener boundaries.

Behavior/contracts traced: stored-template mention authority versus trigger-rendered routing; canonical provenance uniqueness and relay signature/current NIP-11 identity; effective-author capability flow through normal and setup listeners; OwnerOnly/Allowlist/Anyone/Nobody and DM policy; reconnect/key rotation, transient fetch failure, mixed-version fail-closed behavior; CI selection and required gates.

Findings: no unresolved blocking defect. The prior setup-attribution defect is fixed: setup preserves the verified effective_author and p-tags that principal rather than the raw relay signer (crates/buzz-acp/src/lib.rs:349-365; crates/buzz-acp/src/setup_mode.rs:469-496,630-673). The production-path regression at setup_mode.rs:741-821 fails causally when the raw signer is reintroduced. The prior Clippy failure is fixed by ListenerBoundaryScenario (lib.rs:6202-6229), and workspace Clippy plus hosted Rust Lint/Windows Rust pass.

Stored explicit mentions alone mint workflow-mention authority; trigger-substituted mentions retain ordinary routing without authority (crates/buzz-relay/src/workflow_sink.rs:153-184,354-390,899-1048). ACP requires kind 9, unique canonical workflow/owner/target metadata, valid active-relay signature, and the receiving agent's exact mention; forged, tampered, malformed, duplicate, wrong-kind, wrong-relay, and mixed-version cases fall back to raw-signer policy (crates/buzz-acp/src/lib.rs:236-307,5920-6067).

Author action: none.
Verification owner: reviewer/tooling for any fresh deployed relay + online/setup ACP workflow receipt; existing mesh-demo test owner/CI maintainers for the unrelated local 504 sensitivity.

Validation: exact-head cargo test -p buzz-acp passed 856 library + 9 lifecycle tests; cargo test -p buzz-workflow passed 169 with 2 unrelated PostgreSQL tests ignored; workflow-sink PostgreSQL integration passed 4/4; cargo clippy --workspace --all-targets --all-features -- -D warnings passed. Exact-head hosted required checks are green, including Rust Lint, Unit Tests, Backend Integration, Relay E2E, Desktop, Security, Windows Rust, and DCO. The setup-recipient raw-signer mutation failed the intended regression.

Manual/native evidence: no deployed production receipt was run in this review. The PR body records an isolated exact-head local relay + ACP five-case process matrix; independent review validated source, package/integration tests, mutation behavior, and hosted gates.

Residual risk: one local full buzz-relay run reached 997 passes but repeatedly hit an unchanged mesh-demo 504-vs-200 test; exact-head Backend Integration and Relay E2E are green, and the PR does not modify that test. A paired production deployment receipt remains operational confidence work, not author rework.

— :bot: Jude’s code review agent

:bot: Jude’s code review agent — APPROVE at exact head 22ba7454a603f1e47950737b37a03ee38abc6da7.

Both prior blockers are resolved, and the changed head preserves the intended narrow authority boundary.

Resolved blockers

• Setup recipient attribution: AuthorizedListenerEvent::into_parts() now carries the verified effective_author through setup processing (crates/buzz-acp/src/lib.rs:349-365; crates/buzz-acp/src/setup_mode.rs:467-507). publish_setup_nudge p-tags that principal rather than the relay signer (setup_mode.rs:630-682). The production-path regression asserts workflow owner present and relay signer absent (setup_mode.rs:742-822); independently reintroducing the raw-signer recipient made that exact regression fail.
• Workspace lint: the former ten-argument helper is now represented by ListenerBoundaryScenario (lib.rs:6202-6229). cargo clippy --workspace --all-targets --all-features -- -D warnings passes, as do exact-head Rust Lint and Windows Rust CI.

Integrated security and behavior review

• Stored explicit mentions and trigger-substituted mentions remain separated: legacy rendered p routing does not itself grant workflow authority; canonical buzz:workflow-mention provenance is emitted only for targets resolved from the stored owner-authored template (crates/buzz-relay/src/workflow_sink.rs:153-184,354-390). PostgreSQL integration coverage proves explicit authored mention versus trigger-substituted mention behavior (workflow_sink.rs:899-1048).
• ACP delegation requires kind 9, unique canonical marker/owner/target provenance, the active NIP-11 relay self signer, and an exact target match. Forged, malformed, duplicated, wrong-relay, wrong-kind, and unmatched cases fall back to raw-signer policy (crates/buzz-acp/src/lib.rs:236-307,5920-6067).
• The verified principal remains capability-bound through normal and setup production listeners. OwnerOnly/Allowlist/Anyone/Nobody and stricter DM owner/sibling policy are exercised at both boundaries (lib.rs:367-393,6308-6528). Startup failure, reconnect refresh, identity rotation/retry, stale-key eviction, and mixed-version fail-closed behavior are covered (lib.rs:6491-6529,6599-6942).

Exact-head validation

• cargo test -p buzz-acp: 856 library + 9 lifecycle/integration tests passed on a clean tree.
• cargo test -p buzz-workflow: 169 passed, 2 PostgreSQL tests ignored in the ordinary run.
• PostgreSQL workflow-sink integration: 4/4 passed, including durable authored-vs-trigger provenance.
• cargo clippy --workspace --all-targets --all-features -- -D warnings: passed.
• Exact-head required GitHub checks are all complete and passing.

Confidence gaps, not author action: one concurrent ACP package run showed two timing-sensitive failures that passed exact reruns and the isolated full package rerun. A local full buzz-relay run repeatedly hit unchanged api::mesh_demo::tests::demo_join_forwarded_arm_round_trips_echo with 504 versus 200, while exact-head Backend Integration and Relay E2E CI pass. Those remain with test/CI maintainers if further diagnosis is desired. A fresh deployed relay + online/setup ACP workflow receipt was not required to establish this code-level verdict and remains reviewer/tooling-owned follow-up.

Author action: none.