feat(bot): a self-diagnosing bot, a /record start that knows the session, and linking that names the way out (phase 5 lane B) #521
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/phase5b-bot-self-diagnosis"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes #373, closes #393, closes #392. Phase 5 lane B of the v4.3.0 build order (#514). Five commits; every API change is additive (new endpoints, new defaulted fields), so no
BOT_CONTRACT_VERSIONbump. No migration.#373 self-diagnosis.
api_clientraisesQuestBoardApiError(status, detail)(a subclass ofhttpx.HTTPStatusError, so the startup handshake and existing handlers are untouched); cogs show the backend's own sentence for a 4xx or a 503 (the named acceptance case, "LLM endpoint not configured", is a 503 that always carries actionable prose) and the generic message for a 500 or network failure./next,/quests,/historydistinguish "no campaign is bound to this server" from "nothing scheduled" via a newGET /api/bot/guilds/{id}/campaigns, consulted only on the empty path./askgets an additivereason: "no_transcripts"rather than a null answer that would crash an older bot. NewHelpcog:/help(a test asserts it covers every registered command), anon_guild_joinwelcome with the three setup steps and the guild ID pasted in, a presence set in the constructor so it survives reconnects. Alsocommand_prefix=commands.when_mentioned, the one prefix discord.py exempts from the "message content intent is missing" warning seen on prod after v4.2.3, with a test.#393
/record start. Reconciled with #399, which is left intact: the audio was already rescued on restart, but everything needed to say so died with the process. The active recording is now written to Redis DB 0 (qb:bot:recording:<guild_id>, 7-day TTL, best-effort), andannounce_interrupted_recordingsruns after #399's recovery to report which of three things happened (recovered and being transcribed; kept but not yet submitted; nothing captured)./record start's session argument autocompletes against a newGET /api/bot/guilds/{id}/sessions/recordable; the slash path setssession_title(the "Unknown Session" bug) with the regression test nothing had; a definite 404 refuses the start while a timeout still starts, since capture cannot be redone;defer()precedes the metadata fetch;/record stopfulfils its interaction; failure strings are written for a hosted customer with the exception in the log and a retry link.#392 linking dead ends. Two small self-contained HTML pages replace the raw JSON 400/409, each naming the next step (
/linkagain vs sign in as the other account and/unlink)./auth/linkrecords the outcome and/api/bot/link-status/{token}gains an additivestate(pending|linked|expired|conflict), so the poller speaks as soon as the outcome is known and names/unlinkon a conflict. Closed DMs fall back to a self-deleting channel mention in both the linking poller and the pre-vote prompt. The vote reminder tells unlinked players to run/link;Profile.jsxnames the command. Review note: the backend'sSecurityHeadersMiddlewarenow emitsdefault-src 'none'; style-src 'unsafe-inline'; base-uri 'none'; form-action 'none'; frame-ancestors 'none'fortext/htmlresponses only (the pages contain no user-controlled text); JSON responses keep the strict policy verbatim, with a test.Bot 398 passed (+80), backend full 2378 passed / 13 skipped, frontend 514 passed, ruff and version-sync clean. One adjacent data-loss finding filed as #520 (raw tracks deleted on a mid-conversion failure).
🤖 Generated with Claude Code
Every bot → Quest Board call ended in `raise_for_status()` and every cog caught a bare `Exception`, so the same sentence came back whatever had happened: "Could not reach Quest Board right now. Please try again shortly." The backend was frequently saying something the reader could act on — "LLM endpoint not configured. Set it in Admin → Bot Settings." — and the bot replaced it with a claim that the service was down. `api_client` now raises `QuestBoardApiError`, carrying the response's `detail` and status. It subclasses `httpx.HTTPStatusError` so the startup version handshake and every existing `except httpx.HTTPStatusError` keep working unchanged. `user_message_for()` decides what a user sees: the backend's own words when the backend is *answering* rather than failing (a 4xx, or a 503, which on this backend always names a dependency that is not configured), and the generic line for a 500 or a network failure, where it is the only true thing to say. Three commands had a second, quieter version of the same problem: `/next`, `/quests` and `/history` answered "nothing scheduled" for a server nobody had finished setting up, because every guild endpoint reports an unlinked guild and an idle one identically. A new additive endpoint, GET /api/bot/guilds/{id}/campaigns, answers the question directly — empty list, not 404, since "no campaign is linked here" is an ordinary state of a freshly invited bot. The cogs consult it only on the empty path, and a failure to consult it keeps the old wording rather than accusing a working server of being misconfigured. `/ask` had dead code for "nothing has been recorded yet": it tested for an empty answer, and the backend has always returned prose in that case. The backend now sets `reason: "no_transcripts"` (additive, prose retained for an older bot) and the bot branches on it. `/record start`'s refusal now says that Quest Board only recognises a GM whose Discord account is linked, and names `/link` — the GM lookup only ever returned verified-linked GMs, and never said so. All changes are additive: no BOT_CONTRACT_VERSION bump. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>Four faults, all of them about a recording nobody could account for. **The command asked for something Discord could not give you.** `/record start <session_id>` needed a UUID, and no other command in the bot supplies one: `/next` shows a session and omits its id, `/history` lists only completed ones. The answer in practice was to open a browser and copy it out of the address bar, which is not where a GM is at the moment a session starts. The argument now autocompletes against a new additive endpoint, GET /api/bot/guilds/{id}/sessions/recordable — confirmed sessions from the last twelve hours onwards plus anything already in progress, because a GM reaches for /record start *after* the start time, not before it. **A wrong-but-well-formed UUID was accepted.** The metadata fetch was best-effort and swallowed its failures, so a 404 became visible at stop, hours later, with the audio already captured against an id nothing would accept. A definite 404 now refuses the start. Anything else — a timeout, a 5xx, DNS — still starts: it is not evidence the session is wrong, and capture is the part that cannot be redone. **Every recording announced itself as "Unknown Session".** The slash path fetched the session, assigned campaign_name and game_system, and silently skipped the title — while the identical block on the API path had always set it. The title now reaches both the start embed and the upload embed, with a regression test on the slash path, which nothing had exercised. **A restart made a recording vanish without a word.** #399 already keeps the audio: raw tracks are converted and handed off at the next start, and the sweep no longer deletes them. What it could not do is say so, because the channel id, session id and title lived only in `_active`, a dict in the process that died. The channel was left showing "🔴 Recording started" from four hours earlier, `/record stop` denied that anything was running, and a GM had no way to learn the audio was safe. Each start now writes those facts to Redis DB 0 (`qb:bot:recording:<guild_id>`), and a new pass runs straight after #399's recovery to tell the channel which of three things happened: the audio was recovered and is being transcribed, it is kept but has not reached the backend yet, or nothing had been captured. `/record stop` reads the same record and answers honestly. Redis is best-effort throughout — a Redis outage costs the message, never the session. Also: `/record stop` deferred and then only posted to the channel, leaving "Quest Board is thinking…" on the GM's screen for ever; it now answers its own interaction. And the failure strings are rewritten for the people reading them — no exception text in a public channel (it goes to the log), and "a GM can retry from the session page" with a link, instead of advice to use a console a hosted customer has never seen. All API changes are additive: no BOT_CONTRACT_VERSION bump. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>`GET /auth/link` is opened by a **browser**, from a link the bot DMs, and two of its outcomes answered that browser with raw JSON on a blank page: `{"detail":"Link token is invalid or expired"}` and `{"detail":"This Discord account is already linked to a different Questboard user"}`. The second one is a trap, not just an ugly page. The bot's poller had `linked: true|false` and the flow has four outcomes, so a *conflict* was indistinguishable from "hasn't clicked yet" — the poller waited out its full ten minutes and then DMed the player that the link had **expired**. It had not; it had been refused, on grounds no amount of running `/link` could change, because the fix is `/unlink` on the *other* account and nothing in the product had ever said so. A player who hit this could loop for ever. So: * both failures render a small self-contained page (`app/auth/link_pages.py`) that says what happened and what to do next, naming `/unlink` in the conflict case. Not a redirect into the SPA: that needs a route, a query-parameter vocabulary and a component for a page nobody should see twice, and it lands an unauthenticated reader on an app that bounces them to a login they do not need; * `/api/bot/link-status/{token}` gains an additive `state` (`pending|linked|expired|conflict`), fed by a failure record the link route writes to Redis. The bot branches on it and says the right thing *as soon as it is known* rather than at the end of the window; * `SecurityHeadersMiddleware` relaxes CSP for HTML responses only, and only to allow an inline stylesheet. The blanket `default-src 'none'` covers style-src by fallback, and nginx's own policy is added on top of the backend's, so the browser enforces the intersection and these pages would otherwise render unstyled. Scripts, images and connections stay denied, and the pages contain no user-controlled text. Two smaller silences went with it. `voting.py`'s "your reaction wasn't counted, run /link" DM was `except discord.Forbidden: pass`, so a player with DMs closed was told nothing at all — the exact silence #409 set out to remove, surviving in the branch that runs before any vote is attempted. It now falls back to a short self-deleting channel note, as does every message the linking poller sends. And the vote reminder listed players who are not linked without telling them to run `/link`. `Profile.jsx` names the command instead of alluding to "the bot's account-linking flow", with the two-line instruction that goes with it. All API changes are additive: no BOT_CONTRACT_VERSION bump. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>822bea8ff4842c864ab8842c864ab8f3a0166759