Discord: check bot permissions before handing a token to a node (#225 S3) #249

Merged
logan merged 2 commits from feat/225-s3-perm-check into main 2026-10-05 02:38:49 -04:00
Owner

Slice S3 of #225: the permission check is wired into token picking (MVP backend). Built on origin/main with S1 (#230), S2 (#231) and #227 (several streams per channel) merged.

What changed

  • Where the check runs: streams._start_locked, before the lease. That covers all four entry points: bot POST /discord/streams, web picker POST /discord/nodes/{id}/streams, nodes.py discord_join, and the reconciler's re-place.
  • Which bots are checked: only the ones pick_token could choose (valid, pool or this org's, member or membership unknown, not busy in this server, leased to no node or to a candidate node). No other org's bot is ever asked about this server.
  • Limits: at most discord_perm_check_concurrency (4) at once, all within discord_perm_check_budget_seconds (3 s). A bot not answered in time counts as unknown, and its load keeps filling the cache.
  • pick_token(perms=...):
    • A denied bot is skipped.
    • In _candidate_order, confirmed-ok sorts before unknown, placed after BYO-before-pool.
    • Unknown is allowed (fail open).
    • When every bot is denied, the result is missing_permissions with denied=[{name, missing, reason, fix}], plus the invite links.
    • An explicit preferred_token_id that is denied is refused by name, not swapped.
    • The reuse branch in lease_for_stream falls through when its token is denied, so the stream moves to a bot that can speak.
  • The refusal: a 409 like DRB Radio 2 can't Speak in #dispatch: denied by #dispatch's override for @Muted. In Discord: Edit Channel → Permissions → add DRB Radio 2 → allow Speak, then try again.
    • The bot's /join shows it through _refusal_text (detail + invites; no bot change needed).
    • The reconciler shows it through _mark.
  • bad_channel: a channel that is not a voice channel gets a 400 bad_channel, not missing_permissions (per the S1 review note).
  • Plan finding 1: systems/{id}.preferred_token_id is now read, uncached, so a change applies to the next start.
    • It is a hint: tried first, and another bot is used when it can't take the stream.
    • An explicit preferred_token_id stays strict.
    • This covers owner requirement 1: invite only your system's radio bot, mark it preferred, stream.
  • Owner Q3: a pool bot at discord_pool_bot_guild_cap (90) is never offered as an invite. That applies to pick_token, voice-check and the bot overview. A full pool bot already in the server is still used. With nothing else left, the result is the new pool_full.
  • handle_actual: the edge codes missing_permissions, channel_full, not_in_guild and bad_channel call discord_rest.invalidate(token_id, guild_id).
  • Owner Q2, PUT /tokens/{id}/prefer/{system_id}:
    • A platform admin can set any bot on any system.
    • An org owner can set their own BYO bot on their own org's system.
    • Pool bots and shared systems (no org_id) get a 403.
    • Another org's bot or system gets a 404.
    • Members get a 403.
    • Both pointers stay single-valued, and every change is audited as discord.token.prefer.
  • Kill switch: DISCORD_PERM_CHECK_ENABLED=false means no Discord call before a lease. The system-preferred hint still applies, since it makes no Discord call. The config comment says so.
  • Tests: tests/conftest.py gets an autouse fixture that answers every Discord call with a connect error. No test can reach discord.com, and existing tests see unknown, which fails open to the old picking.

JSON contract (for S4)

409 from any start (POST /discord/streams, POST /discord/nodes/{id}/streams):

{"code": "missing_permissions",
 "detail": "DRB Radio 2 can't Speak in #dispatch: denied by #dispatch's override for @Muted. In Discord: Edit Channel → Permissions → add DRB Radio 2 → allow Speak, then try again.",
 "denied": [{"name": "DRB Radio 2", "missing": ["Speak"],
             "reason": "denied by #dispatch's override for @Muted",
             "fix": "In Discord: Edit Channel → Permissions → add DRB Radio 2 → allow Speak, then try again."}],
 "invite": [{"name": "DRB Radio 3", "invite_url": "https://discord.com/oauth2/authorize?..."}]}
  • With several denied bots, detail is one line per bot (up to 3) and then the fix.
  • Other new codes:
    • bad_channel (400)
    • pool_full (409)
  • nodes.py discord_join returns HTTPException(409, detail=<same text>).

GET /discord/nodes/{node_id}/guilds/{guild_id}/voice-check?refresh=0|1&channel_id=<optional>

  • Auth: any signed-in member of the node's org, or a platform admin.
  • Scope: the server must be in _org_guilds(node.org_id). Anything else is a 404.
  • Response:
{"guild_id": "100", "guild_name": "Fire Co. 7", "node_id": "n1",
 "enabled": true, "checked_at": "2026-10-05T...Z", "refreshed": false,
 "channels": [
   {"channel_id": "1001", "name": "dispatch",
    "state": "ready",
    "any_ok": true, "user_limit": 0, "limit_bypass": false,
    "bots": [{"name": "DRB Radio 1", "ok": true, "status": "ok", "missing": [], "reason": null, "fix": null,
              "available": true, "unavailable_reason": null},
             {"name": "DRB Radio 2", "ok": false, "status": "denied", "missing": ["Speak"],
              "reason": "denied by #dispatch's override for @Muted", "fix": "In Discord: ...",
              "available": true, "unavailable_reason": null}]}],
 "invite": [{"name": "...", "invite_url": "..."}]}
  • state:

    Value Meaning
    ready An ok bot is free.
    unknown A free bot couldn't be checked. A start is still allowed.
    needs_fix Every free bot was denied. This is the only state where Start should be disabled.
    busy The ok bots are all in use.
    no_bot No bot of this org is in the server.
    bad_channel Not a voice channel.
    unchecked The kill switch is off.
  • bots[].status: ok | denied | unknown | not_member | invalid | bad_channel | unchecked.

  • unavailable_reason: busy_here (the bot is already in a voice channel in this server) or other_node.

  • Token data: bot names only for non-admins. A platform admin also gets token_id. No token strings.

  • user_limit is advisory. REST can't see occupancy, so the edge's channel_full stays the enforcement.

  • Channels: the server's directory (voice_channels). When the directory is empty, the voice channels from a bot's snapshot.

  • refresh=1: at most once per 10 s per uid and server. Past that the cached answer is served with refreshed: false.

GET /discord/guilds/{guild_id}/voice-check?channel_id=&refresh=

  • Auth: service key or platform admin.
  • Same shape, plus org_id. Rows carry token_id.

Not in this slice

  • Org owners still have no way to list their BYO bots' ids. Q2 lets them call /prefer, but GET /tokens is admin-only. I'm filing this as a follow-up (linked on #225).
  • #223 (caps race) stays its own PR.

Verification

  • pytest (drb-c2-core): 800 passed.
    • Includes 43 new tests in tests/test_discord_perm_check.py.
    • Those cover all four entry points, the budget and concurrency limits, the kill switch, bad_channel, the preferred hint, the one-bot setup, voice-check scoping and the rate limit, the Q2 matrix, and an end-to-end run through the real REST client on a MockTransport that checks the token never reaches logs or messages.
  • No frontend files were changed, so tsc was not run.

Not merged; the main session merges after review.

🤖 Generated with Claude Code

Slice **S3** of #225: the permission check is wired into token picking (MVP backend). Built on origin/main with S1 (#230), S2 (#231) and #227 (several streams per channel) merged. ## What changed - **Where the check runs:** `streams._start_locked`, before the lease. That covers all four entry points: bot `POST /discord/streams`, web picker `POST /discord/nodes/{id}/streams`, `nodes.py` `discord_join`, and the reconciler's re-place. - **Which bots are checked:** only the ones `pick_token` could choose (valid, pool or this org's, member or membership unknown, not busy in this server, leased to no node or to a candidate node). No other org's bot is ever asked about this server. - **Limits:** at most `discord_perm_check_concurrency` (4) at once, all within `discord_perm_check_budget_seconds` (3 s). A bot not answered in time counts as unknown, and its load keeps filling the cache. - **`pick_token(perms=...)`:** - A denied bot is skipped. - In `_candidate_order`, confirmed-ok sorts before unknown, placed after BYO-before-pool. - Unknown is allowed (fail open). - When every bot is denied, the result is `missing_permissions` with `denied=[{name, missing, reason, fix}]`, plus the invite links. - An explicit `preferred_token_id` that is denied is refused by name, not swapped. - The reuse branch in `lease_for_stream` falls through when its token is denied, so the stream moves to a bot that can speak. - **The refusal:** a 409 like `DRB Radio 2 can't Speak in #dispatch: denied by #dispatch's override for @Muted. In Discord: Edit Channel → Permissions → add DRB Radio 2 → allow Speak, then try again.` - The bot's `/join` shows it through `_refusal_text` (detail + invites; no bot change needed). - The reconciler shows it through `_mark`. - **`bad_channel`:** a channel that is not a voice channel gets a 400 `bad_channel`, not `missing_permissions` (per the S1 review note). - **Plan finding 1:** `systems/{id}.preferred_token_id` is now read, uncached, so a change applies to the next start. - It is a **hint**: tried first, and another bot is used when it can't take the stream. - An explicit `preferred_token_id` stays strict. - This covers owner requirement 1: invite only your system's radio bot, mark it preferred, stream. - **Owner Q3:** a pool bot at `discord_pool_bot_guild_cap` (90) is never offered as an invite. That applies to `pick_token`, voice-check and the bot overview. A full pool bot already in the server is still used. With nothing else left, the result is the new `pool_full`. - **`handle_actual`:** the edge codes `missing_permissions`, `channel_full`, `not_in_guild` and `bad_channel` call `discord_rest.invalidate(token_id, guild_id)`. - **Owner Q2, `PUT /tokens/{id}/prefer/{system_id}`:** - A platform admin can set any bot on any system. - An org **owner** can set their own BYO bot on their own org's system. - Pool bots and shared systems (no `org_id`) get a 403. - Another org's bot or system gets a 404. - Members get a 403. - Both pointers stay single-valued, and every change is audited as `discord.token.prefer`. - **Kill switch:** `DISCORD_PERM_CHECK_ENABLED=false` means no Discord call before a lease. The system-preferred hint still applies, since it makes no Discord call. The config comment says so. - **Tests:** `tests/conftest.py` gets an autouse fixture that answers every Discord call with a connect error. No test can reach discord.com, and existing tests see unknown, which fails open to the old picking. ## JSON contract (for S4) **409 from any start** (`POST /discord/streams`, `POST /discord/nodes/{id}/streams`): ```json {"code": "missing_permissions", "detail": "DRB Radio 2 can't Speak in #dispatch: denied by #dispatch's override for @Muted. In Discord: Edit Channel → Permissions → add DRB Radio 2 → allow Speak, then try again.", "denied": [{"name": "DRB Radio 2", "missing": ["Speak"], "reason": "denied by #dispatch's override for @Muted", "fix": "In Discord: Edit Channel → Permissions → add DRB Radio 2 → allow Speak, then try again."}], "invite": [{"name": "DRB Radio 3", "invite_url": "https://discord.com/oauth2/authorize?..."}]} ``` - With several denied bots, `detail` is one line per bot (up to 3) and then the fix. - Other new codes: - `bad_channel` (400) - `pool_full` (409) - `nodes.py` `discord_join` returns `HTTPException(409, detail=<same text>)`. **`GET /discord/nodes/{node_id}/guilds/{guild_id}/voice-check?refresh=0|1&channel_id=<optional>`** - **Auth:** any signed-in member of the node's org, or a platform admin. - **Scope:** the server must be in `_org_guilds(node.org_id)`. Anything else is a 404. - **Response:** ```json {"guild_id": "100", "guild_name": "Fire Co. 7", "node_id": "n1", "enabled": true, "checked_at": "2026-10-05T...Z", "refreshed": false, "channels": [ {"channel_id": "1001", "name": "dispatch", "state": "ready", "any_ok": true, "user_limit": 0, "limit_bypass": false, "bots": [{"name": "DRB Radio 1", "ok": true, "status": "ok", "missing": [], "reason": null, "fix": null, "available": true, "unavailable_reason": null}, {"name": "DRB Radio 2", "ok": false, "status": "denied", "missing": ["Speak"], "reason": "denied by #dispatch's override for @Muted", "fix": "In Discord: ...", "available": true, "unavailable_reason": null}]}], "invite": [{"name": "...", "invite_url": "..."}]} ``` - **`state`:** | Value | Meaning | |---|---| | `ready` | An ok bot is free. | | `unknown` | A free bot couldn't be checked. A start is still allowed. | | `needs_fix` | Every free bot was denied. **This is the only state where Start should be disabled.** | | `busy` | The ok bots are all in use. | | `no_bot` | No bot of this org is in the server. | | `bad_channel` | Not a voice channel. | | `unchecked` | The kill switch is off. | - **`bots[].status`:** `ok | denied | unknown | not_member | invalid | bad_channel | unchecked`. - **`unavailable_reason`:** `busy_here` (the bot is already in a voice channel in this server) or `other_node`. - **Token data:** bot names only for non-admins. A platform admin also gets `token_id`. No token strings. - **`user_limit`** is advisory. REST can't see occupancy, so the edge's `channel_full` stays the enforcement. - **Channels:** the server's directory (`voice_channels`). When the directory is empty, the voice channels from a bot's snapshot. - **`refresh=1`:** at most once per 10 s per uid and server. Past that the cached answer is served with `refreshed: false`. **`GET /discord/guilds/{guild_id}/voice-check?channel_id=&refresh=`** - Auth: service key or platform admin. - Same shape, plus `org_id`. Rows carry `token_id`. ## Not in this slice - **Org owners still have no way to list their BYO bots' ids.** Q2 lets them call `/prefer`, but `GET /tokens` is admin-only. I'm filing this as a follow-up (linked on #225). - **#223 (caps race)** stays its own PR. ## Verification - `pytest` (drb-c2-core): **800 passed**. - Includes 43 new tests in `tests/test_discord_perm_check.py`. - Those cover all four entry points, the budget and concurrency limits, the kill switch, `bad_channel`, the preferred hint, the one-bot setup, voice-check scoping and the rate limit, the Q2 matrix, and an end-to-end run through the real REST client on a MockTransport that checks the token never reaches logs or messages. - No frontend files were changed, so `tsc` was not run. Not merged; the main session merges after review. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
logan added 1 commit 2026-10-05 02:23:50 -04:00
start_stream now asks Discord (cached REST snapshots, S2) whether each bot
it could pick may View, Connect and Speak in the voice channel, before the
lease. Covers all four entry points: bot POST /discord/streams, web picker,
nodes discord_join, and the reconciler's re-place.

- pick_token: denied bots skipped; confirmed-ok sorts before unknown;
  unknown is allowed (fail open); all denied -> missing_permissions naming
  each bot, missing permission, trace reason and fix. The reuse branch
  falls through when its token is denied.
- Bounded: discord_perm_check_concurrency at once, within
  discord_perm_check_budget_seconds; unanswered = unknown.
- Non-voice channel -> 400 bad_channel (not missing_permissions).
- systems/{id}.preferred_token_id is now read (plan finding 1), as a hint.
- Pool bots at discord_pool_bot_guild_cap are never offered as invites;
  pool_full when nothing else is left (owner Q3).
- handle_actual drops cached access on missing_permissions, channel_full,
  not_in_guild, bad_channel.
- New GET /discord/nodes/{node_id}/guilds/{guild_id}/voice-check (org
  member, names only) and GET /discord/guilds/{guild_id}/voice-check
  (service key or admin).
- PUT /tokens/{id}/prefer/{system_id}: org owners for their own systems
  and own BYO bots; shared systems and pool bots stay admin-only (owner Q2).
  Keeps both pointers single-valued; audited.
- Kill switch: DISCORD_PERM_CHECK_ENABLED=false.
- tests/conftest.py: autouse fixture so no test ever reaches discord.com.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
logan added 1 commit 2026-10-05 02:31:07 -04:00
- guild_overview offered an invite link to a full pool bot whenever it was
  in ANY server (`if member` tested the non-empty guild_ids list, not
  membership of this guild). Now: in this guild, or invitable.
- start_stream re-reads membership once on pool_full as it already does on
  no_bot_in_guild, so a full pool bot an admin just invited by hand is found.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Author
Owner

Review (S3): pushed 8bdcfca on this branch.

  • Fixed: guild_overview gave a full pool bot an invite link whenever it was in any server (if member tested the list, not this guild). Test added.
  • Fixed: start_stream now re-reads membership once on pool_full, as on no_bot_in_guild. Test added.
  • Filed #251: after a permission fix, /join keeps refusing for up to 120 s (cached snapshot).
  • Note: with the kill switch off, the system's preferred bot still applies. The PR says so; the plan said the switch restores picking exactly.
    pytest: 802 passed. Not merged.
Review (S3): pushed 8bdcfca on this branch. - Fixed: `guild_overview` gave a full pool bot an invite link whenever it was in any server (`if member` tested the list, not this guild). Test added. - Fixed: `start_stream` now re-reads membership once on `pool_full`, as on `no_bot_in_guild`. Test added. - Filed #251: after a permission fix, `/join` keeps refusing for up to 120 s (cached snapshot). - Note: with the kill switch off, the system's preferred bot still applies. The PR says so; the plan said the switch restores picking exactly. pytest: 802 passed. Not merged.
logan merged commit ad033b134e into main 2026-10-05 02:38:49 -04:00
Sign in to join this conversation.
No Reviewers
No labels
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: logan/server-26#249