S7: OAuth start/options accept system_id; Discord picker QA fixes (server-26#225, #256 #257 #258)

- discord_oauth: POST /discord/oauth/start and GET /options take an optional
  system_id. The system must be the org's own or shared (another org's or
  unknown = 404). Its preferred bot is strict: one that can't connect is a
  404, never a quiet swap to Automatic. No preference = the usual order.
  The picker's "bot for a system" now always sends system_id.
- #257: _channel_state gives a bot whose token Discord rejected its own
  "invalid" state; the picker shows "Radio bot needs replacing" with no
  invite links, instead of "No radio bot".
- #256: Start no longer stays disabled on "Needs fix". It re-checks with
  Discord first (refresh=1) and starts only if the fresh answer allows it.
  "Not a voice channel" still disables Start.
- #258: own tooltip for "Not a voice channel"; "platform admin" -> "DRB
  support"; PreferredBotsCard says plainly that adding your own bot isn't
  self-serve (no support address exists yet, see #47), and a load failure
  gets a plain message + Try again instead of the raw C2 error.

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

drb-c2-core/app/routers/discord_checks.py

@@ -84,6 +84,8 @@ def _channel_state(rows: list[dict], enabled: bool) -> str:
         return "needs_fix"        # a start is refused with missing_permissions
     if any(r["ok"] for r in rows):
         return "busy"
+    if any(r["status"] == discord_rest.INVALID for r in rows):
+        return "invalid"          # the bot IS here, but Discord rejected its token (#257)
     return "no_bot"
 
 

drb-c2-core/app/routers/discord_oauth.py

@@ -14,7 +14,9 @@ Every route is a 404 while ``discord_oauth_redirect_uri`` is empty or no
 eligible bot has an OAuth secret (PUT /tokens/{id}/oauth).
 
 Which bot (``_pick_bot``): an explicit ``token_id`` the org may use; else the
-radio bot preferred for the node's system (``node_id``); else the org's own
+radio bot preferred for ``system_id`` (the org's own system or a shared one;
+strict: a preferred bot that can't connect is a 404, not a quiet swap); else
+the radio bot preferred for the node's system (``node_id``); else the org's own
 bring-your-own bot; else the pool bot with the fewest servers that is under
 ``discord_pool_bot_guild_cap`` (owner decision Q3). Only bots with an OAuth
 secret set count. A bot the caller's org can't use is a 404, never a 403.
@@ -30,7 +32,7 @@ application's credentials, after the org-scope checks and rate limits here.
 from __future__ import annotations
 
 import asyncio
-from typing import Optional
+from typing import Annotated, Optional
 
 from fastapi import APIRouter, Depends, HTTPException, Query
 from fastapi.responses import JSONResponse
@@ -81,7 +83,7 @@ def _usable(tok: dict, org_id: str, oauth_ids: set) -> bool:
 
 
 async def _pick_bot(org_id: str, *, node_id: Optional[str] = None,
-                    token_id: Optional[str] = None) -> tuple[dict, str]:
+                    token_id: Optional[str] = None, system_id: Optional[str] = None) -> tuple[dict, str]:
     """(token doc, why) — why is ``chosen`` | ``preferred`` | ``org_bot`` |
     ``pool``. Raises HTTPException 404 when nothing fits."""
     tokens = {t["token_id"]: t for t in await fstore.collection_list(leases.TOKENS) if t.get("token_id")}
@@ -96,6 +98,21 @@ async def _pick_bot(org_id: str, *, node_id: Optional[str] = None,
                                      "right now.")
         return tok, "chosen"
 
+    if system_id:
+        # Same visibility as a node's system: the org's own or a shared one
+        # (no org_id). Another org's system is a 404, never a 403.
+        system = await fstore.doc_get("systems", system_id)
+        if not system or system.get("org_id") not in (None, org_id):
+            raise HTTPException(404, "System not found.")
+        pref_id = system.get("preferred_token_id") or ""
+        if pref_id:
+            # The caller asked for THIS system's bot by name: inviting a
+            # different one instead would be the silent swap this exists to stop.
+            pref = tokens.get(pref_id)
+            if not pref or not _usable(pref, org_id, oauth_ids):
+                raise HTTPException(404, "That system's radio bot can't be used to connect a server right now.")
+            return pref, "preferred"
+
     if node_id:
         node = await fstore.doc_get("nodes", node_id)
         if not node or node.get("org_id") != org_id:
@@ -133,17 +150,19 @@ def _bot_summary(tok: dict, why: str) -> dict:
 async def oauth_options(node_id: Optional[str] = Query(None, pattern=_ID),
                         token_id: Optional[str] = Query(None, pattern=_ID),
                         org_id: Optional[str] = None,
-                        decoded: dict = Depends(require_org_owner_token)):
+                        decoded: dict = Depends(require_org_owner_token),
+                        system_id: Annotated[Optional[str], Query(pattern=_ID)] = None):
     """Whether Connect is offered here, and which bot it would invite."""
     _require_enabled()
     scope = resolve_org_scope(decoded, org_id)
-    tok, why = await _pick_bot(scope, node_id=node_id, token_id=token_id)
+    tok, why = await _pick_bot(scope, node_id=node_id, token_id=token_id, system_id=system_id)
     return {"available": True, "bot": _bot_summary(tok, why)}
 
 
 class StartBody(BaseModel):
     node_id: Optional[str] = Field(None, pattern=_ID)
     token_id: Optional[str] = Field(None, pattern=_ID)
+    system_id: Optional[str] = Field(None, pattern=_ID)
     return_to: Optional[str] = None
 
 
@@ -154,12 +173,14 @@ async def oauth_start(body: StartBody, org_id: Optional[str] = None,
     scope = resolve_org_scope(decoded, org_id)
     uid, email = describe_actor(decoded)
     start_limiter.check(f"{uid}:discord_oauth_start")
-    tok, why = await _pick_bot(scope, node_id=body.node_id, token_id=body.token_id)
+    tok, why = await _pick_bot(scope, node_id=body.node_id, token_id=body.token_id,
+                               system_id=body.system_id)
     return_to = oauth.safe_return_to(body.return_to)
     nonce, expires_at = await oauth.create_state(org_id=scope, uid=uid, token_id=tok["token_id"],
                                                  node_id=body.node_id, return_to=return_to)
     await write_audit(uid, email, "discord.oauth.start", details={
-        "org_id": scope, "token_id": tok["token_id"], "reason": why, "node_id": body.node_id})
+        "org_id": scope, "token_id": tok["token_id"], "reason": why, "node_id": body.node_id,
+        "system_id": body.system_id})
     return {"authorize_url": oauth.authorize_url(str(tok["application_id"]), nonce),
             "expires_at": expires_at.isoformat(), "bot": _bot_summary(tok, why)}
 

drb-c2-core/tests/test_discord_oauth.py

@@ -187,6 +187,58 @@ async def test_preferred_bot_for_the_nodes_system_wins(fs):
     assert e.value.status_code == 404
 
 
+async def test_system_id_invites_that_systems_preferred_bot(fs):
+    """The S7 picker's "bot for a system" choice: a platform-pinned pool bot
+    is invited by system_id, never quietly swapped for Automatic."""
+    fs.put("systems", "s1", {"system_id": "s1", "org_id": "org-a", "preferred_token_id": "pool-1"})
+    out = await router.oauth_options(node_id=None, token_id=None, org_id=None, decoded=OWNER_A, system_id="s1")
+    assert out["bot"] == {"name": "DRB Radio 1", "reason": "preferred", "own_bot": False}
+    out, q = await _start(system_id="s1", return_to="/settings")
+    assert q["client_id"] == "201" and out["bot"]["reason"] == "preferred"
+    assert _audit(fs, "discord.oauth.start")[-1]["details"]["system_id"] == "s1"
+    # a shared system (no org_id) serves every org, as on the node path
+    fs.put("systems", "shared", {"system_id": "shared", "org_id": None, "preferred_token_id": "pool-1"})
+    out = await router.oauth_options(node_id=None, token_id=None, org_id=None, decoded=OWNER_B,
+                                     system_id="shared")
+    assert out["bot"]["name"] == "DRB Radio 1"
+    # system_id outranks node_id
+    fs.put("nodes", "n1", {"node_id": "n1", "org_id": "org-a", "assigned_system_id": "s2"})
+    fs.put("systems", "s2", {"system_id": "s2", "org_id": "org-a", "preferred_token_id": "own-a"})
+    out = await router.oauth_options(node_id="n1", token_id=None, org_id=None, decoded=OWNER_A, system_id="s1")
+    assert out["bot"]["name"] == "DRB Radio 1"
+
+
+async def test_system_id_of_another_org_or_unknown_is_404(fs):
+    fs.put("systems", "sb", {"system_id": "sb", "org_id": "org-b", "preferred_token_id": "pool-1"})
+    for sid in ("sb", "nope"):
+        with pytest.raises(HTTPException) as e:
+            await router.oauth_options(node_id=None, token_id=None, org_id=None, decoded=OWNER_A, system_id=sid)
+        assert e.value.status_code == 404 and e.value.detail == "System not found."
+        with pytest.raises(HTTPException) as e:
+            await _start(system_id=sid)
+        assert e.value.status_code == 404
+    assert not _audit(fs, "discord.oauth.start")
+    # a platform admin acting for org-b may use it
+    out = await router.oauth_options(node_id=None, token_id=None, org_id="org-b", decoded=ADMIN, system_id="sb")
+    assert out["bot"]["name"] == "DRB Radio 1"
+
+
+async def test_system_id_whose_bot_cannot_connect_is_404_not_a_swap(fs):
+    # pool-2 is at the cap; another org's bot pinned (bad data); no secret
+    fs.put("systems", "s1", {"system_id": "s1", "org_id": "org-a", "preferred_token_id": "pool-2"})
+    fs.put("systems", "s2", {"system_id": "s2", "org_id": "org-a", "preferred_token_id": "own-b"})
+    fs.put("systems", "s3", {"system_id": "s3", "org_id": "org-a", "preferred_token_id": "pool-1"})
+    fs.data[oauth.OAUTH].pop("pool-1")
+    for sid in ("s1", "s2", "s3"):
+        with pytest.raises(HTTPException) as e:
+            await router.oauth_options(node_id=None, token_id=None, org_id=None, decoded=OWNER_A, system_id=sid)
+        assert e.value.status_code == 404 and "Harbor" not in str(e.value.detail)
+    # no preferred bot at all: the usual order (org's own bot first)
+    fs.put("systems", "s4", {"system_id": "s4", "org_id": "org-a"})
+    out = await router.oauth_options(node_id=None, token_id=None, org_id=None, decoded=OWNER_A, system_id="s4")
+    assert out["bot"]["reason"] == "org_bot"
+
+
 async def test_explicit_bot_must_be_usable_by_the_org(fs):
     out = await router.oauth_options(node_id=None, token_id="pool-1", org_id=None, decoded=OWNER_A)
     assert out["bot"]["reason"] == "chosen"

drb-c2-core/tests/test_discord_perm_check.py

@@ -84,7 +84,7 @@ def fs(monkeypatch):
     monkeypatch.setattr(settings, "discord_unlinked_guild_org_id", "")
     monkeypatch.setattr(settings, "discord_perm_check_enabled", True)
 
-    # Per-token snapshot behaviour: "ok" | "muted" | "unknown" | "slow" | "not_member"
+    # Per-token snapshot behaviour: "ok" | "muted" | "unknown" | "slow" | "not_member" | "invalid"
     fake.snap_mode = {}
     fake.snap_calls = []
 
@@ -99,6 +99,8 @@ def fs(monkeypatch):
             return _snap(tid, status=discord_rest.UNKNOWN)
         if mode == "not_member":
             return _snap(tid, status=discord_rest.NOT_MEMBER)
+        if mode == "invalid":
+            return _snap(tid, status=discord_rest.INVALID)
         return _snap(tid, muted=(mode == "muted"))
     monkeypatch.setattr(discord_rest, "guild_snapshot", _snapshot)
 
@@ -413,6 +415,23 @@ def test_voice_check_needs_fix_when_every_free_bot_is_denied(fs):
     assert out["channels"][0]["state"] == "unknown"        # can't tell -> start still allowed
 
 
+def test_voice_check_invalid_token_is_not_no_bot(fs):
+    # server-26#257: the bot is in the server but Discord rejected its token.
+    # "No radio bot" (and an invite to a bot already there) would be wrong.
+    _token(fs, "t1")
+    fs.snap_mode["t1"] = "invalid"
+    out = run(checks.node_voice_check("n1", GUILD, refresh=0, channel_id="1001", decoded=OWNER))
+    assert out["channels"][0]["state"] == "invalid"
+    assert out["channels"][0]["bots"][0]["status"] == "invalid"
+    # a working bot elsewhere in the list still wins
+    _token(fs, "t2")
+    discord_rest.invalidate()
+    out = run(checks.node_voice_check("n1", GUILD, refresh=0, channel_id="1001", decoded=OWNER))
+    assert out["channels"][0]["state"] == "ready"
+    # no bot at all is still no_bot
+    assert checks._channel_state([], True) == "no_bot"
+
+
 def test_voice_check_is_org_scoped(fs):
     _token(fs, "t1")
     for node, guild, who in (("n1", "200", OWNER), ("nx", GUILD, OWNER), ("n1", GUILD, STRANGER)):