fix(acp): wake agents from workflow messages

[wesbillman]

Pinky, an AI agent, is opening this PR on Wes's behalf.

Summary

Workflow-generated messages can contain a valid agent mention but still fail the ACP inbound author gate because the relay signs the event. This keeps the existing wake policy and gives ACP a narrowly verified effective author:

• preserve the workflow owner's existing p tag and all rendered-mention p tags
• add explicit ["buzz:workflow-owner", <owner hex>] provenance to relay-generated workflow messages
• add ["buzz:workflow-mention", <agent hex>] authority only for mentions resolved from the stored, unrendered workflow step template
• accept that owner only for a verified kind-9 event signed by the relay's current NIP-11 self key, with unique canonical workflow metadata and an explicit workflow mention for the receiving agent
• route the verified owner through the existing author and in-flight mode policies in both normal and setup listeners
• refresh relay identity after reconnects, retaining the last verified key on transient fetch errors while treating a successful response without self as definitive removal

Malformed, duplicate, forged, tampered, wrong-kind, and wrong-relay attribution all fail closed to the raw event signer. respond-to=nobody remains absolute. Old/mixed-version messages without the explicit provenance retain their current fail-closed behavior.

Trust boundary

The workflow owner means “scheduled by,” not “authored every rendered word.” Trigger-controlled substitutions may still produce ordinary p mention routing for compatibility, but they cannot mint buzz:workflow-mention authority. Only a target named in the durable owner-authored step template can receive that authority.

The author gate is not bypassed: after relay signature/provenance verification, the effective owner is evaluated under the same owner-only, allowlist, DM, and nobody policies used for ordinary messages. Owner control commands continue to use the raw event signer.

Why this PR

This is the focused immediate fix for waking an online agent from a stored workflow mention. Earlier attempts were not a finished mergeable fix and had materially different or incomplete trust designs. Larry's larger draft stack addresses durable delivery across restarts; that remains valuable future work and can supersede this effective-author path when it lands.

Validation

At exact clean commit fe5b55619fe44176343eefb4cb7fe180df45a7d8:

• buzz-relay workflow_sink: 25/25 passed, including all four ignored PostgreSQL cases
• buzz-acp --lib: 845/845 passed
• buzz-workflow --lib: 169/169 passed (2 unrelated PostgreSQL tests ignored)
• warnings-denied Clippy passed for the changed Rust packages
• cargo fmt --all -- --check passed
• git diff --check passed
• repository pre-push gates passed, including branch-scoped Rust tests
• CI now selects the ACP library tests and the relay's pure + PostgreSQL workflow-sink tests so these guards cannot silently remain unexecuted

The production event-to-author gate is shared by normal and setup listeners and has biting regression tests for accepted explicit attribution, legacy owner-p rejection, and forged-attribution rejection.

Exact-head local relay + ACP proof

Following the release-binary/local-relay shape in TESTING.md, the exact commit above passed a fresh isolated real-process matrix using:

• a freshly recreated Postgres database with migrations
• isolated Redis
• exact-head release buzz-relay, buzz, buzz-admin, and buzz-acp binaries
• newly provisioned owner, channel, and bot member through the CLI
• workflow creation and triggering through the running relay
• a deterministic ACP protocol subprocess capturing actual session/prompt dispatches
• a NIP-11 self value verified against the running relay signer

Cases:

1. A stored explicit workflow mention woke an owner-only agent exactly once.
2. A workflow message without an agent mention did not wake it.
3. A non-relay signer forging every workflow authority tag did not wake it.
4. Trigger-controlled {{trigger.text}} containing @Wake Agent retained ordinary p routing but received no authority-bearing workflow-mention tag and did not wake the agent.
5. respond-to=nobody remained absolute for a valid relay-authenticated workflow mention.

The deterministic ACP subprocess isolates and directly proves relay → ACP authorization and prompt dispatch without depending on external model behavior.

Deployment and residual risk

Relay and ACP changes must be deployed together for the new wake behavior; mixed versions fail closed. Production paired-deployment proof remains distinct from the successful local integration run. Setup-mode behavior has automated coverage but was not a separate case in the five-case local matrix. Relay-key rotation is observed at ACP startup/reconnect; transient NIP-11 errors retain the last verified key, an intentional availability tradeoff documented in code.

[github-actions]

🔐 Codex Security Review

Status: review required for the current range.

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

[jedwards27]

: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.

[wpfleger96]

🤖 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.

[wpfleger96]

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.

[jedwards27]

: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.

[jedwards27]

: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.

[wesbillman]

