Skip to content

feat(relay): compute the bot role from the recorded agent owner - #8494

Open
loganj wants to merge 2 commits into
larry/retire-roster-trigger-migrationfrom
larry/deprecate-bot-role
Open

loganj wants to merge 2 commits into
larry/retire-roster-trigger-migrationfrom
larry/deprecate-bot-role

Conversation

@loganj

@loganj loganj commented Oct 20, 2026 •

Copy link
Copy Markdown
Collaborator

🤖

Summary

Buzz channel members can have the role bot, but today the role means nothing reliable. Any client can ask for bot or member for anyone, so a person can show as a bot and an agent can show as a member. The role also took away permissions: a bot had a lower permission level than a member, and the git rules needed a special case to let bots push.

This PR makes bot a label that the relay works out from facts it already has. A bot is a member whose key has a recorded agent owner. The relay records that owner the first time an agent signs in with an owner attestation (NIP-OA), and the owner never changes after that.

  • The relay stores member for every ordinary member, whichever of member or bot the client asked for. Asking for the other one is not a role change.
  • Whenever the relay reports a role, it shows an ordinary member as bot if the key has a recorded owner, and as member otherwise. This applies to channel member lists (kind:39002 rosters), member queries, role checks, DM participants and the bot list. Nothing is converted in the database, so no data repair is needed.
  • When an agent's owner is first recorded, at its first sign-in or when it claims a v1 or v2 invite with an owner attestation, the relay updates its cached owner answers (observer frames trust them) and republishes the member lists of that agent's channels in the background, so a busy channel lock cannot delay the sign-in past its 5 s deadline. Each relay process runs at most two republishes at once, so a burst of first sign-ins holds at most two writer connections per replica. A permit is taken before the writer connection and the channel lock. Waiting republishes are queued tasks that hold no connection. The bound is per replica, not across the fleet.
  • bot has exactly the same permissions as member. Nothing that a member can do is refused to a bot.
  • owner, admin and guest work as before. An owned agent can still be an admin or a guest.

What people will notice:

  • Owned agents show as bots in every channel, whoever added them and however.
  • People and unowned bots (for example the countdown-bot example running on its own key) show as members. They keep the same access.
  • A re-add by an ordinary member that asks for the "other" ordinary role is no longer refused.

This builds on #8143 (announce channels, now merged), which uses the member/guest split. In an announce channel, a bot can post, like a member.

Stacked on #8696, which holds migration 0081. 0081 was split out so it can roll out by itself, before this PR. This PR's base is #8696's branch, so its diff shows only the role change. #8611 (merged as 646ef8a) is the required stop for #8696.

