feat(bot/webapp): a misconfigured channel fails loudly, and Discord votes answer back (phase 5 lane A) #523
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/phase5a-notifications-and-votes"
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 #369, closes #371. Phase 5 lane A of the v4.3.0 build order (#514). Three commits. One migration:
f0ab1c2d3e4f(nullable JSONBcampaigns.discord_delivery_status) one9fa0b1c2d3e;alembic headsis one head. NoBOT_CONTRACT_VERSIONbump (reasoning below).#369 a wrong channel ID no longer swallows every notification.
_resolve_channelreturns why it failed (invalid_id/not_found/forbidden/not_text/unknown, pluscannot_sendfrom aForbiddenon the post itself, the commonest misconfiguration after a wrong ID), and the bot reports it through a new additivePOST /api/bot/campaigns/{id}/delivery-status. The record is written before the stream entry is acked; if the record fails too, that is logged at ERROR with campaign, reason and event type, so nothing is ever both unrecorded and acked. NULL is not "ok" and nothing is backfilled. The status is onCampaignResponseandCampaignDetailshows a banner at the top of the page, outside the collapsed Edit form; it clears on the first successful post, including the first after a restart. The "Send test message" button had three further problems (admin-only on a GM page, reported success on publishing to Redis, tested both fields as one) and is replaced by two GM-authorised, synchronous tests viaPOST /api/campaigns/{id}/discord-test: the channel test calls a newPOST /test-channelon the bot which resolves and posts before answering and writes the verdict; the webhook test is guarded byis_discord_webhook_url. The channel input validates 17–20 digits with a hint.#371 Discord votes answer back. Reactions are not interactions, so every reaction now gets a self-deleting channel message mentioning the player (15 s success, 30 s failure with the reason); DMs are gone from the cog, reconciled with #409 and #392 (the unlinked-user prompt is a self-deleting channel note; the ❗ fallback survives).
vote_service.notify_vote_castis called by both the web and bot vote paths.bot_submit_votereturns 409 on a closed poll;api_clientraisesVotingClosedError(aQuestBoardApiErrorsubclass) and the bot removes the reaction and repeats the sentence with no retry offer; confirming a session relabels the poll "Voting closed" and drops the instructions but keeps the option list. "Maybe" is scoped down to a legend on the poll (a third reaction per slot would mean ten seeded reactions per poll). Slate editing needed more than wiring up the unused API calls: the bot's reaction→slot mapping is positional, so both timeslot endpoints publish a newsession_slate_changedevent and the bot rebuilds embed, reactions and mapping in place, mapping first.SessionDetailgains "+ Add another time option" and a per-slot Remove with a confirm naming the votes it drops. The Redis in-memory fallback warns loudly and every dropped reaction is logged.Contract:
session_slate_changedis a new event type an older bot ignores; the 409 becomes the transport failure an older bot already handles (it says the vote did not count, which is true). No shape changed and no older bot can be led into recording a vote incorrectly.Rebased over lane B (#521):
VotingClosedErrornow sits on lane B'sQuestBoardApiError, and the rebase caught a live bug where the closed-poll message would have rendered as the generic text; a test now asserts the seam. Bot 432 passed, backend 2405 passed / 13 skipped, frontend 539 passed, ruff and version-sync clean, migration round-trips up/down/up on Postgres 16. Follow-up filed as #522 (a constructor guard onQuestBoardApiError).🤖 Generated with Claude Code
The bot notification channel is a Discord snowflake a GM types into a plain text field. When it is wrong — a typo, a channel deleted since, a channel the bot cannot see or cannot post in — `_resolve_channel` returned None, logged a warning nobody reads, and `_process_entry` XACKed the entry exactly as if it had been delivered. Every notification that campaign would ever send vanished, permanently, with no error anywhere in the product. The ack stays: an entry the bot cannot handle must not wedge the consumer group for every other campaign. What changes is that it is no longer the end of the story. - `_resolve_channel` now returns *why* it failed (invalid_id, not_found, forbidden, not_text, unknown) with a sentence written for the GM who typed the ID, and `on_bot_notify` reports it through a new additive endpoint, `POST /api/bot/campaigns/{id}/delivery-status`, before returning — so the record is written before the caller acks. `discord.Forbidden` raised by the *post* is caught separately as `cannot_send`: a channel the bot can see but not speak in was indistinguishable from success. - The record lands in a new nullable JSONB `campaigns.discord_delivery_status` (migration `f0ab1c2d3e4f`, chained on `e9fa0b1c2d3e`; one head). NULL is deliberately not "ok" — a channel nobody has posted to has demonstrated nothing — so nothing is backfilled. - `CampaignResponse` exposes it to every member, not just the GM: it holds a channel ID, a reason code and a sentence, no secrets, and a player wondering why the reminders stopped is owed the same answer. `CampaignDetail` renders it as a banner at the *top of the page*, outside the collapsed Edit form — putting the warning behind the same disclosure as the broken field would inherit the invisibility this fixes. - If the record itself cannot be written, that is logged at error with the campaign, reason and event type, because the entry is about to be acked and the log line is then the only trace that will ever exist. - The warning clears itself: the first successful post per campaign reports "ok", including the first after a restart, so a banner left by a dead process cannot outlive the problem. The "Send test message" button is fixed in the same change, because it had the same lie in it. It called the admin-only `/api/admin/settings/bot/test-notify` from a page any GM can open (403 for every self-hoster who is not their own admin), it published to Redis and reported success on the *publish*, and it tested the bot channel and the webhook as one thing. There are now two buttons, one per field: `POST /api/campaigns/{id}/discord-test` is GM-authorised and synchronous — for a channel it calls a new `POST /test-channel` on the bot's internal server, which resolves and posts before answering, and writes the result to `discord_delivery_status` so a passing test clears the banner and a failing one raises it. The channel field also rejects anything that is not 17–20 digits before it can be saved, with a line saying where to find the ID. No `BOT_CONTRACT_VERSION` bump: `/api/bot/campaigns/{id}/delivery-status` is purely additive, and an older bot simply never calls it. `/test-channel` is backend→bot rather than part of that contract, and a backend talking to an older bot gets a 404 it reports as "upgrade the bot image" rather than blaming the channel ID. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>