[Ops] Unify bot event delivery with at-least-once semantics and idempotency #81

New issue

Closed

opened 2026-07-14 19:45:28 +00:00 by claude-bot · 0 comments

claude-bot commented

2026-07-14 19:45:28 +00:00

Contributor

Context

Backend→bot event delivery is split across three mechanisms with different reliability:

Reminders are safe: a DB-backed poller plus session_reminders_sent dedup.
session_proposed goes HTTP-first to the bot's /notify endpoint (_post_bot_notification, webapp/backend/app/tasks/reminder_tasks.py:55-69) with Redis pub/sub as fallback.
session_summarised, session_completed, vote_update, session_summary_approved are pub/sub-only: published via publish_bot_event / publish_bot_event_async (webapp/backend/app/services/bot_pubsub.py:29-46), consumed by listen_for_bot_notifications (bot/questboard_bot/services/redis_bus.py:42-72), dispatched to on_bot_notify (bot/questboard_bot/cogs/notifications.py:155).

Current behavior

Two failure modes:

Silent loss. Redis pub/sub with zero subscribers "succeeds". Any event fired while the bot is down, restarting, or mid-reconnect is silently dropped — no error, no retry, no fallback triggers for the pub/sub-only event types.
Double-post. The HTTP path uses a 10s timeout (reminder_tasks.py:62-67). If the bot posts the embed but the response is lost/slow, the backend falls back to pub/sub and the bot posts the same embed twice. The on_bot_notify dispatcher has no idempotency key.

Fix / Spec

Replace the pub/sub transport with a Redis Stream (e.g. qb:bot:events) plus a consumer group:
- Backend XADDs every event; bot consumes via XREADGROUP, ACKs (XACK) only after the handler completes.
- On reconnect/startup the bot first drains its pending/backlog entries, then blocks for new ones. This is backlog-drain on the stream, NOT a periodic catch-up poll — do not add polling.
- Give the stream a bounded length (MAXLEN ~) so it cannot grow forever.
Every event gets an event_id UUID stamped at publish time.
Bot-side idempotency: keep a short-TTL Redis set (or key-per-id with TTL, e.g. 1h) of handled event_ids; on_bot_notify skips any id already present.
Migrate ALL event types (session_proposed, session_summarised, session_completed, vote_update, session_summary_approved) onto this one mechanism and delete the per-type divergence. The HTTP-first /notify path may remain as a latency optimization, but it must carry the same event_id so the stream copy is deduplicated.
Keep the recording live-status channel (recording:{guild_id}, see redis_bus.py:16-39) as plain pub/sub — ephemeral status is fine to lose.

Acceptance criteria

Integration-style test (or a scripted manual procedure documented in the PR) proving: an event published while the bot process is stopped is delivered exactly once after the bot restarts.
A replayed/duplicate event_id is ignored by the bot (logged at debug, no second Discord post).
All five event types flow through the stream; grep shows no remaining pub/sub publish for them.
Recording status channel unchanged.

References

webapp/backend/app/services/bot_pubsub.py:29-46
webapp/backend/app/tasks/reminder_tasks.py:55-69
bot/questboard_bot/services/redis_bus.py:42-72
bot/questboard_bot/cogs/notifications.py:155

Filed from the July 2026 full-project review.

## Context Backend→bot event delivery is split across three mechanisms with different reliability: - **Reminders** are safe: a DB-backed poller plus `session_reminders_sent` dedup. - **`session_proposed`** goes HTTP-first to the bot's `/notify` endpoint (`_post_bot_notification`, `webapp/backend/app/tasks/reminder_tasks.py:55-69`) with Redis pub/sub as fallback. - **`session_summarised`, `session_completed`, `vote_update`, `session_summary_approved`** are pub/sub-only: published via `publish_bot_event` / `publish_bot_event_async` (`webapp/backend/app/services/bot_pubsub.py:29-46`), consumed by `listen_for_bot_notifications` (`bot/questboard_bot/services/redis_bus.py:42-72`), dispatched to `on_bot_notify` (`bot/questboard_bot/cogs/notifications.py:155`). ## Current behavior Two failure modes: 1. **Silent loss.** Redis pub/sub with zero subscribers "succeeds". Any event fired while the bot is down, restarting, or mid-reconnect is silently dropped — no error, no retry, no fallback triggers for the pub/sub-only event types. 2. **Double-post.** The HTTP path uses a 10s timeout (`reminder_tasks.py:62-67`). If the bot posts the embed but the response is lost/slow, the backend falls back to pub/sub and the bot posts the same embed twice. The `on_bot_notify` dispatcher has no idempotency key. ## Fix / Spec 1. Replace the pub/sub transport with a **Redis Stream** (e.g. `qb:bot:events`) plus a consumer group: - Backend `XADD`s every event; bot consumes via `XREADGROUP`, ACKs (`XACK`) only after the handler completes. - On reconnect/startup the bot first drains its pending/backlog entries, then blocks for new ones. This is **backlog-drain on the stream, NOT a periodic catch-up poll — do not add polling**. - Give the stream a bounded length (`MAXLEN ~`) so it cannot grow forever. 2. Every event gets an **`event_id` UUID** stamped at publish time. 3. Bot-side idempotency: keep a short-TTL Redis set (or key-per-id with TTL, e.g. 1h) of handled `event_id`s; `on_bot_notify` skips any id already present. 4. **Migrate ALL event types** (`session_proposed`, `session_summarised`, `session_completed`, `vote_update`, `session_summary_approved`) onto this one mechanism and delete the per-type divergence. The HTTP-first `/notify` path may remain as a latency optimization, but it must carry the same `event_id` so the stream copy is deduplicated. 5. **Keep the recording live-status channel** (`recording:{guild_id}`, see `redis_bus.py:16-39`) as plain pub/sub — ephemeral status is fine to lose. ## Acceptance criteria - [ ] Integration-style test (or a scripted manual procedure documented in the PR) proving: an event published while the bot process is stopped is delivered exactly once after the bot restarts. - [ ] A replayed/duplicate `event_id` is ignored by the bot (logged at debug, no second Discord post). - [ ] All five event types flow through the stream; grep shows no remaining pub/sub publish for them. - [ ] Recording status channel unchanged. ## References - `webapp/backend/app/services/bot_pubsub.py:29-46` - `webapp/backend/app/tasks/reminder_tasks.py:55-69` - `bot/questboard_bot/services/redis_bus.py:42-72` - `bot/questboard_bot/cogs/notifications.py:155` _Filed from the July 2026 full-project review._

claude-bot added the

backend

bot

enhancement

labels

2026-07-14 19:45:47 +00:00

claude-bot added this to the v3.4.0 - Reliability, Operations & Self-Hosting milestone

2026-07-14 19:45:48 +00:00