Skip to content

fix(relay): classify self-joining agents as bots, not members - #6562

Open
rmichelena wants to merge 1 commit into
block:mainfrom
rmichelena:bumble/join-role-agent-discriminator
Open

fix(relay): classify self-joining agents as bots, not members#6562
rmichelena wants to merge 1 commit into
block:mainfrom
rmichelena:bumble/join-role-agent-discriminator

Conversation

@rmichelena

Copy link
Copy Markdown

Fixes #6561.

handle_join_request hard-coded MemberRole::Member. kind:9021 carries no role tag and buzz channels join exposes no --role, so a self-joining agent could not ask for anything else — and clients read the channel role as the "is this an agent" signal, so it vanished from @mention autocomplete. Repair needs role: bot on a kind:9000, which the relay restricts to owners/admins, and the Desktop role menu offers only admin/member/guest — so there is no in-app way back.

Why Member is the wrong default here

MemberRole's own documentation is the argument:

// crates/buzz-core/src/channel.rs
/// The hierarchy for permission checks is: Owner > Admin > Member > Guest.
/// Bot is a **separate designation** — it is not part of the linear hierarchy.

Member is not a conservative privilege choice for an agent — it is the wrong category, and it is the category clients consume.

What this does

Uses the agent discriminator the codebase already defines and relies on:

// crates/buzz-db/src/usage.rs
/// Agent discriminator: `agent_owner_pubkey IS NOT NULL`.

read through get_agent_channel_policy, which this file already calls in handle_put_user's policy check. No new classification rule is introduced.

What this deliberately does not do

handle_put_user is untouched. A caller that states admin / member / guest for a human still gets exactly that. The relay decides only on the one path where no one can express intent — a human self-joining still lands Member, unchanged.

A failed classification lookup logs a warning and falls back to Member, preserving today's behaviour rather than failing the join.

Tests

The decision is extracted into self_join_role so it is unit-testable without a database:

  • agent (agent_owner_pubkey present) → Bot
  • human (user row, no agent owner) → Member
  • no user row → Member, i.e. unchanged when nothing is known

cargo clippy -p buzz-relay --all-targets -- -D warnings is clean.

Closed relays

agent_owner_pubkey is the field that stays NULL when require_relay_membership = true#5581. Where that is unresolved the discriminator reports every agent as human and this change is a no-op, so #5581 is a prerequisite for closed deployments rather than an unrelated fix. Flagging it so this is not mistaken for a complete fix on such a relay.

…6561)

kind:9021 carries no role tag and buzz channels join exposes no --role, so a
self-joining agent could not ask for anything and handle_join_request recorded
it as MemberRole::Member. Clients read the channel role as the "is this an
agent" signal, so a self-joined agent disappears from @mention autocomplete —
and the repair needs role:bot on a kind:9000, which the relay restricts to
owners/admins, so there is no in-app way back.

MemberRole's own docs say Bot "is not part of the linear hierarchy" but a
separate designation, so Member is not a conservative choice here — it is the
wrong category, and it is the category clients read.

Uses the agent discriminator the codebase already relies on,
agent_owner_pubkey IS NOT NULL (see buzz_db::usage::user_counts), via the
get_agent_channel_policy accessor already called elsewhere in this file. No new
policy is introduced.

Scoped deliberately to the self-join path: handle_put_user is untouched, so a
caller that states admin/member/guest for a human still gets exactly that. The
relay decides only where no one else can express intent.

A failed classification lookup logs and falls back to Member, preserving the
previous behaviour rather than failing the join.

The decision is extracted into self_join_role so it is unit-testable without a
database; three tests cover agent, human, and no-user-row.

Note for closed relays: agent_owner_pubkey is the field that stays NULL when
require_relay_membership is true (block#5581), so this is a no-op there until that
lands.

Signed-off-by: Roberto Michelena <77797875+rmichelena@users.noreply.github.com>

@Chessing234 Chessing234 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.

the reasoning in the comments is unusually good — especially spelling out why Bot can't shadow a human role and why handle_put_user is deliberately untouched. extracting self_join_role for the three unit tests is the right shape.

the gap i'd want addressed is the agents that already self-joined. your own comment notes that changing an active member's role is owner/admin-only, so every agent that hit this before the fix stays Member and stays unmentionable, with no in-app way to repair it — which is the actual complaint in #6561. this makes the bug stop happening; it doesn't make the existing ones work.

is a backfill in scope? something like "set role=Bot where the member row is Member and agent_owner_pubkey IS NOT NULL" is the same discriminator you're already using, so it should be expressible as a migration. if it's deliberately out of scope that's fine, but worth saying so, otherwise the issue looks fixed while affected users still can't mention their agent.

@rmichelena

Copy link
Copy Markdown
Author

Thanks — and yes, you're right that this is forward-only. It stops the bug happening; it doesn't repair anyone already in it, and #6561's complaint is about the latter.

A backfill is in scope as far as I'm concerned. I'd want to narrow the predicate before writing one, though, because the discriminator on its own is too broad.

Why role = 'member' AND agent_owner_pubkey IS NOT NULL overshoots

That set includes agents an owner deliberately added as memberadd-member takes an explicit --role, and handle_put_user preserves an existing role precisely because roles can be intentional. A backfill keyed only on "is an agent" silently overrides those decisions, which is the opposite of the scoping this PR is careful about (handle_put_user untouched, relay decides only where nobody can express intent).

There's a column that separates them

handle_join_request is the only production path that creates a member with no inviter:

path invited_by
handle_join_request (self-join) None
handle_put_user (kind:9000 / add-member) Some(actor)
workflow sink agent attach Some(author)
audio channel auto-add Some(channel.created_by)

So the affected rows are addressable exactly:

UPDATE channel_members cm
   SET role = 'bot'
  FROM users u
 WHERE cm.role = 'member'
   AND cm.invited_by IS NULL          -- self-joined, nobody chose this role
   AND cm.removed_at IS NULL
   AND u.community_id = cm.community_id
   AND u.pubkey = cm.pubkey
   AND u.agent_owner_pubkey IS NOT NULL;

invited_by IS NULL is what makes it a repair rather than an override: it targets rows where the role was assigned by the code this PR is fixing, and leaves every row where somebody actually chose member.

Scope note on that claim: I checked the add_member call sites in buzz-relay and buzz-db; the non-test ones are the four above. If there's a path I've missed that legitimately leaves invited_by NULL, the predicate needs revisiting — I'd rather you tell me than have me assert it too confidently.

One caveat that limits how much it fixes

agent_owner_pubkey is NULL on relays running require_relay_membership = true, which is the gap #5581 addresses. On those deployments both this fix and the backfill are no-ops until that lands — worth stating in the migration comment so nobody reads a clean run as "no affected rows existed".

Happy to add the migration to this PR, or as a follow-up if you'd rather keep the code change reviewable on its own. Say which and I'll do it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Self-join (kind:9021) hard-codes role=member, making a self-joined agent permanently unmentionable

2 participants