Posted by Brain (an agent) on behalf of Wes (wesbillman) — findings and code are mine, the account is his. An earlier revision of this comment mis-tagged an unrelated user; apologies for the stray notification.

@jedwards27 Your P2 is valid and now fixed at 0cc47e0921925d9f1333150bf88e829dd9565f26. I reproduced your mutation independently before changing anything:

	cargo test -p buzz-acp --lib
clean fe5b55619	845 passed
relay_self: None at both listeners (lib.rs:3039, setup_mode.rs:438)	845 passed — unchanged

So the PR's headline behavior could be deleted at both production call sites with no test noticing. I had previously rated this survivor "availability-only, accept as-is" in my own review. That was the wrong call: fails-closed speaks to severity, your objection was about coverage, and a surviving mutation at a shipped call site is an uncovered seam regardless of blast radius. Thank you for holding the line on it.

Fix: delete the mutable argument rather than test around it

Adding a listener harness would have tested a copy of the wiring, not the wiring. Instead the relay identity is no longer a per-event parameter. InboundAuthorGate owns it:

• connect() — loads it via the real refresh_relay_self when the listener connects
• refresh() — re-reads it on reconnect, retaining the last verified key on transient failure
• evaluate() — takes no relay identity argument

Both listeners construct once and call evaluate per event. Your exact mutation is no longer expressible at the per-event call site; there is nothing there to set to None.

The seam is now covered — verified by mutation, not by reading

Three regressions in author_gate_tests build the gate through the same constructor the listeners use, against a stub NIP-11 document served over loopback, then evaluate a genuine relay-signed workflow dispatch. Re-run against the new head:

Mutation	Result
N-1 — drop the identity load from connect()	KILLED (test_connected_gate_wakes_owner_only_agent_for_relay_signed_workflow)
N-2 — evaluate() ignores the loaded identity	KILLED (2 tests)
N-3 — refresh() no-ops on reconnect	KILLED (test_gate_refresh_arms_attribution_after_reconnect)

The fail-closed direction is asserted too: a NIP-11 document with no self must fall back to the raw signer and not wake an owner-only agent, so the fix cannot be "achieved" by trusting everything.

Validation at the new head: cargo test -p buzz-acp → 848 lib + 9 lifecycle, 0 failed (845 + 3 new); cargo clippy -p buzz-acp --all-targets -D warnings clean; cargo fmt clean; no base skew (merge-base be8d7756d, nothing on main since touches these crates); all mutations reverted and md5-verified. No production behavior change — pure restructuring plus tests.

One residual seam I want to name rather than paper over

A listener that never constructs the gate at all is still not unit-catchable, because both listeners live inside a ~900-line tokio_main select loop and extracting that is scope creep inside a security fix. What changed is the shape of the risk: from two per-event arguments, each independently droppable to one construction site adjacent to a covered refresh. If you consider that remaining gap blocking, say so and I'll take the extraction as a follow-up rather than argue it — but I don't think it belongs in this PR.

Your other two notes I agree are not author rework: the live workflow→relay→ACP run was exercised at fe5b55619 by the author, and paired rollout / key-rotation observation are deployment concerns, as you classified them.

[wesbillman]

Posted by Brain (an agent) on behalf of Wes (wesbillman) — the account is his, the finding and the action below are mine.

The automated Codex security review has never actually run on this PR.

All three Codex Security Review workflow runs for this branch ended with Run Codex Security Review = skipped:

Run	Head	Authorize Security Review result
33121105302	fe5b55619	skipped
33132092420	fe5b55619	skipped
33139282861	0cc47e092	skipped

The authorize job log for the current head says:

Pull request #6953 requires authorization from a Block organization member.

That is the author_association defect #6952 is fixing: prepare() gates the pull_request_target path on pullRequest.author_association (.github/scripts/codex-security-review.js:365-373), and the Actions token reports it incorrectly for this member-authored PR. The REST API returns author_association: MEMBER for #6953, and GET /orgs/block/members/wesbillman returns 204, so the membership is real — the workflow just cannot see it.

Consequence: the 🔐 comment on this PR is the "review required for the current range" placeholder, not a completed review. No Codex security review exists for any head of this branch. This workflow is advisory by design and is not in the ruleset's required contexts, so it is not what is blocking the merge — but this PR changes an authorization boundary, so it is the one review class worth having before merge rather than after.

The issue_comment authorization path is unaffected by the defect (it checks the comment's own association, not the PR author's), so I am authorizing the review for the exact current head in the next comment. That command has to be an exact-match string, which is why this disclosure is a separate comment.

Nothing about the code changed with this finding: head is still 0cc47e0921925d9f1333150bf88e829dd9565f26, CI is 30/30 green at that head, and main has moved 6 commits since the merge-base without touching crates/buzz-acp, crates/buzz-relay, crates/buzz-workflow, or .github/workflows/ci.yml.

[wesbillman]

@buzz-security-review 0cc47e0

[wesbillman]

Posted by Brain (an agent) on behalf of Wes (wesbillman) — the account is his, the findings and actions are mine.

Update: the review I authorized was cancelled at the 30-minute job timeout — and this is not specific to this PR. The Codex security review is currently failing repo-wide.

My authorization worked: run 33144988481 is the first time Run Codex Security Review has ever started on this branch. It then ran 35 minutes and was killed by timeout-minutes: 30. The step that never finished is Review pull request (the openai/codex-action invocation); every step before it succeeded, so checkout, range pinning, and the API-key gate are all fine.

Every Codex Security Review run in the repo since #6962 merged (80177e4c8, 2026-08-27T22:01:43Z) — 87 runs:

Run Codex Security Review	Count
skipped (not authorized)	76
cancelled at 35 min	10
success	1

Nine of those ten cancellations are unrelated PRs — team sharing, NIP-98 admin auth, canvas versioning, DatabricksAuthCoordinator, effort wrappers. The single success (23:59Z, "lift right auxiliary pane") finished in 3 minutes, and the one before it took 12. So the pattern is bimodal: the review either returns quickly or hangs until the timeout kills it.

Worth stating plainly: this run used codex-version: '0.150.1' from main, not the 0.149.0 pinned on this PR's branch. pull_request_target loads the workflow from the base branch, and the run's head_sha is b49675894 (current main). #6962 bumped the CLI to 0.150.1 specifically to "unhang security review jobs" — that bump is live and jobs are still hanging. So #6962 did not fully resolve the hang, which is information for whoever owns that fix rather than anything about this PR's diff. Also note the concurrency group is codex-security-review-<pr> with cancel-in-progress: true, but these are single runs per PR reaching 35 minutes, not runs cancelling each other.

What this means for #6953: unchanged. That workflow is advisory by design — its own comment says its skipped jobs must not be required contexts, and main's ruleset does not list it among the 14 required checks. The 30 required checks are green at 0cc47e092. I have no automated security-review verdict to offer, and given the repo-wide hang I do not expect to get one tonight; I am not going to keep re-triggering a job that is timing out for everyone.

What still stands as evidence for this PR is the mutation work at this exact head, which is in the PR comments above: the listener→gate identity seam that earlier reviews correctly flagged is now covered by three tests built through the real constructor, and all three ways to break it (drop the load from connect(), ignore the identity in evaluate(), no-op the reconnect refresh()) fail a test. cargo test -p buzz-acp = 848 lib + 9 lifecycle, 0 failed, and I confirmed those three tests execute in the CI Unit Tests job rather than merely compiling.

One correction to something I implied earlier: require_last_push_approval: true is set on main, and the last push (0cc47e092) is mine. So Will's 22:31Z approval on fe5b55619 cannot satisfy the gate even though dismiss_stale_reviews_on_push is false. This needs one fresh approval at 0cc47e092.

[wesbillman]

Posted by Brain (an agent) on behalf of Wes (wesbillman) — the account is his, the findings and actions are mine.

Follow-up filed, and two deployment preconditions verified. No code change; head is still 0cc47e0921925d9f1333150bf88e829dd9565f26.

1. The re-trigger cycle is now tracked, not just noted

#6975. It is pre-existing (the relay's loop fence covers the workflow's own relay-signed output, not an agent's reply to that output), and it is a runaway-execution concern rather than an authorization one — the loop runs wholly as the owner's own authority. Splitting it out keeps an engine-level breaker from being bolted onto an identity-seam fix.

2. Two of the residual risks named in review are now closed

Both were listed as deployment/integration gaps. I checked them rather than leaving them open:

The production relay actually publishes NIP-11 self. This is the precondition the whole delegation path depends on — without it every workflow wake falls back to the raw signer and fails closed, meaning a paired rollout would be silently inert rather than wrong:

GET https://buzz.block.builderlab.xyz  (Accept: application/nostr+json)
  self -> 76aa9daa7edc9c2e1dc6a2a90dbce10c1bd52f060fa6506ba7bad4f50240fc01

RestClient::relay_self() reads exactly that key (/ then /info), re-normalizes it through PublicKey::from_hex, and treats a document without self as Ok(None) → fail closed. So the live document satisfies the contract as written.

Every reconnect path re-arms attribution, including the ones the listeners don't drive. InboundAuthorGate::refresh is only called at the two relay.reconnect() sites, which raised the obvious question of whether the relay task's autonomous reconnects bypass it. They don't: all six autonomous-reconnect paths in relay.rs emit an event_tx.try_send(None) sentinel first, and None from the event stream is precisely what drives the listener's reconnect arm — so an autonomous reconnect still lands on refresh. There is no reconnect path that resumes with a stale identity and no refresh.

State at this head

• 30/30 required checks green; the three new gate tests confirmed executing in the Unit Tests job (848 total), not merely compiling.
• No base skew: main has moved 6 commits since the merge-base, none touching buzz-acp, buzz-relay, buzz-workflow, or ci.yml.
• reviewDecision is still CHANGES_REQUESTED. Worth being precise about what is needed: main sets require_last_push_approval: true, and the last push is mine, so the existing approval cannot satisfy the gate even though it was never dismissed. This needs one fresh approval at 0cc47e092 — not a merge click.

The automated security review still has not produced a verdict on this PR; it is advisory and not a required context, and it is currently timing out repo-wide (10 of 87 runs cancelled at the 30-minute job limit), so re-triggering it would just burn CI. Details in my earlier comment.

[wesbillman]

Posted by Brain (an agent) on behalf of Wes (wesbillman) — the account is his, the findings and the code are mine.

Jude's P2 was only half-fixed at 0cc47e092. Now closed at 56e7e0dcb.

Re-reading the review rather than assuming my previous commit satisfied it: the requested action was a regression that bites if either listener's "relay identity input or gate invocation" is replaced with absent attribution. 0cc47e092 closed the first clause by moving the identity into InboundAuthorGate. The second clause was still open, and I verified that by mutation rather than by reading:

// crates/buzz-acp/src/lib.rs — normal listener, at 0cc47e092
let author_gate = InboundAuthorGate {
    agent_pubkey_hex: pubkey_hex.clone(),
    relay_self: None,      // ← every delegated workflow wake silently off
}.evaluate(...)

cargo test -p buzz-acp --lib → 848 passed; 0 failed   ← unchanged. No signal.

Two things kept the seam alive: evaluate_inbound_author_gate(..., relay_self, ...) still existed as a free function in scope, and InboundAuthorGate's fields were visible crate-wide, so the struct literal above type-checked. Jude was right the first time and right about the part I under-read.

The fix: encapsulation, not another test

A test asserting "the listener passed the right thing" would itself be bypassable. Instead the bad state is now unrepresentable — InboundAuthorGate lives in its own module with private fields, and the gate body is inlined into evaluate, so connect (which loads the identity) is the only way to obtain one.

Mutation at 56e7e0dcb	Result
evaluate() ignores the loaded identity	KILLED (3 tests)
connect() drops the identity load	KILLED (2 tests)
reconnect refresh no-ops	KILLED (1 test)
normal listener bypasses the gate (Jude's mutation)	COMPILE ERROR — E0451
setup listener bypasses the gate	COMPILE ERROR — E0451

error[E0451]: fields `agent_pubkey_hex` and `relay_self` of struct
              `inbound_author_gate::InboundAuthorGate` are private

The three regressions now build their gate through the real connect path against a stub NIP-11 server, so the identity load is inside what they cover.

No production behavior change — same attribution, same fail-closed fallback to the raw signer, same refresh points. cargo test -p buzz-acp: 848 lib + 9 lifecycle, 0 failed; clippy -D warnings and cargo fmt --all --check clean; parent is exactly 0cc47e092, no skew.

Residual seam, stated plainly

A listener that never constructs the gate at all is still not unit-catchable — both listeners live inside a ~900-line tokio_main select loop, and extracting that is scope creep on a security fix. The seam is now "one construction site adjacent to a covered refresh," and the two per-event bypasses that did exist are compile errors. If a reviewer considers the extraction blocking, I'll do it as a follow-up.

One unrelated note for whoever watches flakes

The pre-push hook failed once on buzz-desktop's test_probe_node_descendant_holds_stdout_returns_promptly_and_kills_group, which my commit cannot affect (it touches one file in buzz-acp). I verified it rather than dismissing it: passes standalone at both 0cc47e092 and 56e7e0dcb, and the full 2777-test desktop suite passes at both heads. It spawns a real process under a 3s timeout, so it's load-sensitive under the parallel pre-push lanes. Genuinely flaky, not a regression — worth a separate issue.

@jedwards27 — the seam you identified is closed at 56e7e0dcb, including the half I had missed. Since require_last_push_approval is enabled and the last push is mine, this needs a fresh review at that exact head; all five prior verdicts predate it.

[jedwards27]

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.

[wesbillman]

Fixed in 2884cb9855ed56c0664cd7bf889077fa898d8073.

Both production listeners now call InboundAuthorGate::evaluate_listener_event, which owns channel classification, trusted workflow attribution, and author policy as one boundary. The raw author_allowed policy is private to that module, so rewiring either listener to the former raw-signer path is no longer expressible. The owner-only workflow regression now drives this exact production callable with stream metadata and a live NIP-11 identity.

Validation on the pushed head: cargo test -p buzz-acp passed 848 library + 9 lifecycle tests; focused author-gate tests passed 21/21; formatting and diff checks passed.

buzz-auto-pr-comments:v1 request_id=6489972d-fb28-43f6-8c1b-3b3bc118c509 head_sha=b30d5af0db1d8e04d65711800664f60146974a71 snapshot=eff46629bab808c39390ef86d9503239c19eebd2e288029fb9b500ed93876329

[wpfleger96]

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.

[wesbillman]

Fixed both Clippy failures in ad385be9c6c9f68c3f7341c137a658c141ec7c9e.

The listener gate now accepts the existing BuzzEvent envelope instead of separate event and channel arguments. This preserves the indivisible production boundary while satisfying Clippy's argument limit. Workspace Clippy, focused author-gate tests (21/21), formatting, hooks, remote SHA, and commit attribution all passed.

buzz-auto-pr-comments:v1 request_id=1344fc4c-f6cf-40c8-b534-36ca7fbb1f4f head_sha=2884cb9855ed56c0664cd7bf889077fa898d8073 snapshot=89ad661d11e570d8341fd2c933804589dcaa387fd2a081ff2bdf8873918811bd

[jedwards27]

: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.

[wesbillman]

Fixed in 7616bd7fb1848baaaec7f047049867029aa39af2.

The normal and setup loops now each use an extracted production listener callable. Regression tests drive both callables through trusted workflow-owner attribution, respond-to=nobody denial, and generation-zero relay-identity recovery. Independently replacing either callable's gate invocation with a compiling raw-signer/allowed: true decision now fails both boundary tests deterministically.

Validation on the pushed head: cargo test -p buzz-acp passed 854 library tests, 9 integration tests, and doc tests; the two focused boundary tests passed. Both independent compiling bypass mutations failed the new tests. Formatting, diff checks, pre-push hooks, remote SHA, signature, author, sign-off, and Carl co-author attribution were verified.

buzz-auto-pr-comments:v1 request_id=74f27373-c11b-49e8-a796-895ac69bc33e head_sha=2cc972e24b5e88abb776bf480680326e200695cf snapshot=56901ec9b3dc9b96d0e927a5cd749b335f498f8c400e89dbfe986ad94dc8fe40

[jedwards27]

: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.

[jedwards27]

: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.

[wpfleger96]

🤖 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.

[jedwards27]

: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.

[wesbillman]

Implemented the exact-head review requests.

• Both production listeners now consume the raw event into a private authorization capability before queueing or publishing, with source-seam regression checks on both loop edges.
• Added production-boundary DM policy coverage for external allowlist/Anyone denial, owner/sibling allowance, and absolute Nobody denial.
• Mutation checks: deleting either marked loop authorization edge fails the structural regression; disabling DM classification fails the behavioral regression.
• Validation: cargo fmt --check, cargo check -p buzz-acp, and cargo test -p buzz-acp (856 library + 9 lifecycle) pass. Pre-push Rust and Tauri lanes also passed after merging current main.

[jedwards27]

: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.

[wpfleger96]

🤖 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.

[wesbillman]

Implemented both exact-head review requests.

• Normal-mode subscription matching, queueing, seen reaction, and steer/interrupt handling now consume typed authorized ingress state instead of unpacking raw event data in the listener loop. The brittle source-marker test was removed.
• Setup nudges preserve the verified effective author and p-tag the workflow owner rather than the relay signer, with a regression through the setup production callable.
• Replaced the 10-argument test helper with a scenario struct, resolving the deterministic Rust Lint and Windows Rust Clippy failure.
• Validation at 22ba7454a603f1e47950737b37a03ee38abc6da7: cargo fmt --all -- --check, cargo check -p buzz-acp --all-targets, cargo clippy -p buzz-acp --all-targets --all-features -- -D warnings, and full cargo test -p buzz-acp passed (856 unit, 9 integration, and doc tests). Pre-push Rust/Tauri gates passed. Remote SHA, signature, Wes author/sign-off, and Carl co-author attribution were read back.

[wpfleger96]

🤖 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.

[jedwards27]

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

[jedwards27]

: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.