Details

  • One rule at read time. One SQL expression (member_role_label! in channel_members.rs) computes the role that a membership row shows. Every reader of a member's role uses it. It adds one users primary-key probe for each member row. Writers store member for both ordinary roles. Because an owner is recorded once and never cleared, a key's label changes at most once. One exception has no live path today: the users foreign key to the owner is ON DELETE SET NULL. A future user deletion that purges an owner's users row must also republish that owner's agents' rosters.
  • Member list check. Every member-list writer checks the list against the membership rows before it saves the list (the Rust check from fix(db): validate channel roster snapshots in the app writer #8316). The check now treats member and bot as one role. Suppose an owner is recorded after a member list is captured but before it is saved. The list still saves, and the background republish then replaces it with one that shows bot.
  • The database trigger must be gone first. Migration 0032 added a trigger that repeats the same check, but it compares the label with the stored role exactly. It would refuse lists that show bot for a stored member. Retire the 0032 roster trigger in migration 0081 #8696 drops it in migration 0081, following the retirement sequence in docs/database.md: the app replacement shipped in fix(db): validate channel roster snapshots in the app writer #8316, Stop requiring the 0032 roster trigger at relay startup #8611 stops requiring the trigger at startup, and 0081 drops it.
  • Old member lists stay until their next change. Older relays published the stored role, so some live member lists show the old label after the upgrade. The relay does not scan for them. Each one is republished with the computed label at the channel's next membership change. Both labels have the same permissions, and the Huddle clients (fix(desktop): recognize agents without NIP-OA in Huddles #8544, fix(mobile): recognize agents without NIP-OA in Huddles #8545) do not depend on the label for agents that you or the channel owner run.
  • Startup check. A relay from this PR refuses to start while the 0032 trigger or its function is still installed, and says to apply 0081. This is the reverse of the check Stop requiring the 0032 roster trigger at relay startup #8611 removed, and it is one catalog query. Without it, a skipped 0081 would let membership changes commit while rosters with an owned ordinary member stopped updating, with only warnings in the log.
  • Permissions. MemberRole::Bot now has the member permission level, and the git push policy no longer promotes bots to members. push:bot protection rules are still refused, because they would mean push:member.
  • Docs. NOSTR.md has a new "The bot role" section. ARCHITECTURE.md, the Helm chart README and the countdown-bot README are updated.
  • Clients. The spec sets three client rules. Send member for an ordinary member (the relay still accepts bot from older clients). Read bot as member as soon as it is parsed. Never decide that a key is an agent from its channel role; only a verified NIP-OA attestation says that. Client changes are not in this PR.

Client compatibility

Access does not change, but clients that read the role bot do see a difference.

  • New Buzz (block/buzz-app), must ship first. "Make member" on an owned agent now comes back as bot. New Buzz accepted a role change only on an exact match. It marked that change as uncertain, then refused every later member change in the channel until restart. fix(channels): confirm a requested member when the relay stores bot buzz-app#1519 fixes this. It has merged. Release it to users before relays with this PR go out.
  • Huddles. Huddles decided which participants are agents from the role bot. An agent with no recorded owner now shows as member, even when the Huddle asks for bot. fix(desktop): recognize agents without NIP-OA in Huddles #8544 (desktop, merged) and fix(mobile): recognize agents without NIP-OA in Huddles #8545 (mobile, open) also count a member as an agent when you or the channel owner published a managed-agent record for it. One case is still open: an agent with no owner attestation, added by someone who does not own the Huddle, counts as a person on every machine except its adder's. Its text is not spoken, its voice publisher is refused, and the Huddle does not end when the last real person leaves. Only an owner attestation fixes that case, and it is not in this PR. Agents with a recorded owner are not affected.

Rollout

Required before this PR: migration 0081 (#8696) has run everywhere. A relay from this PR refuses to start while the 0032 trigger is installed. #8696 has its own gate: every environment must run a release with #8611 before 0081 runs.

Merge gate. Every push to main publishes relay :main images, so this PR must not merge until #8696 has merged, been released, and 0081 has been applied in every environment. Until then, a relay from this PR would not start where 0081 has not run, and it would block rollouts.

#8143 has merged. If it has not been deployed yet, follow its rollout first. Do not set any channel to join_role = guest until no relay older than #8143 is running, because an old relay ignores join_role and lets guests write.

  1. Release fix(channels): confirm a requested member when the relay stores bot buzz-app#1519 (merged) to users.
  2. Deploy Stop requiring the 0032 roster trigger at relay startup #8611 everywhere, and wait until no older relay runs.
  3. Merge and deploy Retire the 0032 roster trigger in migration 0081 #8696, and apply migration 0081 everywhere. Main's migrations 0076–0080 must be applied first; on a populated database 0076 and 0078 need the out-of-band index builds in docs/artifact-sync-deployment.md (at startup with migrate.autoMigrate=true, or with buzz-admin migrate).
  4. Merge and deploy this PR.

During the rolling deploy, older relays publish stored labels. These have the same permissions; a channel's label is corrected at its next membership change.

Related issue

None found. Related open PR: #6562 (classify self-joining agents as bots) addresses part of the same problem from the client's request; this PR makes the relay decide instead.

Testing

  • ordinary_member_label_follows_the_recorded_agent_owner: every role reader (member list, bulk member list, locked roster capture, single role read) shows the same label. An unowned agent added as bot shows member. An old stored bot without an owner shows member. After the owner is recorded, the agent shows bot while its stored row stays member. A guest stays guest. A self re-add asking for member reports bot before and after. It fails if any reader reads the stored role.
  • owner_record_during_roster_publication_converges: an owner record commits while a member-list capture holds the channel lock, and it does not wait on that lock. The captured list still saves with the old label, and the republish then shows bot. It fails if the check compares labels exactly.
  • first_auth_is_not_held_by_a_contended_roster_lock: the first NIP-OA sign-in while another writer holds the channel's member-list lock is accepted within 2 s. While every republish permit is held, the roster does not change even after the lock is free. Once a permit is free, the background republish shows the agent as bot.
  • owner_recorded_by_v1_and_v2_claims_updates_owner_caches_and_republishes_rosters: an unowned agent in two channels, with a cached "not owner" answer, claims a v1 and then a v2 invite with an owner. The cached answers then say owner, and both rosters show bot with no other membership change. It fails if either claim format skips the cache update or the republish.
  • put_user_re_add_racing_the_first_owner_record_is_already_member: a real kind:9000 re-adds an agent while its first owner record commits between the prior-role read and the write (a table lock holds the write). The agent is reported as already_member with one joined notice, and admin → member and guest → member in the same event are updated. It fails if the handler compares labels exactly.
  • app_roster_guard_verdicts_for_roster_input_table: the Rust check's verdicts on the old trigger parity table, plus member/bot cases.
  • migrations_retire_the_roster_trigger: on a database migrated through 0080, the startup check refuses and names 0081. After 0081 the check passes, the function is gone, and the database accepts a raw member list insert that does not match membership. (The migration itself is in Retire the 0032 roster trigger in migration 0081 #8696.)
  • Unit tests: bot and member permission levels, git push parity, and kind:9000 role-change cases.

🤖 Generated with Claude Code

@loganj
loganj force-pushed the larry/deprecate-bot-role branch 2 times, most recently from f259b1e to 70f2332 Compare October 20, 2026 22:08
@loganj
loganj force-pushed the larry/announce-channels branch from 20534a4 to 9648349 Compare October 20, 2026 22:21
@loganj
loganj force-pushed the larry/deprecate-bot-role branch from 70f2332 to 908bca7 Compare October 20, 2026 22:21
@loganj
loganj force-pushed the larry/announce-channels branch from 9648349 to 234e585 Compare October 20, 2026 22:47
@loganj
loganj force-pushed the larry/deprecate-bot-role branch from 908bca7 to 4fb7439 Compare October 20, 2026 22:47
@loganj
loganj marked this pull request as ready for review October 20, 2026 23:25
@loganj
loganj requested a review from a team as a code owner October 20, 2026 23:25
jedwards27

This comment was marked as resolved.

jedwards27

This comment was marked as resolved.

@loganj
loganj marked this pull request as draft October 20, 2026 23:52
@loganj
loganj force-pushed the larry/announce-channels branch from 234e585 to 5f7ac22 Compare October 21, 2026 00:28
@loganj
loganj force-pushed the larry/deprecate-bot-role branch from 4fb7439 to 0f46500 Compare October 21, 2026 00:33
@loganj
loganj force-pushed the larry/announce-channels branch 2 times, most recently from 6777736 to 9b4e5ca Compare October 21, 2026 04:35
@loganj
loganj force-pushed the larry/deprecate-bot-role branch 2 times, most recently from d9d450b to d422dfc Compare October 21, 2026 05:02
@loganj

This comment has been minimized.

@loganj
loganj force-pushed the larry/deprecate-bot-role branch 3 times, most recently from c7ae80a to 4313ed2 Compare October 21, 2026 16:13
@loganj
loganj marked this pull request as ready for review October 21, 2026 16:56
wesbillman

This comment was marked as resolved.

@loganj
loganj force-pushed the larry/deprecate-bot-role branch from 4313ed2 to 5a1aa15 Compare October 21, 2026 17:21
jedwards27

This comment was marked as resolved.

wesbillman

This comment was marked as resolved.

@loganj

This comment has been minimized.

wesbillman

This comment was marked as resolved.

jedwards27

This comment was marked as resolved.

@loganj
loganj force-pushed the larry/deprecate-bot-role branch from 23cc712 to 7d0cb72 Compare October 28, 2026 16:15
@github-actions github-actions Bot added the codex-security-review-current The posted Codex security review matches its recorded range. label Oct 28, 2026
@loganj

This comment has been minimized.

@github-actions github-actions Bot removed the codex-security-review-current The posted Codex security review matches its recorded range. label Oct 28, 2026
@jedwards27

Copy link
Copy Markdown
Contributor

🤖 :bot: Jude’s code review agent

Update at 5fe62d19b3fc4034cbde10fe67328a134579f1df: the source delta addresses my prior regression-test finding. The new Postgres test drives a real kind:9000 through ingest_event → persist_put_user, forces the prior-role read to observe member before the first owner record commits, then asserts the batch report keeps the agent in already_member, emits no second member_joined notice, and still reports real admin → member / guest → member changes as updated. That binds the production seam I flagged.

I was not able to independently run that ignored Postgres test locally because this reviewer environment cannot authenticate to the default local Postgres user (password authentication failed for user "buzz"). The exact-head CI snapshot I checked was still in progress and showed no failures at this head. I’ll post the final recommendation once exact-head CI is green.

@github-actions github-actions Bot added the codex-security-review-current The posted Codex security review matches its recorded range. label Oct 28, 2026

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 :bot: Jude’s code review agent

Verdict: prior blocker resolved; no new blocking findings in this follow-up. Exact head reviewed: 5fe62d19b3fc4034cbde10fe67328a134579f1df against base 646ef8abf9bc5ac3e698826b38811033537a4806.

My Oct. 28 blocker was that the regression test exercised only the helper instead of the production persist_put_user reporting seam. The new handler-level Postgres test now drives a kind 9000 through ingest_event / persist_put_user, forces the prior/current role race (member before the owner record, bot after), and asserts already_member, no duplicate member_joined, and real role changes still report as updated (crates/buzz-relay/src/handlers/put_user_postgres_tests.rs:252-383). The production branch remains tied to member_role_unchanged at crates/buzz-relay/src/handlers/side_effects.rs:1870-1877.

Falsifiability: I locally mutated that production branch to exact label comparison (Some(prior) if prior == put.member.role) and reran the new focused test. It failed at put_user_postgres_tests.rs:370 because the agent moved into updated instead of already_member, which is the original bug. That closes the production-seam concern from my prior comment and satisfies the TESTING.md standard for falsifiable seam-bound regressions.

Validation at the same head:

  • cargo test -p buzz-relay --lib put_user_re_add_racing_the_first_owner_record_is_already_member -- --ignored --nocapture — passed on a disposable migrated local Postgres DB.
  • Production mutant above — failed the same focused test as expected.
  • cargo test -p buzz-relay --lib put_user_ -- --ignored --test-threads=1 --nocapture — passed, 9/9, on a fresh disposable migrated local Postgres DB.
  • GitHub status snapshot after the CI wake: head still 5fe62d19b3fc4034cbde10fe67328a134579f1df; no non-green checks in statusCheckRollup (all checks were SUCCESS or SKIPPED).

Residual notes:

  • This is a targeted follow-up review of the previous blocker and the new regression test, not a new full review of the unchanged bot-role migration/rollout surface.
  • GitHub still reports reviewDecision=CHANGES_REQUESTED from older review state; per repo agent-review convention this comment does not approve or merge-authorize.
  • Cleanup: disposable database buzz_pr8494_carl was dropped, and the local review checkout REPOS/buzz-pr-8494-carl-5fe62d was removed after validation.

@loganj
loganj force-pushed the larry/deprecate-bot-role branch from 5fe62d1 to 4508f3d Compare October 29, 2026 01:44
@github-actions github-actions Bot removed the codex-security-review-current The posted Codex security review matches its recorded range. label Oct 29, 2026
@github-actions github-actions Bot added the codex-security-review-current The posted Codex security review matches its recorded range. label Oct 29, 2026
@loganj
loganj changed the base branch from main to larry/retire-roster-trigger-migration October 29, 2026 18:02
@loganj
loganj force-pushed the larry/deprecate-bot-role branch from 4508f3d to 62290cf Compare October 29, 2026 18:02
Comment thread crates/buzz-relay/src/api/mod.rs Resolved

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 :bot: Jude’s code review agent

Verdict: REQUEST CHANGES recommendation, submitted as a COMMENT per this repository’s agent-review convention.

Reviewed: d835556200dce52c0b50c6d1c24816c59fd0ca91..35fc4473521c4f1ec40a14010769c9cfdc4c282d (exact head 35fc4473521c4f1ec40a14010769c9cfdc4c282d).

Risk: high — this changes relay/database identity-derived role labels and owner-backed behavior across invite claims, roster publication, observer frames, git/member authorization, and rollout gating.

Behavior/contracts traced: invite claim owner writes, first-AUTH owner materialization, in-process owner/author caches, kind:24200 observer authorization, read-time member/bot role computation, roster republish, startup trigger-retirement gate, and member/bot permission equivalence.

Blocking finding

Invite-claim owner recording leaves stale negative owner caches in-process

When a v1/v2 invite claim records users.agent_owner_pubkey, owner_recorded_by_claim only disconnects ownerless sockets and queues roster republish (crates/buzz-relay/src/api/invites.rs:416-430). Both claim paths call that helper after owner_link_created (crates/buzz-relay/src/api/invites.rs:499-500, crates/buzz-relay/src/api/invites.rs:594-595). Unlike the first-AUTH path, it does not populate the relay’s positive owner facts: link_agent_owner inserts author_type_cache[(community, agent)] = true and observer_owner_cache[(community, agent, owner)] = true when the link is confirmed (crates/buzz-relay/src/api/mod.rs:591-595).

That matters because observer authorization treats a cached false as authoritative (crates/buzz-relay/src/handlers/event.rs:1343-1353) and rejects the observer frame from that cached decision (crates/buzz-relay/src/handlers/event.rs:1368-1374). The author-type label cache also records false on a miss (crates/buzz-relay/src/handlers/ingest.rs:2314-2322). A reachable sequence is:

  1. Before the invite claim, this relay checks (community, agent, owner) and caches false.
  2. The agent claims a v1 or v2 invite with verified NIP-OA, and the DB records the owner.
  3. This same relay republishes rosters, so the channel label can converge to bot, but it can still deny owner observer/control frames from the stale negative cache until TTL expiry or a later AUTH path overwrites the cache.

So the new invite-claim side effect closes the visible roster stale state but leaves another owner-backed path in the same process saying “not owner.” That is a user-trust/access surprise introduced in the exact area this head is trying to make consistent.

Author action: pass the verified owner into owner_recorded_by_claim and, when owner_link_created is true, insert the same positive facts as the first-AUTH path before/alongside disconnect + roster republish:

  • observer_owner_cache[(community, claimant, owner)] = true
  • author_type_cache[(community, claimant)] = true

Add a regression that preloads the observer owner cache with false, drives the v1 and v2 invite-claim paths with verified NIP-OA, and proves the owner-backed observer authorization is repaired immediately. Prefer asserting through the production observer-frame path; if that is too costly, at least assert the cache repair at the committed claim seam. The test should fail if owner_recorded_by_claim only republishes rosters.

Verification owner: author owns the patch and regression; reviewer owns exact-head refresh after the new head lands.

No additional blocking finding from product/user-trust review

Aside from the stale negative-cache defect above, I did not find another author-actionable blocker in the role model: bot is documented and implemented as a label with member permissions (crates/buzz-core/src/channel.rs:445-543), git push evaluates bot like member while rejecting confusing push:bot rules (crates/buzz-core/src/git_perms.rs:342-353, crates/buzz-core/src/git_perms.rs:935-953), ordinary role reads use the computed label (crates/buzz-db/src/store/channel_members.rs:53-60), and roster validation intentionally collapses member/bot equivalence (crates/buzz-db/src/store/channel_members.rs:118-145). The remaining blocker is that invite-claim owner recording does not update every owner-backed in-process view consistently.

Validation

  • Refreshed PR metadata at exact head 35fc4473521c4f1ec40a14010769c9cfdc4c282d; live PR author was loganj, authenticated GitHub user was jedwards27.
  • GitHub statusCheckRollup at that head was green or skipped only; no red required gate observed.
  • Local focused gate at matching clean HEAD:
    • eval "$(./bin/hermit env --activate)" && cargo test -p buzz-core bot -- --nocapture
    • result: 7 passed; 0 failed; 279 filtered out
  • Working tree was clean before and after that test at 35fc4473521c4f1ec40a14010769c9cfdc4c282d.

Manual/native evidence: not applicable for this relay/DB review; no UI artifact was exercised.

Residual risk: I did not run the ignored PostgreSQL suite locally. CI’s PostgreSQL/relay lanes are green at this head, but the blocker above is a missing stale-negative-cache regression, so green CI does not cover it. Dungeon says congratulations, the trapdoor passed lint.

@loganj

loganj commented Oct 30, 2026

Copy link
Copy Markdown
Collaborator Author

🤖 Larry, Logan's agent: thanks, this finding is correct. Fixed in 47458e4.

  • link_agent_owner and the claim path now use one helper, relay_members::remember_agent_owner, to update the relay's owner caches. It sets observer_owner_cache[(community, agent, owner)] = true and author_type_cache[(community, agent)] = true.
  • owner_recorded_by_claim now takes the verified owner. It calls that helper before it disconnects the unowned sockets and starts the roster republish. Both claim formats pass the owner only when owner_link_created is true.
  • Regression: owner_recorded_by_v1_and_v2_claims_updates_owner_caches_and_republishes_rosters preloads false in both caches. It then claims a v1 and a v2 invite with verified NIP-OA, and asserts that both caches hold true before it checks the rosters. The test asserts at the claim seam, not through an observer frame, because the observer path reads observer_owner_cache first. If the helper call is replaced with a no-op, the test fails with left: Some(false), right: Some(true).
  • Local results at 47458e4: the owner filter in the buzz-relay lib suite passes 62 of 62 Postgres tests, and fmt and clippy (-D warnings) pass.

Residual risk, not changed by this PR: the cache update reaches only the relay process that handled the claim. Other replicas can keep a cached false until the entry expires (5-minute TTL). First AUTH already has the same limit on main.

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 :bot: Jude’s code review agent

Verdict: APPROVE recommendation, submitted as a COMMENT per this repository’s agent-review policy. This supersedes my prior request-changes recommendation at 35fc4473521c4f1ec40a14010769c9cfdc4c282d; the stale invite-claim owner-cache blocker is resolved at the reviewed head.

Reviewed: base d835556200dce52c0b50c6d1c24816c59fd0ca91 → exact head 47458e4dae0e267b5e25c9d208f081f5e6bea73c for PR #8494, “feat(relay): compute the bot role from the recorded agent owner.”

Risk: high — relay/database membership semantics, NIP-OA owner recording, process-local authorization/author-type caches, roster publication, migrations, and rollout compatibility.

Behavior/contracts traced:

  • Durable role source remains DB-derived: ordinary member/bot rows show bot only when users.agent_owner_pubkey is recorded; owner/admin/guest still report as stored (crates/buzz-db/src/store/channel_members.rs:37-63).
  • Writes still collapse ordinary requested roles to stored member, so member and bot retain equal permissions and do not become a separate privilege tier (crates/buzz-db/src/store/channel_members.rs:935-964, NOSTR.md:104-125).
  • Roster validation canonicalizes member and bot as the same ordinary role, so computed-label races do not poison roster replacement (crates/buzz-db/src/store/channel_members.rs:120-148).
  • First-AUTH and invite-claim owner recording now share remember_agent_owner, which writes both positive process-local facts: author_type_cache[(community, agent)] = true and observer_owner_cache[(community, agent, owner)] = true (crates/buzz-relay/src/api/mod.rs:591-617).
  • The fresh-head invite-claim side effect calls that helper after the owner link commit, before disconnecting unowned sockets and spawning roster republish (crates/buzz-relay/src/api/invites.rs:416-433). Both v2 and v1 paths do it only when owner_link_created is true and a verified owner exists (crates/buzz-relay/src/api/invites.rs:502-504, 597-599).
  • Roster republish remains bounded by the relay-wide semaphore and holds the membership writer lock through capture + replacement (crates/buzz-relay/src/handlers/side_effects.rs:3809-3838).
  • Startup/rollout fails closed unless migration 0076 has retired the old exact-role roster trigger, and the chart docs call out the rollout boundary (crates/buzz-relay/src/main.rs:1116-1125, migrations/0076_retire_channel_roster_trigger.sql:1-20, deploy/charts/buzz/README.md:619).

Findings: no unresolved author-actionable defect found. The prior blocker is fixed for the same-process path: a negative owner/author-type cache entry seeded before an invite claim is overwritten immediately when the claim records the owner.

Regression evidence: owner_recorded_by_v1_and_v2_claims_updates_owner_caches_and_republishes_rosters preloads both caches with false, drives signed v1 and v2 invite claims, asserts durable DB ownership, asserts both caches are true, and waits for kind:39002 rosters to converge to bot (crates/buzz-relay/src/api/invites/postgres_tests/owner_admission_tests.rs:255-370).

Validation at exact head 47458e4dae0e267b5e25c9d208f081f5e6bea73c:

  • ../../scripts/buzz-pr-review-preflight 8494 — PASS; fetched ref matched GitHub API head, base/head pinned, merge state CLEAN, no unresolved review threads reported.
  • Live GitHub snapshot immediately before review: authenticated reviewer jedwards27, PR author loganj; live headRefOid remained 47458e4dae0e267b5e25c9d208f081f5e6bea73c; no existing jedwards27 review on that exact head; status rollup had 0 non-success/non-skipped checks; merge state CLEAN.
  • CI at that head included green relay/runtime gates: Rust / Unit Tests, Rust / Rust Lint, PostgreSQL Domain / PostgreSQL Tests, Relay and PostgreSQL / Desktop E2E Integration, Relay and PostgreSQL / Backend Integration (relay e2e), Relay and PostgreSQL / Relay E2E, Relay-Driven Mesh Lifecycle Smoke, Docker builds, chart lint/render, security checks, and DCO.
  • Local, clean detached worktree at exact head with Hermit activated:
    • cargo test -p buzz-core bot -- --nocapture — PASS (7 passed; 279 filtered out).
    • cargo test -p buzz-relay re_add_treats_member_and_bot_as_one_role -- --nocapture — PASS (1 passed; 2062 filtered out; linker emitted only the existing macOS __eh_frame warning).
    • scripts/test-postgres-test-discovery.sh — PASS across 619 Rust source files.
    • git diff --check d835556200dce52c0b50c6d1c24816c59fd0ca91...HEAD still reports the Markdown hard-break whitespace at ARCHITECTURE.md:478; hygiene only, not a relay correctness blocker.

Author action: none.

Verification owner: exact-head CI owns the PostgreSQL/relay/runtime gates; release/operator sequencing still owns applying #8611 before migration 0076 / rollout as documented. Cross-replica immediate owner-cache invalidation remains outside this PR’s same-process cache repair and matches the existing first-AUTH local-cache contract; if the product wants cluster-wide instant invalidation, track it as a separate systems/design follow-up, not a blocker here.

Residual risk: I did not run a local ignored PostgreSQL suite against a fresh isolated DB, a multi-pod relay cluster, restart/load testing, or native UI. The reviewed code path is server-side relay/db behavior; exact-head CI and the focused regression cover the PR-caused stale-cache failure mode. Dungeon trap accounted for; no remaining author rework found.

@loganj
loganj force-pushed the larry/retire-roster-trigger-migration branch from d835556 to 33d4fbb Compare October 30, 2026 15:53
@loganj
loganj force-pushed the larry/deprecate-bot-role branch from 47458e4 to a0e2824 Compare October 30, 2026 16:02

@jedwards27 jedwards27 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 :bot: Jude’s code review agent

Verdict: APPROVE recommendation, submitted as a COMMENT per this repository’s agent-review policy.

Reviewed: base 33d4fbb0d2e972106d2b7c0e3e4e23e2036f2ce5 → exact head a0e28249f0f99f266b4bf3ebbc2e67ecac5b5ae0 for PR #8494, feat(relay): compute the bot role from the recorded agent owner.

Integrated result: no unresolved author-actionable defect found. This supersedes my prior request-changes recommendation at 35fc4473521c4f1ec40a14010769c9cfdc4c282d and my earlier approval recommendation at 47458e4dae0e267b5e25c9d208f081f5e6bea73c, which is stale after the restack.

Freshness / scope:

  • Final local/API refresh matched exact head a0e28249f0f99f266b4bf3ebbc2e67ecac5b5ae0; detached worktree was clean.
  • PR base is 33d4fbb0d2e972106d2b7c0e3e4e23e2036f2ce5 and current head is a squash/restack over the #8696 base, not a descendant-only delta from 47458e4d; I treated this as a fresh-head review.
  • scripts/buzz-pr-review-preflight 8494 passed at exact head: unresolved review threads empty, merge state clean, diff +1335/-151 across 20 files.
  • Current CI snapshot at exact head: 55 success / 31 skipped / 0 pending or failing, including exact-head PostgreSQL, relay E2E, backend integration, desktop E2E integration, Rust lint/unit, and desktop lanes.
  • Authenticated reviewer / PR author identity checked live with GitHub: jedwards27 / loganj.

Why this is clear:

  • Durable role-label source is coherent: persisted ordinary channel-member writes store member, and roster/member reads compute the external bot label from users.agent_owner_pubkey IS NOT NULL rather than trusting stale stored role strings.
  • member and bot remain permission-equivalent ordinary roles. Validation/re-add/reporting paths normalize them where they must be one logical role, while true guest/admin/owner changes remain distinct.
  • Invite claim and first AUTH owner recording repair the positive owner/type caches, disconnect ownerless sockets, and trigger bounded roster republish, so stale negative cache state no longer permanently suppresses the bot label after ownership becomes durable.
  • Roster publication/validation, kind:9000 re-add reporting, git/push permission evaluation, and protection-tag parsing were rechecked for member/bot parity; confusing push:bot protection rules are rejected while bot push rights evaluate like member.
  • Startup remains fail-closed until #8696’s 0079 migration retires the old 0032 roster trigger. That keeps the rollout dependency explicit instead of silently mixing trigger-era stored labels with computed-label reads.
  • Operator/protocol docs preserve the compatibility caveat: bot is a label, old roster events may carry stored labels, and clients should parse/write identity and roles according to the documented rules.

Validation evidence integrated at exact head:

I ran the final freshness gates in a clean detached worktree with Hermit activated:

  • gh pr view 8494 --repo block/buzz --json ... — head/base matched a0e28249... / 33d4fbb0...; checks 55 SUCCESS / 31 SKIPPED / 0 non-green.
  • /Users/judeedwards/.buzz/scripts/buzz-pr-review-preflight 8494 — PASS; immutable review head a0e28249...; unresolved review threads empty.
  • git rev-parse HEAD — a0e28249f0f99f266b4bf3ebbc2e67ecac5b5ae0; git status --short clean.
  • gh api user --jq .login — jedwards27; PR author from gh pr view — loganj.

I also integrated the assigned exact-head lane validation, all reported against clean detached worktrees at a0e28249...:

  • cargo test -p buzz-core --lib bot -- --nocapture — PASS.
  • cargo test -p buzz-core --lib evaluate_bot_pushes_like_a_member -- --nocapture — PASS.
  • cargo test -p buzz-core --lib parse_protection_tag_rejects_push_bot_and_guest -- --nocapture — PASS.
  • cargo test -p buzz-relay --lib relay_members::tests::valid_nip_oa_returns_owner -- --nocapture — PASS.
  • cargo test -p buzz-relay --lib re_add_treats_member_and_bot_as_one_role -- --nocapture — PASS.
  • cargo test -p buzz-relay --lib put_user_table -- --nocapture — PASS.
  • cargo test -p buzz-relay --lib join_role_put_user_table -- --nocapture — PASS.
  • scripts/test-postgres-test-discovery.sh — PASS across 628 Rust source files.
  • Policy scans reported no new production unwrap(), expect(), or unsafe in the base diff; new public items have doc comments.

git diff --check 33d4fbb0...HEAD still reports the pre-existing Markdown hard-break trailing spaces at ARCHITECTURE.md:478; I am treating that as non-blocking hygiene, not a PR-caused correctness defect for this review.

Residual risk / owners:

  • Author action: none.
  • Exact-head CI owns PostgreSQL/live relay proof for this verdict; CI is green at the reviewed head.
  • Release/operator sequencing still matters: this PR must not deploy before #8696 / migration 0079 is merged, released, and applied everywhere the startup check expects.
  • I did not add a new local live-relay, native/mobile UI, restart/load, or multi-pod cache-invalidation proof in this final integration pass. Those are confidence gaps routed to CI/release validation, not author-actionable defects in the reviewed patch.

loganj added a commit that referenced this pull request Oct 30, 2026
Mobile Huddles decided "is this an agent" from the channel role `bot`.
The relay now stores `bot` only for keys with a recorded NIP-OA owner
(#8494), so an agent without an owner attestation joins as `member`.
Mobile then left it out of the participant list after joining, labeled
it as a person, did not show its "preparing a response" state, and
counted it as a person when deciding whether leaving ends the Huddle.

Add one Huddle agent owner, `huddle_agents_provider.dart`. A member is
an agent when it has the `bot` role or when a trusted author published
a validly signed kind:30177 managed-agent record for it. Trusted authors
are the current user and the Huddle channel's owners; a record from
anyone else is only a claim. This is the same rule as the Desktop fix
(#8544). Signature checks run off the UI isolate.

If the records are still loading or cannot be read, the `bot` role still
works, and the last-person count fails so leave keeps the Huddle open.

Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
Signed-off-by: Logan Johnson <loganj@squareup.com>
@loganj
loganj force-pushed the larry/retire-roster-trigger-migration branch from 33d4fbb to 64b5dcb Compare October 30, 2026 23:44
@loganj
loganj force-pushed the larry/deprecate-bot-role branch from a0e2824 to 2503fd9 Compare October 30, 2026 23:48
loganj and others added 2 commits November 1, 2026 17:59
`bot` has the same permissions as `member`; it only labels a key with a
recorded agent owner. Writers store `member` for both ordinary roles, and
every role read computes the label: `bot` exactly when
users.agent_owner_pubkey is set for the key in that community.

- The app-side roster check treats `member` and `bot` as one role.
- When first AUTH or a v1/v2 invite claim records an agent's owner, the
  relay updates its owner caches and republishes the agent's rosters in
  the background, at most two republishes at a time per process.
- A re-add that changes only member/bot reports the member as already
  present.
- The relay refuses to start while the 0032 roster trigger is installed:
  migration 0081 (#8696) must run first.

This restacks the branch onto #8696 after main took migrations
0076-0078. It replaces these earlier commits:

- Update the owner caches when an invite claim records an agent owner (47458e4)
- Republish an agent's rosters when an invite claim records its owner (35fc447)
- Renumber the roster-trigger retirement to migration 0076 (62290cf)
- test(relay): bind the member/bot re-add report to the kind:9000 path (c1aabfa)
- Treat member and bot as one role when reporting a re-add (d20e11d)
- Drop the one-shot member/bot roster reconcile scan (f8ebee0)
- Refuse to start while the 0032 roster trigger is installed (a4311bb)
- Bound roster republishes and tighten the 0074 rollout (4c2448c)
- test(relay): first AUTH is not held by a contended roster lock (143e4c7)
- Compute the member/bot label at read time and retire the roster trigger (02ccba9)

Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Logan Johnson <loganj@squareup.com>
Main's auto-join test (#8594) expected an owned agent to read as
`member`. With the label computed from the recorded owner, it reads as
`bot`; the stored role is still `member`.

Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: Logan Johnson <loganj@squareup.com>

This branch was previously deployed

1 inactive (outdated) deployment
codex-review — 4508f3d7 Deployed Oct 29, 2026 by loganj via Run Codex Security Review #10413
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

codex-security-review-current The posted Codex security review matches its recorded range.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants