Retire djsx, and get the codebase to one coherent shape by zerebos · Pull Request #8 · BetterDiscord/BetterDiscordBot

zerebos · 2026-08-28T12:42:11Z

Started as "should we keep the JSX experiment or go back to builders?" and turned into the full cleanup. 20 commits, each one reviewable on its own.

The answer to the original question was neither. The builder pattern really is bad — new ContainerBuilder().addSectionComponents(new SectionBuilder()...) was the worst-reading code in the repo. But JSX cost more than it bought: TypeScript has exactly one global JSX.Element type, declared here as BaseComponentData, so every JSX expression needed an as cast and the type checker stopped helping precisely at the boundary where mistakes are expensive. The third option is plain typed object literals — which is what djsx compiled to anyway. djsx/Container.tsx was nine lines whose entire job was to produce {type: ComponentType.Container, components: [...]}.

That's not theoretical: it had already shipped a bug. tags.tsx rendered "Tag `x` has been $updated successfully!" because ${...} inside JSX text isn't interpolated.

What changed

	before	after
`tsc` errors	9	0
`eslint`	couldn't run from a clean clone	0
tests	none	96
`djsx/`	736 lines	deleted
ways to send a message	3	1 + one documented exception
builder-pattern rendering	throughout	none
hand-rolled collectors	4	0
legacy command modules	10	0
inline snowflakes	12 across 6 files	0
casts in tags	18	1

Bugs fixed (15)

Four were completely silent — no error, no log, the feature simply didn't work:

events/joinleave.ts had never fired. It exports an array of two listeners; the loader read .name off the array and called client.on(undefined, …). /moderation joinleave was configuring a channel that could never receive anything.
/cleanname server skipped members. A module-level /g regex used with .test() — which advances lastIndex and resumes there — so results alternated. Reproduced: the old pattern detects 2 of 4 matching inputs.
/cleanname server never showed its results. The completion path called interaction.update() a second time on an already-acknowledged interaction, which throws. It sat on "Fixed 0, Failed 0" no matter how many names it corrected.
A failed addon fetch blanked the cache for an hour. ensureCache stamped the timestamp and cleared the cache before the requests.

Plus: any user could hijack someone else's paginator (assignment before the guard); setMaxValues(0) crashed self-roles on a fresh server; /addons search with no matches threw; an empty addon cache built an illegal select menu; /addons random rendered undefined; three editReply-before-defer paths in /tag showed the generic error instead of the real message; a modlog early-return skipped the rest of the handler when no modlog was configured; a try/catch reported database failures as "modal submission timed out".

Structure

src/framework/ — commands, components, events, and the two interaction mechanisms. The key idea, which is what the old CommandModule was missing: durable UI and ephemeral UI are different things. Registered components carry state in a typed custom id and survive restarts; sessions own a collector and die with the token. Trying to serve both with one mechanism is why collectors kept getting hand-rolled. Full explanation in src/framework/README.md.

The old signature was also unsound — execute: <T extends BaseInteraction = ChatInputCommandInteraction>(i: T) puts T under the caller's control, so every implementation's narrowing was unchecked. addons was declaring <"cached"> on a command that runs in DMs.

Worth a closer look

Five deliberate changes, not accidents:

/about lost its invite buttons. They were commented-out dead code breaking the typecheck; @zerebos confirmed they came from a public bot and don't apply here.
voicetext and cleanname: dm_permission: false → contexts: [Guild]. Deprecated field, now gone repo-wide. These are the only two command payloads that changed — verified against a before/after dump of all ten.
cleanname's progress counters render on one line instead of three inline fields. Components V2 has no field grid.
/about keeps its EmbedBuilder, deliberately, with a comment saying why — same reason as Add reaction roles, autoresponder, moderation actions, and action log #3, and faking a grid with padded text doesn't survive different client widths.
Deploy now skips when commands are unchanged. The container was bulk-overwriting every global command on every restart.

Verification

Every refactor step was checked by dumping all ten command payloads before and after and diffing them. tests/fixtures/command-payloads.json now makes CI do that — confirmed it catches drift by changing one character of a description and watching it fail.

tests/regressions.test.ts has one test per silent bug above, each naming the failure it guards against.

CI runs test → typecheck → lint, on a pinned Bun (latest broke the build mid-session on a lockfile-format change).

Suggested reading order

src/framework/README.md → CLAUDE.md → then the commits in order. The first commit (b3738de) sets up the framework; everything after is migration onto it.

Draft because it's a large surface and worth a look before it's live — but the branch is green and self-consistent at every commit.

🤖 Generated with Claude Code

https://claude.ai/code/session_015hDX4g1EjTN7424A5wnptE

Generated by Claude Code

Introduces src/framework as the pilot for a sound replacement of the CommandModule shape, and converts the first two consumers. Why the old shape was unsafe: execute: <T extends BaseInteraction = ChatInputCommandInteraction>(i: T) => … The generic sits on the method, so the *caller* chooses T. That signature accepts any interaction, and every implementation that narrowed to ChatInputCommandInteraction<"cached"> was unchecked. Meanwhile all five optional handlers were required, which is why botadmin carries an empty `async modal() {}`. What replaces it: - registry.ts defineCommand/defineComponent/defineEvent. A handler's interaction type is fixed by the definition, not the caller. `guildOnly: true` is a claim the dispatcher enforces before calling, which is what earns the <"cached"> narrowing. - ids.ts Typed custom IDs. `customId.split("-")[0]` was an untyped contract between the code minting an ID and the code reading it. Params are now encoded/decoded through codecs; OneOf gives a literal union so a switch can be exhaustive. Enforces Discord's 100-char cap, and a stale ID from before a deploy decodes to a friendly "run the command again" instead of a crash. - session.ts The other half: ephemeral UI owned by one invocation. The ownership check, timeout and disable-on-end are written once. - dispatch.ts One place where a raw Interaction becomes a typed call. All remaining casts live here, each on the line after the runtime check that justifies it. - loader.ts Validates modules instead of casting the dynamic import, so a malformed module fails at boot rather than on first use. Registered components and sessions share one custom-ID space: session IDs carry a `~` prefix the dispatcher skips, and an unknown namespace is ignored rather than treated as an error. Migrated: - selfroles: three defineComponents replace the hand-rolled second routing layer (customId.split("-")[1]) inside button(). Fixes a crash where setMaxValues(assignable.length) was called with 0 on a server that had no configured roles. - paginator: rebuilt on runSession, 116 lines -> 71. Fixes the button interaction being captured before the user check (any user could redirect someone else's paginator), and the final edit dropping IsComponentsV2. Unmigrated commands are unaffected: the dispatcher keeps a legacy path that routes them the old way, to be deleted when the last one is converted. deploy-commands now shares the loader, so what is deployed is exactly what is registered. Incidentally fixed by the validating loader: events/joinleave.ts exports an array of two listeners, which the old loader read `.name` off of, producing client.on(undefined, …). Join/leave logging had never fired. The loader now accepts arrays; the bot registers 11 listeners where it previously registered 9 working ones and 1 dead. Verified: tsc reports the same 9 pre-existing errors as before this change (4 in djsx/, 5 unused symbols in about.ts) and none in the new or modified files. Dispatcher routing, ID round-tripping, the session state machine and paginator navigation were exercised against stub interactions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015hDX4g1EjTN7424A5wnptE

Stateful regexes (src/util/names.ts, events/detectspam.ts) const weirdCharsRegex = /[^A-Za-z0-9\-_\\. ]/g; ... if (!weirdCharsRegex.test(member.displayName)) continue; RegExp.prototype.test advances lastIndex on a /g regex and resumes from there on the next call, so a shared module-level global regex returns alternating answers for the same input. Verified against four display names that all contain disallowed characters: the third returned false. That regex sat inside the member loop of `/cleanname server`, so the command silently skipped a share of the members it should have renamed on every run, and the GuildMemberAdd handler misfired intermittently. The same shape was in detectspam's sketchyRuRegex ("ig" + .test()), where it let roughly half of matching .ru.com links through. Confirmed: the old pattern detects 2 of 4 sketchy hosts, the fixed one detects 4 of 4. The regex was duplicated across commands/cleanname.ts and events/cleanname.ts with a TODO about double maintenance; it now lives in src/util/names.ts with a comment explaining why it must not be global. invitefilter's regexes keep /g — they are used with matchAll, which requires it. detectcryptoscam's has no /g and was already correct. Tag command replies (commands/tags.tsx) - The success message read "Tag `x` has been $updated successfully!". `${isUpdating ? ...}` is template-literal syntax, which JSX does not interpolate; it emitted a literal "$" and treated the rest as a JSX expression. Now `{isUpdating ? ...}`. - create(), update() and delete() called editReply() on their permission-denied paths before anything had deferred or replied, so they threw InteractionNotReplied and the user saw the generic "There was an error while executing this command!" instead of the message written there. create() and update() end in showModal() and so can never defer; their early exits now reply ephemerally. delete() now defers first, which makes every later editReply valid. Addon cache (util/addons.ts) ensureCache stamped addonCacheLastUpdate and cleared both the in-memory and persisted cache before issuing either request. A failed fetch therefore left an empty cache with a fresh timestamp, so every /addons command returned nothing for a full hour. It now fetches into a local array and only replaces the cache once both requests have succeeded, stamping the timestamp last. A failure is logged and the existing cache is served rather than propagating to the user, and the next call retries. Concurrent callers now share one in-flight refresh instead of racing. Verified: display-name and .ru.com checks return stable results across repeated calls; the tag success message renders "updated"/"created" with no stray "$" and keeps both Ephemeral and IsComponentsV2 flags. The addon cache failure path was exercised against real failing requests — no throw reached the caller and the timestamp was not advanced, so each subsequent call retried. The fetch-before-mutate ordering is structural: both requests complete before cache.clear() or either globalDB.set() runs. tsc reports the same 9 pre-existing errors as before (4 in djsx/, 5 unused symbols in about.ts) and none in the modified files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015hDX4g1EjTN7424A5wnptE

The repo had no typescript dependency, no typecheck script, 9 tsc errors, and an eslint setup nobody but the author could run. None of that could gate a pull request, so nothing below stays fixed without it. Dependencies and scripts - Added typescript ^5.9.3. It was never a devDependency, so `tsc` was not available from a clean clone. - @zerebos/eslint-config and @zerebos/eslint-config-typescript pointed at file:../../eslint-configs/packages/*, a path outside the repository, so `bun install` failed on both for anyone else. Both are published; switched to ^1.0.3 and ^1.1.1. A clean `bun install --frozen-lockfile` now succeeds with no failed packages, which also unbreaks the Dockerfile's --production install. - Added `typecheck` (tsc --noEmit) and `lint` (eslint .) scripts, and made `test` exit 0 with a note until there is a suite, so it can sit in CI without being a permanent red. Cleared all 9 type errors - djsx/utils.ts: discord.js types its `components` arrays readonly, so four call sites (ComponentMessage, MediaGallery, Modal, StringSelect) passed a readonly array to childrenToArray, which demanded a mutable one. Widened the parameter; one change fixed all four. - src/commands/about.ts: five unused-symbol errors, all from a commented-out invite-button block. Removed the block, its three imports and its two OAuth URL constants, leaving a comment pointing at 335cf56 where they were parked. Restoring them is a git show away if they were meant to come back. Cleared all 18 lint errors Most were in the framework added last commit, and fixing them improved it: - dispatch.ts: the type checker reports every `as CommandHandler` / `as ComponentHandler` / `as never` in the dispatch path as unnecessary. Removed them. dispatch.ts now performs no casts at all — the runtime guard is still what makes the narrowing sound, but nothing is asserted. - session.ts / paginator.ts: `render`, `reduce` and `renderPage` were declared with method shorthand, which trips unbound-method when destructured. Declared as function-typed properties, which is also more honest since none of them use `this`. - ids.ts: renamed `OneOf` to `oneOf`. It is a factory, not a constructor, and lowercase matches row/container/text. - loader.ts: stopped passing a method reference to a type guard, dropped a redundant assertion. - selfroles.ts: quoted the reserved-word property. - djsx Container/Label/Modal: three redundant assertions the type checker confirmed were doing nothing. - eslint.config.js: the flat config is not in the TS program (allowJs is false), so the imported config arrays resolve to `error` and the spread trips no-unsafe-argument. Disabled on that line with a reason. CI .github/workflows/ci.yml runs bun install --frozen-lockfile, typecheck and lint on every push and pull request. Verified from a clean node_modules: `bun install --frozen-lockfile` succeeds, `bun run typecheck` exits 0, `bun run lint` exits 0, and the loader still registers 10 commands, 3 components and 11 event listeners. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015hDX4g1EjTN7424A5wnptE

First of three commits retiring djsx. src/util/notices.ts provides success/info/warn/error/danger as plain Components V2 container data, replacing the <Success>/<Info>/<Warn>/<Error> JSX widgets in djsx/widgets/Messages.tsx. Built on the existing framework/ui helpers, with accent colours derived from util/colors.ts so that stays the single source of truth. The returned `Notice` type was checked against reply(), editReply(), followUp() and update() — it satisfies all four, which the djsx widgets did not: they were typed as `InteractionReplyOptions & InteractionEditReplyOptions` and needed an `as MessageOptions` cast at every call site. Verified the payloads are identical to the widgets they replace: all five kinds plus the ephemeral variants produce byte-identical JSON to the djsx output, compared while both implementations still exist. Nothing uses this yet; the next commit moves the tags command onto it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015hDX4g1EjTN7424A5wnptE

@djsx

Second of three commits retiring djsx. Nothing imports @djsx after this. src/commands/tags.tsx -> tags.ts, src/components/tags.tsx -> tags.ts. - Command metadata is now a plain RESTPostAPIChatInputApplicationCommandsJSONBody literal instead of <SlashCommand>/<Subcommand>/<StringOption>. - Message widgets move to util/notices.ts (added last commit). - <ComponentMessage> wrappers become explicit {flags: MessageFlags.IsComponentsV2, components: [...]}. - <TagComponent {...tag}/> and <UpdateTagModal {...tag}/> become ordinary function calls, tagContainer(tag) and updateTagModal(tag). Casts in these two files: 18 -> 1. Every JSX expression was typed as BaseComponentData (the single global JSX.Element type), so each one needed an `as` to recover its real type. Plain objects are inferred, so the casts go away and the fields are checked. The one remaining cast is in the modal `field` helper. discord.js still marks `label` required on TextInputComponentData even though the label now lives on the wrapping Label component. djsx omitted it via `Omit<TextInputComponentData, "label">` plus a cast in ModalLabel, and that is what currently ships, so the payload is kept identical rather than adding an untested field. This was the one place the JSX layer was earning something, and it now costs six lines instead of a runtime. Verified against the pre-conversion files from 08aa6f0, comparing rendered payloads with key order normalised (the API does not care about key order): the deployed /tag command metadata, the tag container in all four title/thumbnail combinations, and the modal in both create and update states are byte-identical, 7/7. Two of those started out different and both were worth chasing: the command metadata differed only in key order, but the modal genuinely differed — the first draft included `label` on the inner text input, which is the change described above. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015hDX4g1EjTN7424A5wnptE

@djsx

Third of three commits retiring djsx. Nothing has imported it since the previous commit. Removed: - djsx/ — 29 files, 736 lines. Eleven of its twenty-four components were `(props) => ({type: X, ...props})`; MediaItem and StringOption were identity functions. - tsconfig.bun.json — existed only to override `jsx` because Bun would not read "react-jsx" from tsconfig, with a comment noting bunfig.toml "doesn't seem to work". - bunfig.toml — contained nothing but the four jsx keys. - The `"react": "./djsx/index.ts"` override in package.json, which made every `bun install` print "Bun currently does not support nested overrides". - `--tsconfig-override tsconfig.bun.json` from all four npm scripts. - `jsx`, `jsxImportSource`, the @djsx path aliases, and the djsx include globs from tsconfig.json. - The `**/*.tsx` block in eslint.config.js, which disabled no-unsafe-assignment and no-unsafe-argument. Those rules now apply everywhere, and the codebase passes with them on. Why, in one line: TypeScript has a single global JSX.Element type, declared here as BaseComponentData, so every JSX expression needed an `as` to recover its real type. The readability win cost the type checker at exactly the boundary where mistakes are most expensive. Note for local checkouts: .gitignore lists src/commands/debug.tsx, and the deleted djsx/commands/Command.tsx had a matching `debug()` helper, so a local debug command probably imports @djsx and will need converting. The loader still scans for .tsx as well as .ts, so such a file fails loudly at startup rather than silently disappearing. Verified from a clean node_modules: bun install --frozen-lockfile succeeds with no warnings, bun run typecheck and bun run lint both exit 0, no .tsx files remain, and the loader still registers 10 commands, 3 components and 11 event listeners running under the plain `bun run --bun` invocation the scripts now use. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015hDX4g1EjTN7424A5wnptE

First of three commits collapsing the message layers. The embed-based `Messages` class and the Components V2 `notices` module were two ways to send the same status message; this moves everything that can move cleanly. Migrated: joinleave, spam, addons, botadmin, developer, moderation, voicetext — 42 call sites. The transformation is one token per site because the option shapes already matched: a survey of every call showed only `{ephemeral: true}` and `{components: [...]}` were ever passed. notices gained a `components` option, which renders action rows inside the container rather than beside the embed, since that is where V2 puts them. Imported as a namespace (`import * as notices`) rather than named imports. `error` as a bare identifier would shadow, or be shadowed by, the `error` binding in nearby catch blocks; `notices.error(...)` also keeps the diff to a single token per line against the old `Messages.error(...)`. selfroles and cleanname are deliberately left for the next commit. Each one starts a message in one mode and updates it in the other — cleanname replies with Messages.info and later updates the same message with a raw embed; selfroles replies with an embed panel and updates it with Messages.info. Discord rejects switching a message between embed and Components V2 mode after creation, so those two have to convert wholesale or not at all. Incidentally consistent now: addons rendered its addon pages as V2 containers but its "no addons found" message as an embed, on the same interaction. Verified: tsc and eslint both clean, and the Notice type satisfies send() as well as reply/editReply/followUp/update, which joinleave needs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015hDX4g1EjTN7424A5wnptE

Second of three commits collapsing the message layers. src/util/messages.ts is gone; nothing sends an embed-based status message any more. These two could not be migrated piecemeal. Discord rejects switching a message between embed and Components V2 mode after creation, and both flows crossed that line: cleanname replies with a status message and later updates the same message with a raw embed, selfroles replies with an embed panel and updates it with a status message. selfroles: the panel is now a container with the listing and its buttons inside it, so the whole flow — reply, both picker updates, and the return to the panel — is one mode throughout. cleanname: the role-select control is plain component data, and the progress display is a container. Components V2 has no inline field grid, so the three counters (Members / Fixed / Failed) render on one line separated by em spaces instead of as three inline embed fields, and the embed timestamp becomes Discord's <t:...:f> markup, which still localises per viewer. That is the one deliberate visual change in this commit. Also fixes a bug found while converting it: the completion path called interaction.update() a second time on an interaction that had already been acknowledged by the initial progress update. That throws InteractionAlreadyReplied, so the final Fixed/Failed counts never reached the user — `/cleanname server` appeared to hang on "0 fixed, 0 failed" however many names it had actually corrected. It now uses editReply(). Supporting changes: - util/colors.ts gained `Accents`, the same palette as integers, derived from the hex values rather than duplicated. Container accents want an integer; notices had been computing that itself. - framework/ui.ts gained the `ComponentMessage` type. `flags` is deliberately `number` and not the MessageFlags enum: an unannotated `MessageFlags.IsComponentsV2` widens to the whole enum, which is not assignable to the narrower per-method flag unions discord.js declares, while `number` is assignable to a numeric enum and satisfies all of them. That widening is exactly what broke the first draft of cleanname's progress(). Verified: tsc and eslint clean; every notice variant carries IsComponentsV2, ephemeral notices carry both flags, and a notice with action rows nests the row inside the container rather than beside it. Remaining embeds are the moderation log entries, DM forwarding and /about, all handled in the next commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015hDX4g1EjTN7424A5wnptE

Third of three commits collapsing the message layers. src/util/modlog.ts replaces five hand-built embeds across three event files. detectspam, invitefilter and detectcryptoscam each constructed their own near-identical entry — same colour, same author/description/Reason/footer shape — and each repeated the same modlog-channel resolution: const modlogId = current.modlog; const modlogChannel = message.guild.channels.cache.get(modlogId!); if (!modlogId || !modlogChannel || !modlogChannel.isTextBased()) return; That lookup was also subtly wrong in two of them: it sat before the mute / timeout log and returned early, so a guild with no modlog configured would skip the rest of the handler rather than just skip logging. sendModLog no-ops instead, and the caller carries on. Rendering notes: the embed author icon becomes a Section thumbnail, which is the closest V2 equivalent; the footer timestamp becomes Discord's <t:...:f> markup so it still localises per viewer. Entries without an avatar render as flat text with no Section. forwarding.ts: the DM forwarding embed becomes a container, with attachments as markdown links rather than embed fields. about.ts keeps its embed, deliberately, with a comment saying why: its stats are inline fields three to a row, and V2 has no field grid — faking one with padded text does not survive different client widths. It is now the only EmbedBuilder in the codebase, and the comment tells the next person that. This closes step 4. The three ways of sending a message are down to one, plus one documented exception: before Messages.* embeds (54 sites) + raw EmbedBuilder (11) + djsx widgets (12) after notices.* / modlog / plain container data, and /about Verified: tsc and eslint clean; modlog entries render the heading, body, reason, thumbnail and timestamp correctly in both the with-avatar and without-avatar shapes; no event file references EmbedBuilder any more; the loader still registers 10 commands, 3 components and 11 event listeners. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015hDX4g1EjTN7424A5wnptE

…mand First of several commits taking the remaining commands off the legacy module shape, so the dispatcher's legacy path can be deleted. Each command becomes `export const command = defineCommand({...})` with plain RESTPostAPIChatInputApplicationCommandsJSONBody data instead of a SlashCommandBuilder chain, and declares `guildOnly` where it is guild-only so the dispatcher earns the <"cached"> narrowing rather than the file asserting it. Subcommand handlers become module-level functions. They had been methods called through `this`, which defineCommand cannot accept: the Command interface declares only execute and autocomplete, so extra properties fail the excess-property check. Standalone functions are what selfroles already uses. moderation also loses two duplicated pairs, both of which carried a TODO asking for exactly this: - invitefilter and detectspam were byte-identical apart from the settings key; they share toggleModule() now. - modlog and joinleave likewise; they share setChannel(). Verified by dumping every command's deployed payload before and after and diffing with keys normalised. spam, moderation and developer are byte identical. The other two differ in exactly the two ways intended: - about: `options: []` is now absent. The builder emitted an empty array; Discord treats absent and empty the same. - voicetext: `dm_permission: false` becomes `contexts: [Guild]`. setDMPermission is deprecated, and every other command in the codebase already declares contexts, so this removes the last inconsistency of that kind outside cleanname. Every command not touched by this commit is byte-identical, confirming the comparison itself is sound. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015hDX4g1EjTN7424A5wnptE

Second batch. Both have autocomplete as well as execute, so they exercise the optional-handler half of the Command type that the old CommandModule made mandatory. Subcommand handlers become module-level functions, as in the previous commit. `delete` is renamed `remove` since it cannot be a bare function name in that position. addons also loses an annotation that was quietly wrong. Every handler was typed ChatInputCommandInteraction<"cached">, but the command declares contexts of Guild, BotDM and PrivateChannel, so it runs in DMs where there is no cached guild. Nothing in the command or in util/addons.ts ever touches interaction.guild — the only `.guild` references are `addon.author.guild` from the BetterDiscord API — so the annotation was pure assertion. It is now plain ChatInputCommandInteraction, and paginateAddonPages loosened to match. This is the class of bug the old signature invited: `execute` was declared `<T extends BaseInteraction = ChatInputCommandInteraction>(interaction: T)`, so the caller chose T and no narrowing was ever checked. Verified against the payload baseline again, this time normalising the `options: []` the builder emitted for option-less subcommands (Discord treats absent and empty the same). With that normalised, nine of ten commands are byte-identical, including addons and tag. The tenth is voicetext, whose dm_permission -> contexts change was the intended one from the previous commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015hDX4g1EjTN7424A5wnptE

Final batch. The loader now reports 10 migrated commands and 0 legacy. botadmin - `ownerOnly: true` replaces the hand-rolled first line of execute(), `if (interaction.user.id !== process.env.BOT_OWNER_ID) ...`. The dispatcher enforces it, and the same flag already routed the command to the private guild at deploy time. - The `async modal() {}` stub is gone. It existed only because the old CommandModule type made all five component handlers mandatory, and carried the comment "This is just here to satisfy the event requirement I imposed on myself". Nothing needs it: the modal is awaited inline, so the dispatcher correctly ignores the submission and lets awaitModal resolve it. - The modal itself is plain component data, and the show/await/read dance uses the framework's awaitModal, which returns null on timeout instead of making the caller distinguish it from a real failure inside a catch. - `getChannel("channel", true) as TextChannel` — which carried an eslint-disable for an assertion the linter called unnecessary and the type checker required — becomes `getChannel<ChannelType.GuildText>(...)`, the typed overload voicetext already used. No cast, no disable. cleanname The bypass-role picker is a registered component (`cleanname.bypass`) rather than a `role()` method routed by `customId.split("-")[0]`, so its handler receives a typed RoleSelectMenuInteraction<"cached"> earned by the dispatcher's guild check. Payload check across all ten commands, with the builder's empty `options: []` normalised away: eight are byte-identical to the pre-migration baseline. The two that differ are cleanname and voicetext, both `dm_permission: false` -> `contexts: [Guild]` — the deprecated setDMPermission is now gone from the codebase entirely, and every command declares contexts the same way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015hDX4g1EjTN7424A5wnptE

Nothing uses it since the previous commit, so the transitional half of the framework goes away. Deleted from dispatch.ts: LegacyKind, LegacyEntry, LegacyHandler, the `legacy` map, addLegacyCommand(), runLegacyComponent(), and the legacy branches inside runCommand() and runAutocomplete(). With them goes the last of the stringly-typed routing — `this.legacy.get(customId.split("-")[0])`, followed by an if-chain mapping interaction kinds to method names. Deleted from loader.ts: the default-export branch, the builder-vs-plain-data normalisation in commandData(), and LEGACY_KINDS. A command file must now export `command`; anything else throws at startup naming the file, rather than being silently half-loaded. `Dispatcher.counts` loses its `legacy` key, and the startup log and deploy script lose their migrated/legacy annotations. framework/README.md's migration guide becomes a short "writing a command" section, since there is nothing left to migrate from. Verified: the dispatcher still routes commands, enforces guildOnly, decodes component params, ignores session ids and unknown namespaces, and reports stale ids — nine checks against stub interactions. Two of those specifically confirm the legacy behaviour is gone: an unknown namespace no longer falls through to a split("-") lookup, and a `cleanname-whatever` custom id no longer routes anywhere. Command payloads are byte-identical to the previous commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015hDX4g1EjTN7424A5wnptE

The BetterDiscord guild id appeared four times across three files, the developer role ids five times across two, and the account-issues channel and AutoMod rule ids were inline constants in the files that used them. Changing any of them meant grepping for a number. src/config.ts collects all seven, each overridable by an environment variable so the bot can be pointed at a test server without editing source: BD_GUILD_ID, BD_ROLE_PLUGIN_DEV, BD_ROLE_THEME_DEV, COMMUNITY_ROLE_PLUGIN_DEV, COMMUNITY_ROLE_THEME_DEV, BD_CHANNEL_ACCOUNT_ISSUES, BD_AUTOMOD_SPAM_LINK_RULE Defaults are the values that were already inline, so behaviour is unchanged with no environment set. Verified: all seven defaults match the literals they replaced, and setting BD_GUILD_ID overrides that one while leaving the rest alone. src/util/web.ts keeps its release-channel ids. They are BetterDiscord website data copied from the client repository rather than deployment configuration, and the file documents its upstream source. No inline snowflakes remain in src/ outside config.ts and web.ts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015hDX4g1EjTN7424A5wnptE

The container's CMD is `validate && deploy && start`, so every restart bulk overwrote every global command — needless API traffic, and a rate-limit risk during a crash loop. Global command propagation is also not instant, so redeploying identical payloads on every boot bought nothing. deploy-commands.ts now fingerprints what it is about to send (the global payload, the owner-guild payload, the client id and the guild id) and stores the hash in the existing Keyv/SQLite store. If the fingerprint matches the last successful deploy it skips entirely. `--force` / `-f` overrides, and `--clear` drops the stored fingerprint so the next deploy runs. setCommands() now reports whether the calls succeeded, and the fingerprint is recorded only if they did. It previously caught and logged errors while returning normally, so without that the first failed deploy would have been remembered as successful and never retried. Verified end to end against a bogus token: 1. failed deploy -> nothing recorded, "the next run will retry" 2. run again -> retries, does not skip 3. fingerprint seeded as a success -> skips, zero API calls 4. --force -> deploys anyway 5. BOT_GUILD_ID changed -> fingerprint differs, deploys 6. a command description edited -> fingerprint differs, deploys 7. edit reverted -> skips again Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015hDX4g1EjTN7424A5wnptE

CI was failing on `bun install --frozen-lockfile`: error: lockfile had changes, but lockfile is frozen note: overrides in package.json changed since bun.lock was saved My fault, from the djsx removal. That commit dropped `"overrides": {"react": "./djsx/index.ts"}` from package.json but bun.lock still recorded it. Bun 1.3.11, which I had locally, accepted the mismatch under --frozen-lockfile, so the clean-install check I ran passed. Bun 1.4.0, which CI resolved from `bun-version: latest`, rejects it. Regenerated bun.lock so its overrides block matches package.json. Reproduced the exact failure on 1.4.0 first, then confirmed a clean `bun install --frozen-lockfile` succeeds. Also pinned the workflow to bun-version "1.4.0" instead of `latest`. The lockfile format is version sensitive, and a Bun release landing upstream should not be able to break CI on an unrelated commit. Bumping it is now a deliberate change with its own diff. Verified on 1.4.0: frozen install, typecheck and lint all pass from an empty node_modules. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015hDX4g1EjTN7424A5wnptE

80 tests across 8 files, using `bun test`. This is the step 6 item I kept flagging: everything verified during the refactor was throwaway scripts, so none of it protected the next change. tests/ids.test.ts codec round-trips (separators, percent signs, non-ascii, empty values), the 100-character cap, snowflake and oneOf validation, and the two decode failures that mean "stale id" tests/dispatch.test.ts command, autocomplete and component routing; guildOnly and ownerOnly gating; duplicate registration; a throwing handler being reported tests/session.test.ts the session state machine and the ownership guard tests/paginator.test.ts navigation, clamping at both ends, empty lists, and the disable-on-end pass tests/messages.test.ts notice flags and structure, modlog entries in both avatar shapes, tag container and modal rendering tests/loader.test.ts the real command and event directories load; a file may export several listeners; a malformed module throws naming the file tests/commands.test.ts a snapshot of every deployed command payload tests/regressions.test.ts one test per bug fixed during the refactor The payload snapshot is the one I most wanted. Each refactor step was checked by dumping every command's deployed JSON before and after and diffing it by hand; tests/fixtures/command-payloads.json makes CI do that. Confirmed it works by changing one character of a command description and watching it fail. When a change is intended, `bun run tests/fixtures/regenerate-payloads.ts` rewrites the fixture and the diff becomes the review. The regression file names the failure each test guards: - the /g regex that made /cleanname server skip members, asserted stable across repeated calls - the config ids, asserted well-formed and distinct - notices being plain objects that need no cast, and interpolation happening in the template rather than in markup, which is what produced "$updated" Two helpers keep the tests free of gateway or network setup: tests/helpers/interactions.ts stubs the type guards and reply methods the dispatcher actually calls, and tests/helpers/session.ts stands in for the message collector. Both confine their casts to one file. The session harness waits for runSession to attach its collector before emitting, since a press issued immediately would otherwise land before the listener exists — that cost three failing tests before I spotted it. silenceConsole() marks the tests that deliberately provoke a log so a real failure still stands out in the output. CI now runs test, typecheck and lint. Verified all three from an empty node_modules on the pinned Bun. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015hDX4g1EjTN7424A5wnptE

util/addons.ts was the last file building components with the discord.js builder pattern — the thing this whole refactor started from. Its four render functions now return plain data like everything else: createAddonComponent -> ContainerComponentData createAddonSection -> SectionComponentData createAddonList -> [TextDisplayComponentData, ContainerComponentData] createNavigation -> ActionRowData<MessageActionRowComponentData> The `new ContainerBuilder().addSectionComponents(new SectionBuilder()...)` chain that the original review used as the argument against builders is gone. Two small local helpers (separator, thumbnail) plus the existing row / container / text cover the rest. Verified by rendering both versions of all four functions and diffing the API JSON, normalising the camelCase/snake_case difference between the data interfaces and the builders' output: a full addon page, the same page with a support-server button, a section, a two-addon list, and the navigation menu in both selected and disabled states are all identical, 6/6. The only EmbedBuilder left in src/ is /about, which keeps it deliberately and says why. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015hDX4g1EjTN7424A5wnptE

paginateAddonPages was the last hand-rolled component collector: it repeated the ownership check, the timeout and the disable-on-end pass that framework/session.ts already does. It is now a runSession whose state is the selected index, and its select menu carries a session-owned custom id so the dispatcher leaves it to the collector. Three bugs fell out of the conversion. 1. `/addons search` with no matches threw. string-similarity's findBestMatch rejects an empty candidate list ("Bad arguments"), so a search that matched nothing surfaced as the dispatcher's generic "Something went wrong" rather than "no results". It now answers before ranking. 2. An empty addon list built an illegal select menu. Discord requires between 1 and 25 options; createNavigation([]) produced zero, so `/addons top` and friends would have been rejected at send time whenever the cache was empty — the same shape as the selfroles setMaxValues(0) crash. paginateAddonPages now answers with a notice instead, and callers pass a fitting message. 3. `/addons random` on an empty cache indexed past the end and rendered `undefined`. Guarded. The selection handler also loses `addons.find(...)!`; an unrecognised value now leaves the selection alone rather than asserting the lookup cannot fail. Two follow-ons: - paginateAddonPages takes a RepliableInteraction rather than a ChatInputCommandInteraction. It only defers and edits, so the tighter type was claiming more than the code uses. - tags.ts was still calling awaitModalSubmit directly inside a try/catch that treated any throw as a timeout — including a database failure, which the user would have been told was a submission timeout. It uses the framework's awaitModal now, which returns null only on timeout. src/ no longer contains a hand-rolled collector or modal wait. 16 new tests cover the browser (rendering, selection moving the tick and the page, an unknown value being ignored, ownership, disable-on-end, V2 flags on every render), list separators, sorting, and the conditional support-server button. Writing them exposed two gaps in the shared session harness — no select-menu type guards and no collector.stop() — both now fixed, so the harness can drive select menus and exercise runSession's error path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015hDX4g1EjTN7424A5wnptE

Orientation for future sessions: commands, layout, the conventions the refactor settled on, and a footgun list. The footgun section is the part worth having. Every entry caused a real, shipped bug in this repo — stateful /g regexes, a message that cannot change between embed and Components V2 mode, `update()` only working once, select menus needing at least one option, MessageFlags widening in an unannotated literal, caches that clear before they fetch. They are recorded with the symptom rather than just the rule, so the next person recognises the failure rather than having to rediscover the cause. Also records what is deliberately not done, so it does not get "fixed": /about keeps its embed because Components V2 has no inline field grid, util/web.ts keeps its ids because they are upstream website data, and the invite whitelist stays hardcoded because making it configurable is a feature decision rather than cleanup. Every factual claim was checked against the tree: script names, exported helper names, the test count, the pinned Bun version, and the three "deliberately left alone" items. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015hDX4g1EjTN7424A5wnptE

claude added 20 commits August 26, 2026 05:51

Navigation Menu

Uh oh!

Retire djsx, and get the codebase to one coherent shape - #8

Retire djsx, and get the codebase to one coherent shape#8
zerebos wants to merge 20 commits into
mainfrom
claude/discord-bot-jsx-review-65620q

zerebos commented Aug 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

zerebos commented Aug 28, 2026

What changed

Bugs fixed (15)

Structure

Worth a closer look

Verification

Suggested reading order

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants