Skip to content

feat: Add full Discord bot for slash-command requesting (and more!) - #231

Open
NichCodes wants to merge 34 commits into
kikootwo:mainfrom
NichCodes:feature/discord-bot-requesting
Open

feat: Add full Discord bot for slash-command requesting (and more!)#231
NichCodes wants to merge 34 commits into
kikootwo:mainfrom
NichCodes:feature/discord-bot-requesting

Conversation

@NichCodes

@NichCodes NichCodes commented Jun 18, 2026

Copy link
Copy Markdown

Add Discord bot for slash-command requesting (and more!)

Summary

Adds an optional discord.js gateway bot so users can search, request, track, and delete titles directly from Discord — mapped to their existing ReadMeABook account — alongside the Web UI. Includes an in-Discord admin approval flow, a Discord settings tab, and per-user account mapping.

The bot is a process-wide singleton gated entirely on configuration: if it isn't enabled, nothing loads and the rest of the app is unaffected. Care was taken to mirror the existing app's behavior, styling, and conventions throughout — the bot reuses the same request lifecycle as the Web UI rather than introducing a parallel one.

Motivation

ReadMeABook currently talks to Discord outbound only (webhook notifications); all requesting happens in the Web UI. This adds inbound Discord support so users can search, request, check status, and delete without leaving Discord, while keeping a single source of truth for the request lifecycle.

This Discord bot also acts as an alternative method of granting access to your RMAB instance to any server owners who don't want to expose their RMAB's Web UI to the wider internet, letting both users and admins privately make and review requests from wherever they can access Discord -- whether it be in the desktop client, web tab, or mobile app. Which is really nice since RMAB doesn't have a mobile app. (Yet.)

New features

Commands

  • /request <audiobook|ebook> [query] — searches Audible → result dropdown (with details) → confirmation card (cover thumbnail) → creates the request as the mapped user. An optional query skips the search modal and goes straight to results. Audiobooks reuse createRequestForUser; e-books reuse the existing sidecar rule (the audiobook must already be in the library), surfaced as a friendly message when it isn't.
  • /status — paginated rich embeds of the invoker's outstanding requests (admins see all): cover art, author, type, narrator, series, and status footer. Includes a "Cancel a request…" dropdown for cancellable items; cancelling re-renders the page, updates live request cards, and cleans up pending approval messages.
  • /delete — paginated rich embeds with a dropdown of deletable requests (in-flight + completed). Selecting a title shows a two-step confirmation: an enriched preview embed (Duration, Series, Format, Genre, File Size) with Confirm Delete / Cancel, with the dropdown left open so the user can switch titles. Only Confirm commits the cascading soft-delete (removes files from disk, deletes the library item from Audiobookshelf/Plex, and handles download-client torrents/NZBs respecting seeding config). Permission level is configurable (see Configuration).

Both /status and /delete paginate (Previous / Page X of Y / Next) past 10 items. Page and scope are encoded in stateless custom IDs, so pagination survives restarts.

Approval flow

  • When a request needs approval, the bot posts a rich embed (title, author, year, series, cover) to a configured channel, pings a configured admin role, and shows Approve / Deny buttons.
  • Authorized for RMAB admins or holders of the configured admin role. Approving runs the same logic as the Web UI and DMs the requester.
  • Decided messages are locked (buttons disabled); stale/duplicate clicks are handled. Cancelling an awaiting_approval request (from Discord or Web UI) rewrites the embed to 🚫 Request Cancelled with a "Cancelled by" mention and removes the buttons.

Live request cards

  • /request posts a persistent, auto-updating card (cover, description, detail fields, status footer, and a Cancel Request button while in flight). Status changes refresh the card via a hook in send-notification.processor.ts. Message refs are persisted on Request.discordCards (JSON). Placement is configurable (public / dm / both).
  • Actor identity (Requested By / Approved by / Denied by / Cancelled by) renders as a clickable <@id> mention; release year is folded into the embed title (Lonesome Dove (2025)); audiobook-only fields (Narrator, Duration, Format) are omitted for ebooks.

User mapping

  • New User.discordUserId (unique, nullable, indexed) maps a Discord account to an RMAB user.
  • Admins set it per user in the Users page edit modal. Unlinked users get a clear "ask an admin to link your account" message in Discord.

Settings

  • New Discord tab: enable toggle, bot token (encrypted, with Test Token), guild/channel/admin-role IDs, optional separate notify channel, and Resolve Names to fetch human-readable server/channel/role names for confirmation.
  • After a successful token test, a bot-identity pill (avatar + username) links to the bot's Developer Portal page; Test Token and Resolve Names auto-run on tab load when configured.
  • Saving restarts the bot at runtime — no container restart. The enable/disable toggle fully tears the bot up/down (client.destroy() on disable; short-circuits on the disabled gate at start).

Screenshots

New Discord settings tab

Discord Settings Discord Settings (continued) User Role Mangement Discord User Mapping

/request flow

Text Command Dropdown Search Confirmation

Admin approval notification

Admin Approval

/status command

/status command

/delete command

/delete command

Richer Discord webhook notifications

Enriched Discord webhook Notifications

API changes

All additive; no breaking changes to existing endpoints.

  • DELETE /api/requests/:id — replaced the old admin-only hard delete with the full cascading soft-delete service, now enforcing ownership (users delete their own, admins delete any) and returning detailed cleanup results. Added to API_TOKEN_ALLOWED_ENDPOINTS so rmab_ API tokens can manage the full request lifecycle (search → create → delete). Addresses kikootwo/ReadMeABook#228.
  • GET /api/admin/requests/pending-approval and POST /api/admin/requests/:id/approve — added to API_TOKEN_ALLOWED_ENDPOINTS so admin API tokens can list pending requests and approve/deny them programmatically.
  • PUT /api/admin/users/[id] — accepts discordUserId; the select now includes it.
  • Admin settings routes for Discord config: save, test-token, resolve-names (server/channel/role/guild).

Shared services (refactors, no Web UI behavior change)

To guarantee the Web UI and Discord behave identically, two pieces of route-embedded logic were extracted into reusable services; the original routes now call them:

  • processRequestApprovalsrc/lib/services/request-approval.service.ts (used by the approve route and the Discord Approve/Deny buttons).
  • createEbookRequestForUsersrc/lib/services/ebook-request-creator.service.ts (used by the fetch-ebook route and Discord /request ebook).

Architecture

  • Transport: persistent discord.js gateway (WebSocket), not an HTTP interactions endpoint — the app is always-on, so this avoids a public signature-verified URL and lets the bot send proactive pings/DMs.
  • Startup: getDiscordBotService().start() in src/app/api/init/route.ts (once-per-container init, same process as the Bull workers). Idempotent; no-op when unconfigured.
  • Server-only & lazy-loaded: discord.js is in serverExternalPackages, aliased out of the client bundle, and loaded via dynamic import() inside start(). A disabled bot pulls in nothing from discord.js — "off" means zero footprint.
  • Command registration: guild-scoped (instant propagation), re-registered idempotently on each ready.
  • Interaction state: encoded entirely in component customIds (≤100 chars) — no server-side session map, so flows survive restarts.
  • Logging: every handler logs actor context { discordUserId, discordUsername, rmabUserId }.

Database / dependencies

  • Adds users.discord_user_id (TEXT, unique, indexed) via prisma/migrations/20260616000000_add_discord_user_id/. Additive, nullable, applied automatically on startup.
  • Adds discord.js (^14).

Configuration (new discord category)

Key Notes
discord.enabled 'true'/'false'
discord.bot_token encrypted (AES-256-GCM)
discord.guild_id server ID
discord.request_channel_id channel for approval embeds
discord.admin_role_id pinged for approvals; grants Approve/Deny authority
discord.admin_notify_channel_id optional; approval pings go here, else request channel
discord.request_card_mode public (default) / dm / both
discord.requester_role_id optional; restricts who may /request (blank = any linked user)
discord.delete_permission own_only (default) / anyone_any / admin_only / disabled

The bot requires the Server Members privileged intent enabled in the Discord Developer Portal.

/delete permission levels: own_only (users delete own, admins delete all), anyone_any (all linked users delete any), admin_only, disabled.

Additional fixes to the base app

Bugs and UX gaps found and fixed while building this feature, independent of the bot:

Discord webhook notifications (bot-independent)

  • Webhook embeds (Settings → Notifications) now match the bot's visual standard: cover-art thumbnail, year folded into the title, description, and inline Author / Narrator / Duration / Series / Genre fields (Narrator + Duration omitted for ebooks). Metadata is sourced purely from the DB via a new enrichBookMeta helper, so embeds stay rich even with the bot disabled — the provider builds embed JSON with no discord.js import.
  • Added a "Test all event types" checkbox beside Send Test to fire one sample per event type at once.

Correctness / security

  • Self-role-change guard now compares the target's current DB role, not the JWT, so an admin demoted in another session can't re-promote with a stale token.
  • Concurrency-safe approval — request approval claims the awaiting_approval → transition atomically via conditional updateMany, preventing double-enqueued jobs/notifications when two admins approve at once.
  • Ebook notifications render as ebooksrequestType is now threaded through approval/creation so approved/pending ebook notifications no longer show audiobook fields.

UX / styling cleanup

  • Modal backdrops — fixed a Tailwind v4 regression where the removed bg-opacity-* rendered backdrops fully opaque; all modals now use bg-black/50 (Modal, Users edit, Notifications, Jobs, BookDate RecommendationCard).
  • Themed scrollbars — transparent track and theme-aware thumb applied to the Settings tab nav and main page body; fixed the WebKit viewport scrollbar not theming by targeting html instead of body.
  • Audiobookshelf settings — the "Trigger library scan after import" checkbox is now wrapped in a bordered card to match the other sections.

Documentation

All new features, API changes, configuration keys, and fixes above are fully documented in the in-app / repo documentation, following the project's token-efficient format — primarily documentation/integrations/discord-bot.md, with updates to the TABLEOFCONTENTS, database, request-approval, and settings-pages docs.

Testing

  • Build: docker compose build readmeabook succeeds (runs prisma generate + next build).
  • Type-check: tsc --noEmit clean.
  • Tests: full suite green (2595 passed, 4 skipped, 0 failed). New tests/discord/ coverage for the custom-id codec, user/admin resolution, and processRequestApproval parity; updated tests for the additive users API change; new organize-files.processor card-refresh cases.

Separate courtesy fix (unrelated to this feature): a batch of pre-existing React/jsdom tests were already failing under vitest 4 + jsdom 27, which expose localStorage without the Storage API. Resolved with a self-disabling in-memory Storage polyfill in tests/setup.ts so the suite runs clean.

Files of interest

  • Bot core: src/lib/services/discord/ (bot service, interaction router, command definitions, custom-id codec, embeds/, REST helper, handlers).
  • Shared services: src/lib/services/request-approval.service.ts, src/lib/services/ebook-request-creator.service.ts.
  • Webhook notifications: src/lib/services/notification/notification-enrichment.ts, .../providers/discord.provider.ts, src/lib/processors/send-notification.processor.ts.
  • Settings: src/app/admin/settings/tabs/DiscordTab/, src/app/api/admin/settings/discord/.
  • Docs: documentation/integrations/discord-bot.md (+ TOC, database, request-approval, settings-pages updates).

NichCodes added 18 commits June 16, 2026 20:48
Add a persistent discord.js gateway bot so linked users can request and
manage titles from Discord, mapped to their existing RMAB user.

Commands:
- /checkout <audiobook|ebook>: search Audible, pick from a dropdown, confirm,
  and create a request (ebooks reuse the sidecar "must already own" rule).
- /status: list outstanding requests (admins see all).
- /delete: remove a request (own only; admins any).

Approval: requests needing approval post a role-pinged embed with Approve/Deny
buttons in a configured channel; approving notifies the requester via DM.

Supporting changes:
- User.discordUserId (unique) + migration; admins set it on the Users page.
- Extract processRequestApproval and createEbookRequestForUser into shared
  services so the Web UI and Discord run identical code paths.
- New Discord settings tab (bot token + test, guild/channel/role IDs with
  name resolution); bot starts at app init and restarts on settings save.
- Actor-aware logging (Discord ID + display name + RMAB user).
- Docs: integrations/discord-bot.md + TOC/database/approval/settings updates.

Per-user library tagging is intentionally out of scope for this change.
Channel name resolution used GET /channels/{id}, which requires per-channel
View permission and returned 50001 Missing Access for restricted approval
channels, even though the bot was in the guild. Resolve via the guild-level
GET /guilds/{id}/channels listing (consistent with role resolution), falling
back to the direct fetch when no guildId is available.
The log appended 'd' to the action token, producing 'Request denyd via
Discord' for denials. Derive proper past tense (approved/denied) instead.
Discord request cards & approval embeds:
- Cancel awaiting-approval requests now rewrite the approval embed to
  Cancelled (with @mention) and drop the Approve/Deny buttons
- Render actor identity (Requested By / Approved / Denied / Cancelled by)
  as clickable @mentions in field values; status-only decision titles
- Add Genre field (up to two)
- Move release year into the title in parentheses, e.g. 'Lonesome Dove (2025)'
- Omit Narrator/Duration/Format for ebooks (audiobook-only)
- Show only the top-listed Author and Narrator

Admin UI / styling:
- Fix Tailwind v4 opaque-backdrop regression (bg-opacity-* -> bg-black/50)
  across all modals
- Add theme-aware scrollbars (transparent track, white thumb in dark mode)
  on the settings nav and main page scroll
- Widen the 'Link Discord Usernames' button so its label no longer wraps
- Wrap the Audiobookshelf 'scan after import' checkbox in a bordered card
Add a bot identity pill (avatar + username + external-link icon) next to
the Test Token button that links to the bot's Developer Portal page.
Auto-run Test Token and Resolve Names when the Discord tab loads so
admins see results immediately. Resolve Guild ID to show the server name.
Add a clickable Developer Portal link in the Bot Token helper text
(Input.helperText widened to ReactNode). Fix bot re-enable failure by
clearing the config service cache before restart — raw Prisma upserts
bypassed cache invalidation, so a disable/re-enable within the 60s TTL
read stale config and the bot never reconnected.
…ELETE API

- /delete now shows completed requests (available/downloaded) in addition
  to in-flight ones, so users can signal they are done with a book and
  trigger full cleanup (files, ABS/Plex library item, download client).

- New discord.delete_permission setting (own_only/anyone_any/admin_only/
  disabled) controls who may use /delete, with a matching dropdown in the
  Discord settings tab. Enforced at both command and select-menu time.

- DELETE /api/requests/:id now uses the cascading deleteRequest service
  instead of a raw hard delete, with ownership enforcement (users delete
  own, admins delete any). Added to the API token allowlist so third-party
  integrations with rmab_ tokens can manage the full request lifecycle.
… and cancel-from-status

- /status and /delete now show paginated rich embeds (cover art, author, narrator, series, status footer) instead of flat text lists, with Previous/Next buttons for lists over 10
- /status includes a "Cancel a request…" select dropdown that cancels and re-renders the page
- /delete shows a rich confirmation embed with the deleted title's metadata
- /request accepts an optional query parameter to skip the search modal
Discord webhook notifications now match the bot's request-card look without
requiring the bot to be loaded:

- Add optional book metadata (cover, narrator, series, year, genres,
  duration, description) to NotificationPayload
- New notification-enrichment.ts: DB-only enrichment (Audiobook +
  AudibleCache) wired into the send-notification processor, so all existing
  call sites are unchanged
- Rewrite the Discord provider embed: cover thumbnail, year folded into the
  title, description, and inline Author/Narrator/Duration/Series/Genre fields
  (narrator + duration suppressed for ebooks). Still raw embed JSON — no
  discord.js dependency
- Add a "Test all event types" checkbox beside Send Test that fires one
  richly-formatted sample per event; test fixture uses a real public cover

Also fix the test suite: jsdom 27 under vitest 4 exposed localStorage as an
empty object, breaking all .tsx tests — add an in-memory Storage polyfill in
tests/setup.ts (self-disabling if the runtime ever provides a real one).
…hardening

Correctness/security:
- Compare self-role-change guard against the DB role, not the JWT, so a
  stale admin token can't re-promote itself (admin user PUT route).
- Claim the approval transition atomically (conditional updateMany gated on
  status) so concurrent approvals can't double-enqueue jobs/notifications.
- Pass requestType to notifications so approved/pending ebook requests render
  ebook-typed embeds (approval service + both request creators).
- Build the live request card from the request's live DB status to close the
  card-ref persistence race.
- Reject empty/negative/non-integer pages in the custom-id decoder.
- Store null (not a synthetic "discord:<id>") as deletedBy when a Discord
  admin-role holder has no linked RMAB account.

Efficiency/cleanup:
- Reuse one REST client across resolveMembersByIds so discord.js throttles
  the batch through a single set of rate-limit buckets.
- Split embeds.ts (over the file-size cap) into embeds/ (book-fields,
  request-cards, approval, lists + barrel) and route list/confirm embeds
  and the request-select menus through shared helpers.

Docs updated for the concurrency-safe approval, the embeds module layout,
and the relationship between the bot approval message and webhook
notifications. Tests updated/added for the atomic claim, ebook notification
typing, the DB-role guard, and the custom-id page guard.

@kikootwo kikootwo left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First, credit where it's due: this is an impressive PR. I went through the security surfaces, both service extractions (diffed line by line against the old route logic), the customId codec, and the bot lifecycle. The authorization model is done right (every handler re-resolves the actor and re-checks permissions at click time instead of trusting the customId), the extractions genuinely match the old behavior, the zero-footprint-when-disabled claim holds up, token handling is clean (encrypted at rest, masked on read, never echoed back), and Discord failures never block the core request pipeline. Docs are exactly the format this project wants.

That said, there is one real lifecycle bug and a few things I want fixed before this merges. Full details are inline on the relevant lines; the short version:

Critical

  1. processRequestApproval can approve a soft-deleted (cancelled) request and enqueue a real download for it. Neither the fetch nor the two atomic claims filter deletedAt: null, and this PR adds several new ways to cancel an awaiting_approval request while the approval embed keeps live Approve/Deny buttons.

Important
2. The atomic claim now flips status to downloading (and nulls selectedTorrent) before enqueuing the job. If enqueue throws, the request is stuck in downloading with no job and the torrent choice is lost. Needs a compensating rollback.
3. Deleting a request from the web UI or via API token never rewrites the Discord request card or the approval embed, so live buttons linger forever. This is also what makes issue 1 reachable in practice. Hook the rewrites into deleteRequest itself so every surface converges.
4. restart() races an in-flight start(): settings can save but silently not apply.
5. The bot singleton uses a module-level let instead of the globalThis pattern we use in src/lib/db.ts, so dev-mode HMR can spawn duplicate gateway connections that handle every interaction twice.

Smaller things I still want addressed
6. The Discord settings PUT writes config in a non-transactional loop; a mid-loop failure leaves mixed old/new config and skips the bot restart.
7. The fetch-ebook retry path now returns 201 where the old route returned 200. Harmless, but it is the one deviation from the "mirrors exactly" claim, so either restore it or call it out.
8. Deny never DMs the requester even though the PR description says decisions do.

None of this dents the overall design, the bones here are really good. Fix 1 through 5, sweep the smaller ones, and I'm happy to merge.

Comment thread src/lib/services/request-approval.service.ts Outdated
Comment thread src/lib/services/request-approval.service.ts
Comment thread src/app/api/requests/[id]/route.ts
Comment thread src/lib/services/discord/discord-bot.service.ts
Comment thread src/lib/services/discord/discord-bot.service.ts Outdated
Comment thread src/app/api/admin/settings/discord/route.ts Outdated
Comment thread src/app/api/audiobooks/[asin]/fetch-ebook/route.ts Outdated
Comment thread src/lib/services/discord/handlers/approval.handler.ts Outdated
@NichCodes

Copy link
Copy Markdown
Author

Thanks for looking at it. I'll work through each of these issues and ping you for a re-review once they're (hopefully) fixed.

Soft delete sets deletedAt but intentionally leaves status untouched, so a
cancelled request still matched { id, status: 'awaiting_approval' }. An admin
clicking Approve on a stale Discord embed could claim the row and enqueue a
real download for a request the user had already deleted. Because the web
pending-approval list does filter deletedAt, the orphaned download never
surfaced for anyone to clean up.

- Fetch via findFirst filtered on deletedAt: null (findUnique cannot filter on
  a non-unique field), returning not_found for a soft-deleted row.
- Add deletedAt: null to both atomic claims (approve and deny).
- staleStatusResult now reports not_found rather than invalid_status when the
  claim failed because the request was cancelled.

Tests: 4 new cases covering the fetch scope, the soft-deleted approve path, a
mid-flight cancellation, and the deny claim. Route-level tests updated for the
findUnique -> findFirst change.
The atomic claim that closed the double-approve race also reordered the old
route: status flips to downloading (and selectedTorrent is cleared) before the
job is enqueued. If addDownloadJob or addStartDirectDownloadJob then threw, the
catch returned reason: 'error' but left the row stranded in downloading with no
job attached and the admin's torrent choice gone -- unrecoverable from any
surface, where the old enqueue-first ordering had simply left it awaiting
approval for a retry.

- Add releaseClaim(), which restores awaiting_approval and the cleared torrent,
  gated on the status the claim set so a concurrent transition is not clobbered.
- Wrap the download enqueue (including the Anna's Archive downloadHistory
  writes) and the search enqueue, rolling back and returning a retry-friendly
  error on failure.
- No notification fires for a failed approval.

Tests: 4 new cases covering download, Anna's Archive direct-download, and search
enqueue failures, plus a guard that a successful enqueue writes no compensation.
Deleting from the Web UI or via an API token never touched the Discord side:
the request card kept a live Cancel Request button forever, and the approval
embed kept live Approve/Deny buttons. That lingering embed is what made the
missing deletedAt guard reachable in practice.

Rather than patching each surface, the rewrites now live in deleteRequest
itself, so every path converges -- Web UI, API token, admin route, Discord
/status and /delete, and the reported-issue cleanup.

- Add syncDiscordOnDelete(): forces the card to its cancelled render and, when
  the request was awaiting approval, strips the approval embed's buttons. Gated
  on a running bot and dynamically imported, matching the processor hook, so
  discord.js stays unloaded when the bot is disabled. Never throws.
- deleteRequest takes an optional actorDiscordUserId so a Discord-initiated
  delete still attributes "Cancelled by" to the interacting user; other callers
  fall back to the deleting user's linked account.
- applyApprovalCancellation and cancelApprovalMessage accept null, omitting the
  field instead of rendering a broken <@null> mention for actors with no link.
- Drop the now-duplicated calls from the /status and /delete handlers, and
  rewrite the card when a button handler finds the request already gone, so no
  dead button is left behind.

Tests: 6 new cases covering the bot-running and bot-disabled paths, the
awaiting-approval condition, actor precedence, unlinked-actor attribution, and
that a Discord failure never fails the deletion.
The gateway client is only assigned after login() resolves, so a stop() landing
mid-login found nothing to destroy and the follow-up start() returned early on
the `starting` boolean. The in-flight start then completed with the old config
and assigned this.client, leaving the admin's saved settings silently unapplied
while the previous connection stayed live. Container init racing an admin save,
or two quick saves, both hit this.

- `starting` becomes the in-flight promise, so a concurrent start joins it
  rather than returning early, and restart() drains it before tearing down.
- Add a generation counter, bumped by stop(). A start whose epoch moved during
  login() destroys its own connection instead of publishing a stale client, and
  a superseded client no longer flips the service to ready.
- stop() detaches this.client before awaiting destroy(), so a concurrent start
  never observes a half-torn-down client.

Tests: new bot-lifecycle suite driving a controllable discord.js mock, covering
restart racing an in-flight start (asserting the new token is the one that
connects), stop superseding a start, ready suppression on a discarded client,
and start() joining rather than duplicating. 3 of the 4 fail against the
previous implementation.
A module-level `let` means Next dev-mode HMR can re-evaluate the module, build a
second bot service and a second gateway connection, and then handle every
interaction twice -- duplicate requests and approvals while developing.
Production was unaffected, but this matches the pattern already used for the
Prisma client in src/lib/db.ts.

Not gated on NODE_ENV the way db.ts is, since a duplicated gateway connection is
worth preventing in every environment.

Tests: new case asserting the service survives a module re-evaluation as the
same instance (fails against the module-level `let`). The lifecycle helper now
clears the global explicitly, since resetModules alone no longer yields a fresh
service.
The plain-key upserts and the bot-token write ran as independent statements, so
a mid-loop failure left the config half old and half new -- and then skipped the
cache clear and bot restart below, leaving the bot running on a configuration
that was never fully persisted.

Collect the upserts (including the conditional token write) and run them as one
prisma.$transaction, so a save is all-or-nothing. The cache clear and restart
stay outside it, running only after the transaction commits.

Tests: new suite for the route covering the single-transaction write, token
inclusion, masked-token skip, and that a failed transaction neither clears the
config cache nor restarts the bot.
Extracting the route logic into createEbookRequestForUser collapsed two status
codes into one: the old route returned 200 when it re-drove an existing
retryable request and 201 only for a fresh create, while the wrapper returned
201 for every success. Nothing broke, since the Web UI only checks res.ok, but
it was the one real deviation from the "mirrors the original route exactly"
claim, so restore it rather than document it.

- CreateEbookRequestResult's success variant gains `created`, set false on the
  retry path and true on both create paths.
- The route maps it to 201 vs 200.

The Discord /request ebook flow ignores the field, so its behavior is unchanged.

Tests: new suite for the route covering created/retry/approval status codes, the
reason -> status mapping for all five failure cases, and ASIN validation.
The PR description says decisions are DMed to the requester, but notifyRequester
only fired on approve. The card footer covered it when request cards were
enabled, so a plain deny with cards off was completely silent for the requester.

Generalize notifyRequester over the decision and call it for both outcomes.

Tests: new suite covering the deny DM, the approve DM, silence when the
requester has no linked Discord account, and silence when the decision could not
be applied.
Keeps the docs in step with the behavior changes from this review pass, in the
project's token-efficient format.

- request-approval.md: approval guards section (soft-delete filter, atomic
  claim, enqueue-failure compensation) + two Fixed Issues entries.
- request-deletion.md: new Discord Convergence section covering the hoisted card
  and approval-embed rewrites, the deleteRequest signature change, and the
  bot-disabled edge case + a Fixed Issues entry.
- discord-bot.md: decision DM now sent on deny as well as approve; restart
  concurrency, globalThis singleton, and transactional settings save under
  Enable/Disable lifecycle; cancelled-request approval under edge cases.
- ebook-sidecar.md: new API section documenting POST fetch-ebook and its
  201-vs-200 status contract, which had no coverage despite the route header
  pointing here.
Brings in v1.2.2 (10 commits), including kikootwo#276 "delete requests using stored
media path", which rewrote the file-deletion block of deleteRequest that this
branch also touches.

Conflicts resolved (both were additive collisions, nothing dropped):

- tests/services/request-delete.service.test.ts: kept this branch's Discord
  mocks and dropped the buildAudiobookPath mock, which kikootwo#276 made dead when the
  service stopped rebuilding paths and started deleting the persisted filePath.
- documentation/admin-features/request-deletion.md: kept both sides' entries,
  renumbering ours after theirs (edge case 16 -> 18, Fixed Issue 2 -> 3).

src/lib/services/request-delete.service.ts auto-merged without conflict and was
verified by hand rather than trusted: kikootwo#276's rewrite sits in the file-deletion
block while this branch's changes are the signature and the post-delete
syncDiscordOnDelete call, so both survive intact.

Verified on the merged tree: tsc clean, full suite 2684 passed / 4 skipped / 0
failed, including kikootwo#276's stored-path tests alongside this branch's Discord
convergence cases.
Manual testing found the previous convergence fix incomplete. The admin
dashboard's Deny button posts action:'deny' to the approve route, which runs
processRequestApproval -- it never calls deleteRequest, so hooking Discord into
deletion alone left the approval embed live with working Approve/Deny buttons,
never refreshed the request card, and never notified the requester. The stale
embed was then clickable long after the decision, which is the exact hazard the
deletedAt guard was added to defend against.

The mocked unit tests passed because they exercised deleteRequest's callers
rather than the real Web UI path, so they verified the assumption instead of the
behavior.

- Add syncDiscordOnDecision() to processRequestApproval, fired on all three
  success paths: rewrites the approval message to decided (dropping its
  buttons), refreshes request cards, and DMs the requester. Gated on a running
  bot, dynamically imported, never throws.
- Move the requester DM out of the Discord button handler into that shared path,
  so Web UI and API-token decisions notify too.
- Add applyDecisionToApprovalMessage(), notifyRequesterOfDecision(), and
  reconcileApprovalMessage() to discord-cards; the last re-renders a stale
  approval message from current DB state instead of leaving it reading "Pending"
  with merely greyed-out buttons.
- applyApprovalDecision accepts a null actor, omitting the "by" field rather
  than rendering <@null> for a Web UI admin with no linked Discord account.
- Strip the now-duplicated embed edit, card refresh, and DM from the button
  handler so the two surfaces cannot drift apart again.
- Log every early return in the Discord sync helpers. Both hooks previously
  logged only on failure, so a silent no-op was invisible -- which is what made
  this hard to diagnose.

Tests: replace the handler-level DM test, which was pinned to the wrong seam,
with 7 cases driving the shared service and the real web approve route,
including actor attribution, bot-disabled, and throwing-sync. 5 of the 7 fail
against the previous implementation.

Verified end to end against a live Discord guild: web-side deny now rewrites the
embed and card and DMs the requester, and concurrent settings saves leave
exactly one live gateway.
buildSearchSelect already received mediaType but used it only for the customId,
so every result row advertised "Narrated by ..." even for an e-book search. An
e-book has no narrator, and the confirmation card and request embeds already omit
narrator/duration/format for ebooks, so the dropdown was the odd one out.

Tests: 3 cases covering the audiobook row keeping the narrator, the e-book row
dropping it, and author/year surviving on both.
/request always listed "E-book" as a type, even with every e-book source
disabled, so the only possible outcome was a rejection at the end of the flow.
And because e-books are sidecars, a request also fails unless the audiobook is
already in the library -- a constraint the Web UI hides by only showing "Fetch
Ebook" on owned titles, but which Discord users met only after picking a title
from the whole Audible catalogue.

- Extract isEbookRequestingEnabled() from createEbookRequestForUser, including
  its legacy ebook_sidecar_enabled fallback, and share it with the command
  definitions so the offered types and the request path cannot drift.
- buildCommandDefinitions omits the E-book choice when no source is enabled. The
  request path still guards independently, for clients holding a stale command
  list.
- Add DiscordBotService.refreshCommands() and call it when e-book settings are
  saved. Without it the choice would only appear or disappear after an unrelated
  Discord settings save restarted the bot.
- Surface the sidecar rule up front: the e-book search modal says the title must
  already be in the library, and both result dropdowns repeat it, so the
  constraint is visible before a title is chosen rather than after.

Tests: 3 cases on the offered type choices, 5 on the enablement rule including
the legacy-key precedence.
Requests Awaiting Approval fell back to a generic placeholder for anyone who
requested via Discord: a local account has no Plex/OIDC avatar, so avatarUrl was
null. Discord CDN URLs are /avatars/{userId}/{hash} and the hash is not derivable
from the user id we store, so there was nothing to render.

Rather than calling the Discord API, capture the URL from interactions we are
already handling. captureDiscordAvatar() runs from the interaction router, the
single point every command, button, and select passes through, so the cache is
refreshed for free and self-heals when someone changes their avatar. It is
fire-and-forget so it never delays acknowledging within Discord's 3-second
window, and writes only when the URL actually changed so a burst of interactions
is not a write per click.

Stored in a new discord_avatar_url column rather than reusing avatar_url, so a
Plex/OIDC avatar is never overwritten; the dashboard prefers the Discord avatar
and falls back to avatarUrl, then the placeholder. Users with no custom Discord
avatar are left null so the existing placeholder still applies rather than
pinning Discord's default image.

Migration is additive and nullable. Populates on a user's next interaction; it
does not backfill.
Brings the docs in step with the preceding commits, in the project's
token-efficient format.

- discord-bot.md: decisions now sync from the shared approval service, so any
  surface (Discord buttons, Web UI Deny, API token) rewrites the approval
  message, refreshes the card, and DMs the requester; stale clicks reconcile
  from current DB state. Plus the avatar cache under User Mapping, the E-book
  type gating and up-front sidecar warning under Commands, narrator omission in
  search results, refreshCommands() under the lifecycle, and a note on why the
  sync helpers log their skips.
- request-approval.md: syncDiscordOnDecision in the approval-guards section, and
  a Fixed Issues entry for the Web UI decisions that left the embed live.
- ebook-sidecar.md: new Enablement Rule section covering
  isEbookRequestingEnabled() as the single source of truth shared by the request
  path and the /request type choices, and where the sidecar rule is surfaced.
- database.md: users.discord_avatar_url.
- TABLEOFCONTENTS.md: three new mappings.

Verified every symbol and relative source link named in the new prose resolves
against the tree.
@NichCodes
NichCodes requested a review from kikootwo August 20, 2026 04:02
@NichCodes

Copy link
Copy Markdown
Author

Finally got around to this PR (sorry for the delay). Ran through it again and fixed/tested the stuff you pointed out (along with a couple other fixes along the way). Had to also fix a couple merge conflicts/regressions in the meantime, but it should otherwise be nearly good to go. Give it another look whenever you've got the time and lemme know what you think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants