fix(relay): classify self-joining agents as bots, not members - #6562
fix(relay): classify self-joining agents as bots, not members#6562rmichelena wants to merge 1 commit into
Conversation
…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
left a comment
There was a problem hiding this comment.
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.
|
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
|
| 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.
Fixes #6561.
handle_join_requesthard-codedMemberRole::Member.kind:9021carries no role tag andbuzz channels joinexposes 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@mentionautocomplete. Repair needsrole: boton akind: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
Memberis the wrong default hereMemberRole's own documentation is the argument:Memberis 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:
read through
get_agent_channel_policy, which this file already calls inhandle_put_user's policy check. No new classification rule is introduced.What this deliberately does not do
handle_put_useris untouched. A caller that statesadmin/member/guestfor 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 landsMember, 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_roleso it is unit-testable without a database:agent_owner_pubkeypresent) →BotMemberMember, i.e. unchanged when nothing is knowncargo clippy -p buzz-relay --all-targets -- -D warningsis clean.Closed relays
agent_owner_pubkeyis the field that stays NULL whenrequire_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.