From fc2773e83fca0e3dce33bb64722017d9e346a29d Mon Sep 17 00:00:00 2001 From: RedsTom Date: Wed, 16 Sep 2026 22:26:42 +0200 Subject: [PATCH 01/16] feat(core): restore guild state on rejoin and welcome new guilds Add a global core guildCreate listener: explicit config init on first join, reinstall of enabled modules guild commands on rejoin (Discord purges them on kick), and a Components V2 welcome message in the system channel with owner DM fallback. Nothing is deleted on leave so a rejoin restores history. updateModuleActivation is now an upsert. Co-Authored-By: Claude Code --- docs/functional.md | 6 + docs/site/en/legal/privacy.md | 2 +- docs/site/fr/legal/privacy.md | 2 +- src/core/core.module.ts | 5 +- src/core/i18n/en.json | 7 +- src/core/i18n/fr.json | 7 +- .../listeners/guild-create.listener.test.ts | 208 ++++++++++++++++++ src/core/listeners/guild-create.listener.ts | 151 +++++++++++++ src/core/services/module.service.test.ts | 32 ++- src/core/services/module.service.ts | 10 +- src/core/utils/core-messages.ts | 14 ++ 11 files changed, 427 insertions(+), 17 deletions(-) create mode 100644 src/core/listeners/guild-create.listener.test.ts create mode 100644 src/core/listeners/guild-create.listener.ts diff --git a/docs/functional.md b/docs/functional.md index d2beb697..9db61e17 100644 --- a/docs/functional.md +++ b/docs/functional.md @@ -4,6 +4,12 @@ Documentation du comportement visible par les utilisateurs et administrateurs de --- +## Arrivée et départ du bot + +Quand le bot est invité sur un serveur, ses données sont initialisées et un message de bienvenue est posté dans le salon système (ou envoyé en MP au propriétaire si le salon système est indisponible). Le message pointe vers `/modules` et `/config`. Rien d'autre n'est activé automatiquement : seuls les administrateurs décident quels modules activer. + +Quand le bot quitte un serveur, rien n'est effacé : si le bot est réinvité plus tard, la configuration et les modules activés sont restaurés automatiquement (y compris leurs commandes). + ## Module Core Toujours actif, non désinstallable. Fournit la gestion des modules pour les administrateurs. diff --git a/docs/site/en/legal/privacy.md b/docs/site/en/legal/privacy.md index 0074420f..36a88be6 100644 --- a/docs/site/en/legal/privacy.md +++ b/docs/site/en/legal/privacy.md @@ -36,7 +36,7 @@ Data is stored in a PostgreSQL database managed by the bot instance operator. Th ## Data Retention -Configuration and activation data is retained for as long as the bot is active on a guild. When a module is uninstalled, its configuration data may be retained in the guild's configuration blob (configurable by instance operators). To request data deletion, contact the instance operator. +Configuration and activation data is retained for as long as the bot is active on a guild. When the bot leaves a guild (kick or removal), the data is deliberately kept so everything is restored automatically if the bot is re-invited. When a module is uninstalled, its configuration data may be retained in the guild's configuration blob (configurable by instance operators). To request data deletion, contact the instance operator. ## Data Sharing diff --git a/docs/site/fr/legal/privacy.md b/docs/site/fr/legal/privacy.md index a08d3720..ca6baf1a 100644 --- a/docs/site/fr/legal/privacy.md +++ b/docs/site/fr/legal/privacy.md @@ -36,7 +36,7 @@ Les données sont stockées dans une base de données PostgreSQL gérée par l'o ## Conservation des données -Les données de configuration et d'activation sont conservées tant que le bot est actif sur un serveur. Quand un module est désinstallé, ses données de configuration peuvent être conservées dans le blob de configuration du serveur (configurable par les opérateurs d'instance). Pour demander la suppression des données, contactez l'opérateur de l'instance. +Les données de configuration et d'activation sont conservées tant que le bot est actif sur un serveur. Quand le bot quitte un serveur (kick ou retrait), les données sont volontairement conservées afin que tout soit restauré automatiquement si le bot est réinvité. Quand un module est désinstallé, ses données de configuration peuvent être conservées dans le blob de configuration du serveur (configurable par les opérateurs d'instance). Pour demander la suppression des données, contactez l'opérateur de l'instance. ## Partage des données diff --git a/src/core/core.module.ts b/src/core/core.module.ts index efd96d5c..f61213e0 100644 --- a/src/core/core.module.ts +++ b/src/core/core.module.ts @@ -1,3 +1,4 @@ +import { GatewayIntentBits } from "discord.js"; import { defineModule } from "#lib/module.js"; import configCommand from "./commands/config.command.js"; import moduleCommand from "./commands/module.command.js"; @@ -14,6 +15,7 @@ import { resetConfigSelect, } from "./interactions/reset-config.js"; import toggleOptionButton from "./interactions/toggle-option.button.js"; +import guildCreateListener from "./listeners/guild-create.listener.js"; import commandListener from "./listeners/interaction-create.listener.js"; export default defineModule({ @@ -22,7 +24,7 @@ export default defineModule({ description: "The core module of the application, managing core commands and events. It is always loaded.", version: "1.1.1", - intents: [], + intents: [GatewayIntentBits.Guilds], config: coreConfigSchema satisfies CoreConfig, onLoad(_, registry) { // Register the core module's commands and events in the provided registry @@ -30,6 +32,7 @@ export default defineModule({ registry.register(configCommand); registry.register(commandListener); + registry.register(guildCreateListener); registry.register(enableModuleButton); registry.register(disableModuleButton); diff --git a/src/core/i18n/en.json b/src/core/i18n/en.json index b3c433d3..377f4e60 100644 --- a/src/core/i18n/en.json +++ b/src/core/i18n/en.json @@ -87,5 +87,10 @@ "modules.test-config.description": "Development module declaring all configuration types, used to test the configuration UI.", "config.locale.name": "Language", - "config.locale.description": "Bot language" + "config.locale.description": "Bot language", + + "guild.welcome.title": "# Thanks for inviting me!", + "guild.welcome.body": "Admins, start with `/modules` to enable modules and `/config` to configure them.", + "guild.welcome.hint": "Your previous configuration will be restored automatically if I have been here before.", + "guild.welcome.dmPrefix": "(System channel unavailable) " } diff --git a/src/core/i18n/fr.json b/src/core/i18n/fr.json index 8de1a738..d3cee25d 100644 --- a/src/core/i18n/fr.json +++ b/src/core/i18n/fr.json @@ -87,5 +87,10 @@ "modules.test-config.description": "Module de développement déclarant tous les types de configuration, pour tester l'UI de configuration.", "config.locale.name": "Langue", - "config.locale.description": "Langue du bot" + "config.locale.description": "Langue du bot", + + "guild.welcome.title": "# Merci de m'avoir invité !", + "guild.welcome.body": "Admins, commencez par `/modules` pour activer des modules et `/config` pour les configurer.", + "guild.welcome.hint": "Votre configuration précédente sera restaurée automatiquement si je suis déjà venu ici.", + "guild.welcome.dmPrefix": "(Salon système indisponible) " } diff --git a/src/core/listeners/guild-create.listener.test.ts b/src/core/listeners/guild-create.listener.test.ts new file mode 100644 index 00000000..9bc441e3 --- /dev/null +++ b/src/core/listeners/guild-create.listener.test.ts @@ -0,0 +1,208 @@ +import { MessageFlags } from "discord.js"; +import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { addTranslations, initI18n } from "#lib/i18n.js"; + +const { mockModules } = vi.hoisted(() => ({ mockModules: [] as any[] })); +vi.mock("#index.js", () => ({ modules: mockModules, client: {} })); + +const { + clearCacheForGuild, + getFullConfigForGuild, + getAllModulesStateIn, + installModuleCommandsIn, +} = vi.hoisted(() => ({ + clearCacheForGuild: vi.fn(), + getFullConfigForGuild: vi.fn(), + getAllModulesStateIn: vi.fn(), + installModuleCommandsIn: vi.fn(), +})); +vi.mock("#core/services/config.service.js", () => ({ + default: { clearCacheForGuild, getFullConfigForGuild }, +})); +vi.mock("#core/services/module.service.js", () => ({ + default: { getAllModulesStateIn }, +})); +vi.mock("#core/loaders/command-loader.js", () => ({ + installModuleCommandsIn, +})); + +const { default: listener, resolveWelcomeChannel } = + await import("./guild-create.listener.js"); + +function fakeChannel(sendImpl?: (args: unknown) => Promise) { + return { + id: "chan-1", + isTextBased: () => true, + isSendable: () => true, + send: vi.fn(sendImpl ?? (async () => ({}))), + }; +} + +function fakeGuild(overrides: Record = {}) { + const channel = overrides.channel ?? fakeChannel(); + const ownerSend = overrides.ownerSend ?? vi.fn(async () => ({})); + return { + guild: { + id: "guild-1", + preferredLocale: "fr", + systemChannelId: "chan-1", + systemChannel: channel, + channels: { fetch: vi.fn(async () => channel) }, + members: { + me: { + permissionsIn: vi.fn(() => ({ has: () => true })), + }, + }, + fetchOwner: vi.fn(async () => ({ send: ownerSend })), + client: { tag: "fake-client" }, + ...overrides.guild, + } as any, + channel, + ownerSend, + }; +} + +beforeAll(async () => { + await initI18n(); + for (const lng of ["en", "fr"]) { + addTranslations(lng, "core", { + "guild.welcome.title": "title", + "guild.welcome.body": "body", + "guild.welcome.hint": "hint", + "guild.welcome.dmPrefix": "prefix ", + }); + } +}); + +beforeEach(() => { + vi.clearAllMocks(); + mockModules.length = 0; + clearCacheForGuild.mockResolvedValue(undefined); + getFullConfigForGuild.mockResolvedValue({}); + getAllModulesStateIn.mockResolvedValue([]); + installModuleCommandsIn.mockResolvedValue(undefined); +}); + +describe("guildCreate", () => { + it("initializes config data on first join even when nothing is enabled", async () => { + const { guild, channel } = fakeGuild(); + + await listener.execute(guild, undefined); + + expect(clearCacheForGuild).toHaveBeenCalledWith("guild-1"); + expect(getFullConfigForGuild).toHaveBeenCalledWith("guild-1"); + expect( + (clearCacheForGuild as any).mock.invocationCallOrder[0] + ).toBeLessThan((getFullConfigForGuild as any).mock.invocationCallOrder[0]); + expect(installModuleCommandsIn).not.toHaveBeenCalled(); + expect(channel.send).toHaveBeenCalledOnce(); + expect(channel.send).toHaveBeenCalledWith( + expect.objectContaining({ flags: MessageFlags.IsComponentsV2 }) + ); + }); + + it("reinstalls commands only for enabled modules that declare commands", async () => { + const withCommands = { id: "m1", registry: { commands: [{}, {}] } }; + const withoutCommands = { id: "m2", registry: { commands: [] } }; + mockModules.push(withCommands, withoutCommands); + getAllModulesStateIn.mockResolvedValue([ + { module: { id: "m1" }, enabled: true }, + { module: { id: "m2" }, enabled: true }, + { module: { id: "unknown" }, enabled: true }, + { module: { id: "m1" }, enabled: false }, + ]); + const { guild } = fakeGuild(); + + await listener.execute(guild, undefined); + + expect(installModuleCommandsIn).toHaveBeenCalledTimes(1); + expect(installModuleCommandsIn).toHaveBeenCalledWith( + guild.client, + withCommands, + guild + ); + }); + + it("keeps reinstalling other modules when one install fails", async () => { + const modA = { id: "a", registry: { commands: [{}] } }; + const modB = { id: "b", registry: { commands: [{}] } }; + mockModules.push(modA, modB); + getAllModulesStateIn.mockResolvedValue([ + { module: { id: "a" }, enabled: true }, + { module: { id: "b" }, enabled: true }, + ]); + installModuleCommandsIn.mockRejectedValueOnce(new Error("boom")); + const { guild } = fakeGuild(); + + await expect(listener.execute(guild, undefined)).resolves.toBeUndefined(); + expect(installModuleCommandsIn).toHaveBeenCalledTimes(2); + }); + + it("falls back to the owner DM when no system channel exists", async () => { + const ownerSend = vi.fn(async () => ({})); + const { guild } = fakeGuild({ + ownerSend, + guild: { systemChannelId: null, systemChannel: null }, + }); + + await listener.execute(guild, undefined); + + expect(ownerSend).toHaveBeenCalledOnce(); + expect(ownerSend).toHaveBeenCalledWith( + expect.objectContaining({ + content: expect.any(String), + flags: MessageFlags.IsComponentsV2, + }) + ); + }); + + it("falls back to the owner DM when the system channel send is forbidden", async () => { + const channel = fakeChannel(async () => { + throw Object.assign(new Error("forbidden"), { code: 50013 }); + }); + const ownerSend = vi.fn(async () => ({})); + const { guild } = fakeGuild({ channel, ownerSend }); + + await listener.execute(guild, undefined); + + expect(channel.send).toHaveBeenCalledOnce(); + expect(ownerSend).toHaveBeenCalledOnce(); + }); + + it("stays silent when the owner also blocks DMs", async () => { + const ownerSend = vi.fn(async () => { + throw Object.assign(new Error("no dm"), { code: 50007 }); + }); + const { guild } = fakeGuild({ + ownerSend, + guild: { systemChannelId: null, systemChannel: null }, + }); + + await expect(listener.execute(guild, undefined)).resolves.toBeUndefined(); + }); +}); + +describe("resolveWelcomeChannel", () => { + it("returns null without a system channel", async () => { + const { guild } = fakeGuild({ + guild: { systemChannelId: null, systemChannel: null }, + }); + await expect(resolveWelcomeChannel(guild)).resolves.toBeNull(); + }); + + it("returns null without send permission", async () => { + const channel = fakeChannel(); + const { guild } = fakeGuild({ + channel, + guild: { + members: { me: { permissionsIn: () => ({ has: () => false }) } }, + }, + }); + await expect(resolveWelcomeChannel(guild)).resolves.toBeNull(); + }); + + it("returns the channel when readable and writable", async () => { + const { guild, channel } = fakeGuild(); + await expect(resolveWelcomeChannel(guild)).resolves.toBe(channel); + }); +}); diff --git a/src/core/listeners/guild-create.listener.ts b/src/core/listeners/guild-create.listener.ts new file mode 100644 index 00000000..0c86942f --- /dev/null +++ b/src/core/listeners/guild-create.listener.ts @@ -0,0 +1,151 @@ +import { + MessageFlags, + PermissionFlagsBits, + type Guild, + type GuildTextBasedChannel, +} from "discord.js"; +import { installModuleCommandsIn } from "#core/loaders/command-loader.js"; +import configService from "#core/services/config.service.js"; +import moduleService from "#core/services/module.service.js"; +import { guildWelcomeMessage } from "#core/utils/core-messages.js"; +import { modules } from "#index.js"; +import { createT, type TFunction } from "#lib/i18n.js"; +import { declareEventListener } from "#lib/listener.js"; +import { loggerMaker } from "#lib/logger.js"; + +const logger = loggerMaker("guild"); + +/** + * No `guildDelete` purge exists on purpose: when the bot leaves a guild, + * `GuildConfiguration` and `ModuleActivation` rows are kept so a rejoin + * restores everything. See the privacy policy (retention section). + */ +function resolveJoinLocale(guild: Guild): string { + const raw = guild.preferredLocale ?? "en"; + return raw.toLowerCase().startsWith("fr") ? "fr" : "en"; +} + +function asSendableChannel( + guild: Guild, + channel: unknown +): GuildTextBasedChannel | null { + if (!channel || typeof channel !== "object") return null; + const candidate = channel as GuildTextBasedChannel; + if (typeof candidate.isTextBased !== "function" || !candidate.isTextBased()) { + return null; + } + if (typeof candidate.isSendable !== "function" || !candidate.isSendable()) { + return null; + } + const me = guild.members.me; + if (!me) return null; + const canPost = me + .permissionsIn(candidate) + .has([PermissionFlagsBits.ViewChannel, PermissionFlagsBits.SendMessages]); + return canPost ? candidate : null; +} + +export async function resolveWelcomeChannel( + guild: Guild +): Promise { + if (!guild.systemChannelId) return null; + const channel = + guild.systemChannel ?? + (await guild.channels.fetch(guild.systemChannelId).catch(() => null)); + return asSendableChannel(guild, channel); +} + +function logSendError(error: unknown, where: string, guildId: string): void { + if (error instanceof Error && "code" in error) { + const code = (error as { code: number }).code; + if (code === 50001 || code === 50004 || code === 50007 || code === 50013) { + logger.warn( + `Welcome message not sent | where = ${where} | guild = ${guildId} | code = ${code}` + ); + return; + } + } + logger.error( + { err: error }, + `Welcome message failed | where = ${where} | guild = ${guildId}` + ); +} + +/** Reinstalls guild commands for every enabled module (Discord purges them on kick). */ +async function reinstallModuleCommands(guild: Guild): Promise { + let states: Awaited>; + try { + states = await moduleService.getAllModulesStateIn(guild.id); + } catch (err) { + logger.error( + { err }, + `Failed to list module states on join | guild = ${guild.id}` + ); + return; + } + + for (const state of states) { + if (!state.enabled) continue; + const mod = modules.find((m) => m.id === state.module.id); + if (!mod || mod.registry.commands.length === 0) continue; + try { + // Sequential on purpose: the installer already POSTs in parallel per + // command, no need to fan out across modules too. + await installModuleCommandsIn(guild.client, mod, guild); + } catch (err) { + logger.error( + { err }, + `Failed to reinstall commands | module = ${mod.id} | guild = ${guild.id}` + ); + } + } +} + +async function sendWelcome(guild: Guild, t: TFunction): Promise { + const components = guildWelcomeMessage(t); + const channel = await resolveWelcomeChannel(guild).catch(() => null); + if (channel) { + try { + await channel.send({ + components, + flags: MessageFlags.IsComponentsV2, + }); + return; + } catch (error) { + logSendError(error, "systemChannel", guild.id); + } + } + + try { + const owner = await guild.fetchOwner(); + await owner.send({ + content: t("guild.welcome.dmPrefix"), + components, + flags: MessageFlags.IsComponentsV2, + }); + } catch (error) { + logSendError(error, "ownerDm", guild.id); + } +} + +export default declareEventListener({ + eventType: "guildCreate", + execute: async (guild) => { + const t = createT(resolveJoinLocale(guild), "core"); + + // Explicit init on first join (no more late lazy creation): creates the + // row when absent, reloads + recaches it on rejoin. + try { + await configService.clearCacheForGuild(guild.id); + await configService.getFullConfigForGuild(guild.id); + } catch (err) { + logger.error( + { err }, + `Failed to init config on join | guild = ${guild.id}` + ); + } + + await reinstallModuleCommands(guild); + await sendWelcome(guild, t); + }, +}); diff --git a/src/core/services/module.service.test.ts b/src/core/services/module.service.test.ts index 90d82149..96125beb 100644 --- a/src/core/services/module.service.test.ts +++ b/src/core/services/module.service.test.ts @@ -5,12 +5,12 @@ import type { Module } from "#lib/module.js"; // client; stub both so importing it never boots the bot or hits a database. vi.mock("#index.js", () => ({ modules: [], client: {} })); -const { findMany, update } = vi.hoisted(() => ({ +const { findMany, upsert } = vi.hoisted(() => ({ findMany: vi.fn(), - update: vi.fn(), + upsert: vi.fn(), })); vi.mock("#lib/database.js", () => ({ - default: { moduleActivation: { findMany, update } }, + default: { moduleActivation: { findMany, upsert } }, Prisma: {}, })); @@ -21,7 +21,7 @@ const module = { id: "thread-creator", version: "2.0.0" } as unknown as Module; describe("reconcileActivatedVersions", () => { beforeEach(() => { vi.clearAllMocks(); - update.mockResolvedValue(undefined); + upsert.mockResolvedValue(undefined); }); it("bumps activatedVersion to the live version for every drifted guild", async () => { @@ -32,18 +32,30 @@ describe("reconcileActivatedVersions", () => { await moduleService.reconcileActivatedVersions(module); - expect(update).toHaveBeenCalledTimes(2); - expect(update).toHaveBeenCalledWith({ + expect(upsert).toHaveBeenCalledTimes(2); + expect(upsert).toHaveBeenCalledWith({ where: { moduleId_guildId: { moduleId: "thread-creator", guildId: "guild-a" }, }, - data: { activatedVersion: "2.0.0" }, + create: { + moduleId: "thread-creator", + guildId: "guild-a", + activated: true, + activatedVersion: "2.0.0", + }, + update: { activatedVersion: "2.0.0" }, }); - expect(update).toHaveBeenCalledWith({ + expect(upsert).toHaveBeenCalledWith({ where: { moduleId_guildId: { moduleId: "thread-creator", guildId: "guild-b" }, }, - data: { activatedVersion: "2.0.0" }, + create: { + moduleId: "thread-creator", + guildId: "guild-b", + activated: true, + activatedVersion: "2.0.0", + }, + update: { activatedVersion: "2.0.0" }, }); }); @@ -68,7 +80,7 @@ describe("reconcileActivatedVersions", () => { await moduleService.reconcileActivatedVersions(module); - expect(update).not.toHaveBeenCalled(); + expect(upsert).not.toHaveBeenCalled(); }); }); diff --git a/src/core/services/module.service.ts b/src/core/services/module.service.ts index 1e58ae8b..59fe4df4 100644 --- a/src/core/services/module.service.ts +++ b/src/core/services/module.service.ts @@ -173,14 +173,20 @@ class ModuleService implements Service { guildId: string, version: string ) { - await prisma.moduleActivation.update({ + await prisma.moduleActivation.upsert({ where: { moduleId_guildId: { moduleId, guildId, }, }, - data: { + create: { + moduleId, + guildId, + activated: true, + activatedVersion: version, + }, + update: { activatedVersion: version, }, }); diff --git a/src/core/utils/core-messages.ts b/src/core/utils/core-messages.ts index 6258fd07..585d6bce 100644 --- a/src/core/utils/core-messages.ts +++ b/src/core/utils/core-messages.ts @@ -104,6 +104,20 @@ function renderCurrentValue( .join(", "); } +/** + * Welcome panel sent on `guildCreate` (system channel, or DM to the owner as + * a fallback). Deliberately generic: it never details a restored state. + */ +export const guildWelcomeMessage = (t: TFunction): ContainerBuilder[] => { + const container = new ContainerBuilder().setAccentColor(Colors.Turquoise); + container.addTextDisplayComponents( + (text) => text.setContent(t("guild.welcome.title")), + (text) => text.setContent(t("guild.welcome.body")), + (text) => text.setContent(t("guild.welcome.hint")) + ); + return [container]; +}; + /** * Fields rendered per page. Each field costs 3 components (section + text + * button) and the message-wide cap is 40, so a page is kept well under it, From a5c3d27f1dc7b84fe4aac84e22db54e67e313061 Mon Sep 17 00:00:00 2001 From: RedsTom Date: Wed, 16 Sep 2026 22:57:53 +0200 Subject: [PATCH 02/16] fix(core): harden module command registration and version sync Rethrow Discord failures from install/uninstall so the DB state only flips on success; skip the activatedVersion bump on failed updates so they retry next boot. Tolerate missing activatedVersion (""/null) as 0.0.0 instead of throwing. Resync downgraded guilds instead of leaving them behind. Fetch dev-guild module states in parallel and share one bulk-PUT helper between the dev and prod paths. Co-Authored-By: Claude Code --- .../command-loader.integration.test.ts | 152 +++++++++++++++++- src/core/loaders/command-loader.ts | 118 +++++++++----- 2 files changed, 225 insertions(+), 45 deletions(-) diff --git a/src/core/loaders/command-loader.integration.test.ts b/src/core/loaders/command-loader.integration.test.ts index 60988163..04d689e1 100644 --- a/src/core/loaders/command-loader.integration.test.ts +++ b/src/core/loaders/command-loader.integration.test.ts @@ -3,11 +3,17 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { Module } from "#lib/module.js"; // Avoid booting the bot / Prisma when importing the command loader's module graph. -vi.mock("#index.js", () => ({ modules: [], client: {} })); +vi.mock("#index.js", () => ({ + modules: [], + client: { user: { id: "app-1" }, token: "token" }, +})); vi.mock("#lib/database.js", () => ({ default: {}, Prisma: {} })); // Capture REST calls without any network I/O. -const { restPut } = vi.hoisted(() => ({ restPut: vi.fn() })); +const { restPut, restPost } = vi.hoisted(() => ({ + restPut: vi.fn(), + restPost: vi.fn(), +})); vi.mock("discord.js", async (importOriginal) => { const actual = (await importOriginal()) as typeof import("discord.js"); class FakeREST { @@ -15,12 +21,19 @@ vi.mock("discord.js", async (importOriginal) => { return this; } put = restPut; + post = restPost; } return { ...actual, REST: FakeREST }; }); vi.mock("#core/services/module.service.js", () => ({ - default: { getModuleStateFromGuildIdIn: vi.fn() }, + default: { + getModuleStateFromGuildIdIn: vi.fn(), + getModuleStateIn: vi.fn(), + enableModule: vi.fn(), + getGuildsWhereVersionDoesNotMatch: vi.fn(), + updateModuleActivation: vi.fn(), + }, })); // Core module with a single known command so we can assert it is always included. @@ -33,7 +46,12 @@ vi.mock("#core/core.module.js", () => ({ }, })); -const { loadDevGuildCommands } = await import("./command-loader.js"); +const { + loadDevGuildCommands, + installModuleCommandsIn, + checkCommandsForVersionChange, +} = await import("./command-loader.js"); +const { installModule } = await import("./module-installer.js"); const { default: moduleService } = await import("#core/services/module.service.js"); @@ -125,3 +143,129 @@ describe("loadDevGuildCommands", () => { expect(restPut).not.toHaveBeenCalled(); }); }); + +function fakeGuild(id: string) { + return { + id, + commands: { fetch: async () => [], delete: vi.fn() }, + } as unknown as Parameters[1]; +} + +describe("installModuleCommandsIn", () => { + beforeEach(() => { + vi.clearAllMocks(); + restPost.mockResolvedValue(undefined); + }); + + it("rethrows Discord failures instead of swallowing them", async () => { + restPost.mockRejectedValue(new Error("discord is down")); + + await expect( + installModuleCommandsIn( + client, + fakeModule("mod-a", ["alpha"]), + fakeGuild("guild-1") + ) + ).rejects.toThrow("discord is down"); + }); +}); + +describe("installModule", () => { + beforeEach(() => { + vi.clearAllMocks(); + restPost.mockResolvedValue(undefined); + vi.mocked(moduleService.getModuleStateIn).mockResolvedValue({ + activated: false, + } as never); + }); + + it("does not flip the DB state when command registration fails", async () => { + restPost.mockRejectedValue(new Error("discord is down")); + + await expect( + installModule(fakeModule("mod-a", ["alpha"]), fakeGuild("guild-1")) + ).rejects.toThrow("discord is down"); + expect(moduleService.enableModule).not.toHaveBeenCalled(); + }); + + it("enables the module when registration succeeds", async () => { + await installModule(fakeModule("mod-a", ["alpha"]), fakeGuild("guild-1")); + + expect(moduleService.enableModule).toHaveBeenCalledWith( + "mod-a", + expect.objectContaining({ id: "guild-1" }) + ); + }); +}); + +describe("checkCommandsForVersionChange", () => { + beforeEach(() => { + vi.clearAllMocks(); + restPost.mockResolvedValue(undefined); + vi.mocked( + moduleService.getGuildsWhereVersionDoesNotMatch + ).mockResolvedValue([ + { guildId: "guild-1", currentVersion: "1.0.0" }, + { guildId: "guild-2", currentVersion: "1.0.0" }, + ] as never); + }); + + function versionedClient() { + return { + ...client, + guilds: { fetch: async (id: string) => fakeGuild(id) }, + } as unknown as Client; + } + + function versionedModule() { + return { + ...fakeModule("mod-a", ["alpha"]), + version: "2.0.0", + } as unknown as Module; + } + + it("queues guilds with a missing version instead of throwing", async () => { + vi.mocked( + moduleService.getGuildsWhereVersionDoesNotMatch + ).mockResolvedValue([{ guildId: "guild-1", currentVersion: "" }] as never); + + await checkCommandsForVersionChange(versionedClient(), versionedModule()); + + expect(restPost).toHaveBeenCalled(); + expect(moduleService.updateModuleActivation).toHaveBeenCalledWith( + "mod-a", + "guild-1", + "2.0.0" + ); + }); + + it("resyncs downgraded guilds instead of leaving them behind", async () => { + vi.mocked( + moduleService.getGuildsWhereVersionDoesNotMatch + ).mockResolvedValue([ + { guildId: "guild-1", currentVersion: "3.0.0" }, + ] as never); + + await checkCommandsForVersionChange(versionedClient(), versionedModule()); + + expect(restPost).toHaveBeenCalled(); + expect(moduleService.updateModuleActivation).toHaveBeenCalledWith( + "mod-a", + "guild-1", + "2.0.0" + ); + }); + + it("bumps activatedVersion only for guilds whose update succeeded", async () => { + restPost.mockRejectedValueOnce(new Error("discord is down")); + + await checkCommandsForVersionChange(versionedClient(), versionedModule()); + + expect(moduleService.updateModuleActivation).toHaveBeenCalledTimes(1); + expect(moduleService.updateModuleActivation).toHaveBeenCalledWith( + "mod-a", + "guild-2", + "2.0.0" + ); + }); +}); diff --git a/src/core/loaders/command-loader.ts b/src/core/loaders/command-loader.ts index 721e1d1a..ef2814d4 100644 --- a/src/core/loaders/command-loader.ts +++ b/src/core/loaders/command-loader.ts @@ -9,6 +9,32 @@ import type { Version } from "#lib/version.js"; const logger = loggerMaker("commands"); +/** + * Single bulk PUT shared by the dev/prod registration paths, so they cannot + * diverge (client construction, error handling, logging). + */ +async function registerCommands( + client: Client, + route: `/${string}`, + body: { name: string }[], + scope: string +): Promise { + const rest = new REST().setToken(client.token!); + try { + await rest.put(route, { body }); + body.forEach((command) => { + logger.info(`\tRegistering command | name = ${command.name}`); + }); + logger.info( + `Successfully loaded commands | scope = ${scope} | count = ${body.length}` + ); + } catch (error) { + logger.error( + `Failed to load commands | scope = ${scope} | error = ${error}` + ); + } +} + /** * Registers the core commands globally (production). Global commands can take * up to ~1h to propagate; for fast iteration in development use @@ -23,20 +49,12 @@ export async function loadGlobalCommands(client: Client) { command.data.toJSON() ); - const rest = new REST().setToken(client.token); - try { - await rest.put(Routes.applicationCommands(client.user.id), { - body: coreCommands, - }); - coreCommands.forEach((command) => { - logger.info(`\tRegistering command | name = ${command.name}`); - }); - logger.info( - `Successfully loaded global commands | count = ${coreCommands.length}` - ); - } catch (error) { - logger.error(`Failed to load commands | error = ${error}`); - } + await registerCommands( + client, + Routes.applicationCommands(client.user!.id), + coreCommands, + "global" + ); } /** @@ -64,41 +82,38 @@ export async function loadDevGuildCommands( command.data.toJSON() ); - for (const module of modules) { - if (module.registry.commands.length === 0) { - continue; - } + // One round of parallel state lookups instead of N sequential queries. + const states = await Promise.all( + modules.map(async (module) => { + if (module.registry.commands.length === 0) return null; + const state = await moduleService.getModuleStateFromGuildIdIn( + module.id, + guildId + ); + return { module, activated: state.activated }; + }) + ); - const state = await moduleService.getModuleStateFromGuildIdIn( - module.id, - guildId - ); - if (!state.activated) { + for (const entry of states) { + if (!entry) continue; + if (!entry.activated) { logger.info( - `Skipping commands for disabled module on dev guild | module = ${module.id}` + `Skipping commands for disabled module on dev guild | module = ${entry.module.id}` ); continue; } commands.push( - ...module.registry.commands.map((command) => command.data.toJSON()) + ...entry.module.registry.commands.map((command) => command.data.toJSON()) ); } - const rest = new REST().setToken(client.token); - try { - await rest.put(Routes.applicationGuildCommands(client.user.id, guildId), { - body: commands, - }); - commands.forEach((command) => { - logger.info(`\tRegistering command | name = ${command.name}`); - }); - logger.info( - `Successfully loaded dev guild commands | guildId = ${guildId} | count = ${commands.length}` - ); - } catch (error) { - logger.error(`Failed to load dev guild commands | error = ${error}`); - } + await registerCommands( + client, + Routes.applicationGuildCommands(client.user!.id, guildId), + commands, + `dev-guild:${guildId}` + ); } export async function installModuleCommandsIn( @@ -138,6 +153,9 @@ export async function installModuleCommandsIn( { err: error }, `Failed to load commands for module "${module.id}" in guild "${guild.id}"` ); + // Rethrown on purpose: callers only flip the DB state on success, so a + // failed registration retries instead of showing as installed. + throw error; } } @@ -173,6 +191,8 @@ export async function uninstallModuleCommandsIn( { err: error }, `Failed to uninstall commands for module "${module.id}" in guild "${guild.id}"` ); + // Same contract as install: the caller only flips the DB state on success. + throw error; } } @@ -213,9 +233,16 @@ export async function checkCommandsForVersionChange( `\tFound ${guildInfos.length} guilds with version mismatch for module "${module.id}"` ); + // A missing version ("" default after a disable, or legacy null) means + // "older than anything": queue the guild instead of throwing. Any mismatch + // resyncs — including downgrades, so a rollback never leaves guilds behind + // forever on the newer command set. const guildsToFix = guildInfos.filter( (info) => - compareVersions(info.currentVersion as Version, module.version) < 0 + compareVersions( + (info.currentVersion || "0.0.0") as Version, + module.version + ) !== 0 ); logger.info( `\tFound ${guildsToFix.length} guilds to update commands for module "${module.id}"` @@ -230,7 +257,16 @@ export async function checkCommandsForVersionChange( `\tUpdating commands in guild "${guild.id}" for module "${module.id}"` ); - await updateModuleCommandsIn(client, module, guild); + try { + await updateModuleCommandsIn(client, module, guild); + } catch (err) { + // No version bump: the guild stays behind and retries on next boot. + logger.error( + { err }, + `Failed to update commands, will retry next boot | module = ${module.id} | guild = ${guild.id}` + ); + continue; + } await moduleService.updateModuleActivation( module.id, guild.id, From 9a9d6280ec7b9848aca9c432bebda994a4ea7bbc Mon Sep 17 00:00:00 2001 From: RedsTom Date: Wed, 16 Sep 2026 22:58:04 +0200 Subject: [PATCH 03/16] fix(core): run install hooks before flipping DB state A throwing onInstall/onUninstall no longer leaves the guild marked enabled/disabled, which used to deadlock the next install with "already installed". Co-Authored-By: Claude Code --- src/core/services/module.service.test.ts | 61 +++++++++++++++++++++++- src/core/services/module.service.ts | 12 +++-- 2 files changed, 68 insertions(+), 5 deletions(-) diff --git a/src/core/services/module.service.test.ts b/src/core/services/module.service.test.ts index 96125beb..68d2d64b 100644 --- a/src/core/services/module.service.test.ts +++ b/src/core/services/module.service.test.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { Module } from "#lib/module.js"; // The service pulls `client`/`modules` from the bot entrypoint and the Prisma @@ -15,6 +15,7 @@ vi.mock("#lib/database.js", () => ({ })); const { default: moduleService } = await import("./module.service.js"); +const { modules } = await import("#index.js"); const module = { id: "thread-creator", version: "2.0.0" } as unknown as Module; @@ -105,3 +106,61 @@ describe("getActivatedGuildIds", () => { }); }); }); + +describe("enableModule / disableModule hook ordering", () => { + const guild = { id: "guild-1" } as never; + const onInstall = vi.fn(); + const onUninstall = vi.fn(); + + beforeEach(() => { + vi.clearAllMocks(); + // clearAllMocks keeps implementations: drop them so one test's throwing + // hook does not leak into the next. + onInstall.mockReset(); + onUninstall.mockReset(); + upsert.mockResolvedValue(undefined); + (modules as unknown as Module[]).push({ + id: "mod-x", + version: "1.0.0", + registry: {}, + onInstall, + onUninstall, + } as unknown as Module); + }); + + afterEach(() => { + modules.length = 0; + }); + + it("does not mark the module enabled when onInstall throws", async () => { + onInstall.mockImplementation(() => { + throw new Error("hook blew up"); + }); + + await expect(moduleService.enableModule("mod-x", guild)).rejects.toThrow( + "hook blew up" + ); + expect(upsert).not.toHaveBeenCalled(); + }); + + it("runs onInstall before flipping the DB state", async () => { + await moduleService.enableModule("mod-x", guild); + + expect(onInstall).toHaveBeenCalledOnce(); + expect(upsert).toHaveBeenCalledOnce(); + expect(vi.mocked(onInstall).mock.invocationCallOrder[0]).toBeLessThan( + upsert.mock.invocationCallOrder[0] + ); + }); + + it("does not mark the module disabled when onUninstall throws", async () => { + onUninstall.mockImplementation(() => { + throw new Error("hook blew up"); + }); + + await expect(moduleService.disableModule("mod-x", guild)).rejects.toThrow( + "hook blew up" + ); + expect(upsert).not.toHaveBeenCalled(); + }); +}); diff --git a/src/core/services/module.service.ts b/src/core/services/module.service.ts index 59fe4df4..145e447e 100644 --- a/src/core/services/module.service.ts +++ b/src/core/services/module.service.ts @@ -87,6 +87,11 @@ class ModuleService implements Service { throw new Error(`Module with ID ${moduleId} not found`); } + // Hook first: a throwing onInstall must not leave the DB marked enabled, + // otherwise the next install throws "already installed" and the guild is + // stuck. + module.onInstall?.(client, guild, module.registry); + // Create a new activation record const activation = await prisma.moduleActivation.upsert({ where: { @@ -107,8 +112,6 @@ class ModuleService implements Service { }, }); - module.onInstall?.(client, guild, module.registry); - return activation; } @@ -118,6 +121,9 @@ class ModuleService implements Service { throw new Error(`Module with ID ${moduleId} not found`); } + // Same ordering as enableModule: hook first, DB flip only on success. + module.onUninstall?.(client, guild, module.registry); + const activation = await prisma.moduleActivation.upsert({ where: { moduleId_guildId: { @@ -137,8 +143,6 @@ class ModuleService implements Service { }, }); - module.onUninstall?.(client, guild, module.registry); - return activation; } From 9a5fd0fa0515cb5985b885165bed5a182d3df4d1 Mon Sep 17 00:00:00 2001 From: RedsTom Date: Wed, 16 Sep 2026 22:58:08 +0200 Subject: [PATCH 04/16] fix(core): drop vanished entities from config lists References to deleted channels/roles/users deserialized to null and crashed consumers (e.g. channel.id). Lists now filter them out with a structured warning instead of console.warn. Co-Authored-By: Claude Code --- docs/site/en/guide/configuration.md | 1 + docs/site/fr/guide/configuration.md | 1 + src/core/services/config.service.test.ts | 57 +++++++++++++++++++++--- src/core/services/config.service.ts | 16 +++++-- 4 files changed, 66 insertions(+), 9 deletions(-) diff --git a/docs/site/en/guide/configuration.md b/docs/site/en/guide/configuration.md index 0ab36280..883f93de 100644 --- a/docs/site/en/guide/configuration.md +++ b/docs/site/en/guide/configuration.md @@ -180,6 +180,7 @@ model GuildConfiguration { ``` - Entity IDs (user, role, channel) are stored as strings and **deserialized** to Discord objects at read time +- Entities deleted since (removed channel/role/…) are silently dropped from lists at read time, so `config.get()` never contains `null` - An in-memory cache (`configCache`) avoids database reads on every interaction - The cache is invalidated on every write diff --git a/docs/site/fr/guide/configuration.md b/docs/site/fr/guide/configuration.md index 295c4334..93a50380 100644 --- a/docs/site/fr/guide/configuration.md +++ b/docs/site/fr/guide/configuration.md @@ -180,6 +180,7 @@ model GuildConfiguration { ``` - Les IDs d'entités (utilisateur, rôle, salon) sont stockées sous forme de chaînes et **désérialisées** en objets Discord à la lecture +- Les entités supprimées depuis (salon/rôle/… retiré) sont écartées des listes à la lecture, donc `config.get()` ne contient jamais `null` - Un cache en mémoire (`configCache`) évite les lectures base de données à chaque interaction - Le cache est invalidé à chaque écriture diff --git a/src/core/services/config.service.test.ts b/src/core/services/config.service.test.ts index d73b032e..f45ec2e6 100644 --- a/src/core/services/config.service.test.ts +++ b/src/core/services/config.service.test.ts @@ -1,9 +1,14 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { ConfigType } from "#lib/config.js"; import type { Module } from "#lib/module.js"; +const { guildsFetch, channelsFetch } = vi.hoisted(() => ({ + guildsFetch: vi.fn(), + channelsFetch: vi.fn(), +})); vi.mock("#index.js", () => ({ modules: [], - client: { guilds: { fetch: vi.fn() } }, + client: { users: { fetch: vi.fn() }, guilds: { fetch: guildsFetch } }, })); vi.mock("#core/core.module.js", () => ({ default: { id: "core", config: {} }, @@ -54,8 +59,9 @@ vi.mock("#lib/database.js", () => { }); const { default: configService } = await import("./config.service.js"); +const { modules } = await import("#index.js"); -const module = { id: "core", config: {} } as unknown as Module; +const coreModule = { id: "core", config: {} } as unknown as Module; beforeEach(() => { rows.clear(); @@ -66,8 +72,8 @@ describe("ConfigService on a guild without stored configuration", () => { it("creates the configuration once when it is read concurrently", async () => { await expect( Promise.all([ - configService.getConfigForModuleIn(module, "new-guild"), - configService.getConfigForModuleIn(module, "new-guild"), + configService.getConfigForModuleIn(coreModule, "new-guild"), + configService.getConfigForModuleIn(coreModule, "new-guild"), ]) ).resolves.toHaveLength(2); @@ -76,7 +82,7 @@ describe("ConfigService on a guild without stored configuration", () => { }); it("does not fail when another process created the row in the meantime", async () => { - const first = configService.getConfigForModuleIn(module, "racing-guild"); + const first = configService.getConfigForModuleIn(coreModule, "racing-guild"); rows.set("racing-guild", { core: { locale: "fr" } }); const provider = await first; @@ -84,3 +90,42 @@ describe("ConfigService on a guild without stored configuration", () => { expect(provider.locale).toBe("fr"); }); }); + +const channelModule = { + id: "mod-a", + config: { + channels: { + name: "Channels", + description: "Watched channels", + type: [ConfigType.CHANNEL], + }, + }, +} as unknown as Module; + +describe("getConfigForModuleIn", () => { + beforeEach(() => { + (modules as unknown as Module[]).push(channelModule); + rows.set("guild-1", { + "mod-a": { channels: ["chan-1", "chan-gone"] }, + core: {}, + }); + guildsFetch.mockResolvedValue({ channels: { fetch: channelsFetch } }); + channelsFetch.mockImplementation(async (id: string) => { + if (id === "chan-gone") throw new Error("Unknown Channel"); + return { id }; + }); + }); + + afterEach(() => { + modules.length = 0; + }); + + it("drops deserialized entities that vanished instead of returning null", async () => { + const config = await configService.getConfigForModuleIn( + channelModule, + "guild-1" + ); + + expect(config.get("channels")).toEqual([{ id: "chan-1" }]); + }); +}); diff --git a/src/core/services/config.service.ts b/src/core/services/config.service.ts index 48013eee..f1da850b 100644 --- a/src/core/services/config.service.ts +++ b/src/core/services/config.service.ts @@ -8,9 +8,12 @@ import { type ConfigSchema, } from "#lib/config.js"; import database from "#lib/database.js"; +import { loggerMaker } from "#lib/logger.js"; import type { Module } from "#lib/module.js"; import { declareService } from "#lib/service.js"; +const logger = loggerMaker("config"); + const configCache = new Map>>(); const pendingLoads = new Map< string, @@ -299,11 +302,14 @@ class ConfigService { if (Array.isArray(configEntry.type)) { const listType = configEntry.type[0]; if (Array.isArray(value)) { - deserializedConfig[key] = await Promise.all( + const items = await Promise.all( value.map( async (item) => await this.deserializeValue(item, listType, guild) ) ); + // Drop entities that vanished since (deleted channel/role/user): a + // null item would crash consumers (e.g. `channel.id`). + deserializedConfig[key] = items.filter((item) => item !== null); } } else { // Handle single types @@ -365,8 +371,12 @@ class ConfigService { return value; } } catch (error) { - // If deserialization fails, return the original value or null - console.warn(`Failed to deserialize ${type} with value ${value}:`, error); + // The referenced entity is gone (deleted channel/role/…) or Discord + // refused the fetch: report null and let the caller drop it. + logger.warn( + { err: error }, + `Failed to deserialize | type = ${type} | value = ${value}` + ); return null; } } From 53978c12b4a645d115321d47d43c734341653de7 Mon Sep 17 00:00:00 2001 From: RedsTom Date: Wed, 16 Sep 2026 22:58:12 +0200 Subject: [PATCH 05/16] fix(core): explicit guild id extraction for listeners The old heuristic could return a member user id or a whole event object as guildId. Only real string ids resolve now; guild-less events run without config. Co-Authored-By: Claude Code --- src/core/loaders/listener-loader.test.ts | 103 +++++++++++++++++++++++ src/core/loaders/listener-loader.ts | 53 +++++------- 2 files changed, 125 insertions(+), 31 deletions(-) create mode 100644 src/core/loaders/listener-loader.test.ts diff --git a/src/core/loaders/listener-loader.test.ts b/src/core/loaders/listener-loader.test.ts new file mode 100644 index 00000000..bf4561c9 --- /dev/null +++ b/src/core/loaders/listener-loader.test.ts @@ -0,0 +1,103 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("#index.js", () => ({ modules: [], client: {} })); +vi.mock("#core/core.module.js", () => ({ + default: { id: "core", registry: { listeners: [] } }, +})); + +const { getModuleStateFromGuildIdIn, getConfigForModuleIn } = vi.hoisted( + () => ({ + getModuleStateFromGuildIdIn: vi.fn(), + getConfigForModuleIn: vi.fn(), + }) +); +vi.mock("#core/services/module.service.js", () => ({ + default: { getModuleStateFromGuildIdIn }, +})); +vi.mock("#core/services/config.service.js", () => ({ + default: { getConfigForModuleIn }, +})); + +const { loadModuleEvents, extractGuildId } = + await import("./listener-loader.js"); + +const fakeConfig = { tag: "config" }; + +function setup() { + const handlers = new Map void>(); + const client = { + on: vi.fn((event: string, handler: (...args: any[]) => void) => { + handlers.set(event, handler); + }), + } as any; + const execute = vi.fn(async () => ({})); + const module = { + id: "mod-a", + registry: { listeners: [{ eventType: "messageCreate", execute }] }, + } as any; + loadModuleEvents(client, module); + const handler = handlers.get("messageCreate")!; + return { handler, execute }; +} + +beforeEach(() => { + vi.clearAllMocks(); + getModuleStateFromGuildIdIn.mockResolvedValue({ activated: true }); + getConfigForModuleIn.mockResolvedValue(fakeConfig); +}); + +describe("extractGuildId", () => { + it("resolves through the guild, never the member user id", () => { + expect( + extractGuildId([{ roles: [], id: "user-1", guild: { id: "guild-9" } }]) + ).toBe("guild-9"); + }); + + it("accepts a string guildId payload", () => { + expect(extractGuildId([{ guildId: "guild-7" }])).toBe("guild-7"); + }); + + it("ignores a non-string guildId instead of passing garbage down", () => { + expect(extractGuildId([{ guildId: { id: "guild-7" } }])).toBeUndefined(); + }); + + it("returns undefined for guild-less payloads", () => { + expect(extractGuildId([{}])).toBeUndefined(); + expect(extractGuildId([])).toBeUndefined(); + }); +}); + +describe("loadModuleEvents", () => { + it("looks up state with the guild id, not the member id", async () => { + const { handler, execute } = setup(); + + handler({ roles: [], id: "user-1", guild: { id: "guild-9" } }); + await vi.waitFor(() => expect(execute).toHaveBeenCalled()); + + expect(getModuleStateFromGuildIdIn).toHaveBeenCalledWith( + "mod-a", + "guild-9" + ); + expect(execute.mock.calls[0]).toContain(fakeConfig); + }); + + it("runs without config and without state lookup for guild-less events", async () => { + const { handler, execute } = setup(); + + handler({}); + await vi.waitFor(() => expect(execute).toHaveBeenCalled()); + + expect(getModuleStateFromGuildIdIn).not.toHaveBeenCalled(); + expect(execute.mock.calls[0]).toContain(undefined); + }); + + it("skips disabled modules silently", async () => { + getModuleStateFromGuildIdIn.mockResolvedValue({ activated: false }); + const { handler, execute } = setup(); + + handler({ guild: { id: "guild-9" } }); + await new Promise((resolve) => setTimeout(resolve, 10)); + + expect(execute).not.toHaveBeenCalled(); + }); +}); diff --git a/src/core/loaders/listener-loader.ts b/src/core/loaders/listener-loader.ts index c3534301..fbdc9523 100644 --- a/src/core/loaders/listener-loader.ts +++ b/src/core/loaders/listener-loader.ts @@ -7,36 +7,6 @@ import type { Module } from "#lib/module.js"; const logger = loggerMaker("listeners"); -/** The shapes a Discord.js event argument can take when it carries a guild. */ -interface MaybeGuildScoped { - guild?: { id?: unknown } | null; - guildId?: unknown; -} - -/** - * Finds the guild an event happened in by scanning its arguments, since - * Discord.js gives no common interface for that. Returns `undefined` for events - * that carry no guild at all (a DM, say) rather than throwing: the caller then - * runs the listener without any guild-scoped config. - */ -function findGuildId(args: unknown[]): string | undefined { - for (const arg of args) { - const candidate = arg as MaybeGuildScoped | null | undefined; - - const fromGuild = candidate?.guild?.id; - if (typeof fromGuild === "string") { - return fromGuild; - } - - const fromGuildId = candidate?.guildId; - if (typeof fromGuildId === "string") { - return fromGuildId; - } - } - - return undefined; -} - /** * Loads global listeners from the core registry and registers them with Discord. * @@ -56,6 +26,27 @@ export function loadGlobalEvents(client: Client) { } } +/** + * Best-effort guild resolution for a raw client event payload. Only real + * guild ids are returned: a member-like object resolves through its guild + * (never its own user id), and a non-string `guildId` is ignored instead of + * being passed to the database layer. Returns undefined for guild-less + * events (DMs), in which case the listener runs without config. + */ +export function extractGuildId(args: unknown[]): string | undefined { + for (const arg of args) { + if (!arg || typeof arg !== "object") continue; + const record = arg as { guild?: { id?: unknown }; guildId?: unknown }; + if (record.guild && typeof record.guild.id === "string") { + return record.guild.id; + } + if (typeof record.guildId === "string") { + return record.guildId; + } + } + return undefined; +} + /** * Loads module-specific listeners from the core registry and registers them with Discord. */ @@ -74,7 +65,7 @@ export function loadModuleEvents(client: Client, module: Module) { logger.info(`\tRegistering listener | event = ${listener.eventType}`); client.on(listener.eventType, (...args) => { - const guildId = findGuildId(args); + const guildId = extractGuildId(args); if (guildId) { moduleService From 4f7854130349208770429979b2923c1c80e78c14 Mon Sep 17 00:00:00 2001 From: RedsTom Date: Wed, 16 Sep 2026 22:58:21 +0200 Subject: [PATCH 06/16] fix(core): dispatch commands and autocomplete safely Autocomplete now receives the command module config instead of the core one. Command execution is wrapped in try/catch with an ephemeral error reply (followUp when already answered) and guild-less interactions are ignored. New requiresAdmin flag on commands, enforced centrally and set on both core commands. Duplicate interaction customIds warn at dispatch; rejected interaction checks log at debug. Co-Authored-By: Claude Code --- docs/site/en/guide/commands.md | 3 +- src/core/commands/config.command.ts | 3 + src/core/commands/module.command.ts | 2 + src/core/i18n/en.json | 1 + src/core/i18n/fr.json | 1 + .../interaction-create.listener.test.ts | 208 ++++++++++++++++++ .../listeners/interaction-create.listener.ts | 157 ++++++++----- src/lib/command.ts | 7 + 8 files changed, 328 insertions(+), 54 deletions(-) create mode 100644 src/core/listeners/interaction-create.listener.test.ts diff --git a/docs/site/en/guide/commands.md b/docs/site/en/guide/commands.md index 41336d7a..2e0f9e9f 100644 --- a/docs/site/en/guide/commands.md +++ b/docs/site/en/guide/commands.md @@ -56,8 +56,7 @@ interface Command { The `config` parameter is a `ConfigProvider` that gives access to the module's configuration (see [Configuration](./configuration)). It's always injected — even for modules without a config schema. -> [!NOTE] -> In `complete()` (autocomplete) handlers, the injected `config` currently comes from the **Core** module rather than the command's module. This means module-specific config values are not available during autocomplete — only the Core module's config is accessible. +In `complete()` (autocomplete) handlers, the injected `config` is the command's own module config — the same provider `execute()` receives. ```typescript async execute(interaction, config) { diff --git a/src/core/commands/config.command.ts b/src/core/commands/config.command.ts index 37bcace0..356c1c49 100644 --- a/src/core/commands/config.command.ts +++ b/src/core/commands/config.command.ts @@ -29,6 +29,9 @@ export default declareCommand({ .setRequired(true) .setAutocomplete(true) ), + + requiresAdmin: true, + async execute(interaction) { const coreConfig = await configService.getConfigForModuleIn( coreModule, diff --git a/src/core/commands/module.command.ts b/src/core/commands/module.command.ts index ee37b4de..3da89048 100644 --- a/src/core/commands/module.command.ts +++ b/src/core/commands/module.command.ts @@ -20,6 +20,8 @@ export default declareCommand({ .setDefaultMemberPermissions(PERMISSION_ADMINISTRATOR) .setContexts([InteractionContextType.Guild]), + requiresAdmin: true, + async execute(interaction) { // Checked before deferring: requireAdmin replies, which needs a fresh // interaction. diff --git a/src/core/i18n/en.json b/src/core/i18n/en.json index 377f4e60..937a642c 100644 --- a/src/core/i18n/en.json +++ b/src/core/i18n/en.json @@ -31,6 +31,7 @@ "setConfig.label": "Enter a value ({{type}}):", "command.notEnabled": "The command `{{commandName}}` is not enabled in this guild.", + "command.failed": "Something went wrong while running `{{commandName}}`. Please try again later.", "interaction.malformed": "The button is malformed. Please try again later.", "interaction.moduleNotFound": "Module not found. Please try again later.", diff --git a/src/core/i18n/fr.json b/src/core/i18n/fr.json index d3cee25d..88e2b202 100644 --- a/src/core/i18n/fr.json +++ b/src/core/i18n/fr.json @@ -31,6 +31,7 @@ "setConfig.label": "Entrez une valeur ({{type}}) :", "command.notEnabled": "La commande `{{commandName}}` n'est pas activée sur ce serveur.", + "command.failed": "Quelque chose s'est mal passé lors de l'exécution de `{{commandName}}`. Veuillez réessayer plus tard.", "interaction.malformed": "Le bouton est mal formé. Veuillez réessayer plus tard.", "interaction.moduleNotFound": "Module introuvable. Veuillez réessayer plus tard.", diff --git a/src/core/listeners/interaction-create.listener.test.ts b/src/core/listeners/interaction-create.listener.test.ts new file mode 100644 index 00000000..d548ed75 --- /dev/null +++ b/src/core/listeners/interaction-create.listener.test.ts @@ -0,0 +1,208 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const { mockModules } = vi.hoisted(() => ({ mockModules: [] as any[] })); +vi.mock("#index.js", () => ({ modules: mockModules, client: {} })); +vi.mock("#core/core.module.js", () => ({ + default: { id: "core", registry: { commands: [], interactionHandlers: [] } }, +})); + +const { getModuleStateIn, getConfigForModuleIn } = vi.hoisted(() => ({ + getModuleStateIn: vi.fn(), + getConfigForModuleIn: vi.fn(), +})); +vi.mock("#core/services/module.service.js", () => ({ + default: { getModuleStateIn }, +})); +vi.mock("#core/services/config.service.js", () => ({ + default: { getConfigForModuleIn }, +})); + +const { default: listener } = await import("./interaction-create.listener.js"); + +function fakeCommand(commandName: string, overrides: Record = {}) { + return { + commandName, + guild: { id: "guild-1" }, + guildId: "guild-1", + isChatInputCommand: () => true, + isAutocomplete: () => false, + isMessageComponent: () => false, + isModalSubmit: () => false, + replied: false, + deferred: false, + reply: vi.fn(async () => ({})), + followUp: vi.fn(async () => ({})), + ...overrides, + } as any; +} + +function fakeAutocomplete(commandName: string) { + return { + commandName, + guild: { id: "guild-1" }, + guildId: "guild-1", + isChatInputCommand: () => false, + isAutocomplete: () => true, + isMessageComponent: () => false, + isModalSubmit: () => false, + respond: vi.fn(async () => ({})), + } as any; +} + +beforeEach(() => { + vi.clearAllMocks(); + mockModules.length = 0; + getModuleStateIn.mockResolvedValue({ activated: true }); + getConfigForModuleIn.mockImplementation(async (module: { id: string }) => ({ + moduleId: module.id, + t: (key: string) => key, + })); +}); + +describe("handleComplete", () => { + it("hands complete() the command module config, not the core one", async () => { + const complete = vi.fn(async () => ({})); + mockModules.push({ + id: "mod-a", + registry: { + commands: [{ data: { name: "hello" }, complete }], + interactionHandlers: [], + }, + }); + + const interaction = fakeAutocomplete("hello"); + await listener.execute(interaction, undefined); + + expect(getConfigForModuleIn).toHaveBeenCalledWith( + expect.objectContaining({ id: "mod-a" }), + "guild-1" + ); + expect(complete).toHaveBeenCalledOnce(); + expect(complete.mock.calls[0]?.[1]).toEqual( + expect.objectContaining({ moduleId: "mod-a" }) + ); + }); + + it("answers [] when complete() throws", async () => { + const complete = vi.fn(async () => { + throw new Error("boom"); + }); + mockModules.push({ + id: "mod-a", + registry: { + commands: [{ data: { name: "hello" }, complete }], + interactionHandlers: [], + }, + }); + + const interaction = fakeAutocomplete("hello"); + await expect( + listener.execute(interaction, undefined) + ).resolves.toBeUndefined(); + expect(interaction.respond).toHaveBeenCalledWith([]); + }); + + it("answers [] without running complete() for a disabled module", async () => { + const complete = vi.fn(async () => ({})); + mockModules.push({ + id: "mod-a", + registry: { + commands: [{ data: { name: "hello" }, complete }], + interactionHandlers: [], + }, + }); + getModuleStateIn.mockResolvedValue({ activated: false }); + + const interaction = fakeAutocomplete("hello"); + await listener.execute(interaction, undefined); + + expect(complete).not.toHaveBeenCalled(); + expect(interaction.respond).toHaveBeenCalledWith([]); + }); +}); + +describe("handleCommand", () => { + function pushCommand(command: any) { + mockModules.push({ + id: "mod-a", + registry: { commands: [command], interactionHandlers: [] }, + }); + } + + it("replies an error instead of crashing when execute() throws", async () => { + pushCommand({ + data: { name: "boom" }, + execute: vi.fn(async () => { + throw new Error("boom"); + }), + }); + + const interaction = fakeCommand("boom"); + await expect( + listener.execute(interaction, undefined) + ).resolves.toBeUndefined(); + + expect(interaction.reply).toHaveBeenCalledWith( + expect.objectContaining({ content: "command.failed" }) + ); + }); + + it("uses followUp when the interaction was already answered", async () => { + pushCommand({ + data: { name: "boom" }, + execute: vi.fn(async () => { + throw new Error("boom"); + }), + }); + + const interaction = fakeCommand("boom", { replied: true }); + await listener.execute(interaction, undefined); + + expect(interaction.reply).not.toHaveBeenCalled(); + expect(interaction.followUp).toHaveBeenCalledWith( + expect.objectContaining({ content: "command.failed" }) + ); + }); + + it("blocks non-admins from requiresAdmin commands", async () => { + const execute = vi.fn(async () => ({})); + pushCommand({ data: { name: "admin" }, requiresAdmin: true, execute }); + + const interaction = fakeCommand("admin", { + memberPermissions: { has: () => false }, + }); + await listener.execute(interaction, undefined); + + expect(execute).not.toHaveBeenCalled(); + expect(interaction.reply).toHaveBeenCalledWith( + expect.objectContaining({ content: "admin.noPermission" }) + ); + }); + + it("runs requiresAdmin commands for admins", async () => { + const execute = vi.fn(async () => ({})); + pushCommand({ data: { name: "admin" }, requiresAdmin: true, execute }); + + const interaction = fakeCommand("admin", { + memberPermissions: { has: () => true }, + }); + await listener.execute(interaction, undefined); + + expect(execute).toHaveBeenCalledOnce(); + }); + + it("ignores guild-less command interactions without touching the DB", async () => { + const execute = vi.fn(async () => ({})); + pushCommand({ data: { name: "hello" }, execute }); + + const interaction = fakeCommand("hello", { + guild: undefined, + guildId: undefined, + }); + await listener.execute(interaction, undefined); + + expect(execute).not.toHaveBeenCalled(); + expect(getConfigForModuleIn).not.toHaveBeenCalled(); + expect(interaction.reply).not.toHaveBeenCalled(); + }); +}); diff --git a/src/core/listeners/interaction-create.listener.ts b/src/core/listeners/interaction-create.listener.ts index 03615375..9f8758b1 100644 --- a/src/core/listeners/interaction-create.listener.ts +++ b/src/core/listeners/interaction-create.listener.ts @@ -21,14 +21,22 @@ function findCommand(commandName: string) { } function findInteractionHandler(customId: string) { - return [...modules, coreModule] + const matches = [...modules, coreModule] .flatMap((module) => module.registry.interactionHandlers.map((handler) => ({ module, handler, })) ) - .find((entry) => entry.handler.customId === customId); + .filter((entry) => entry.handler.customId === customId); + + if (matches.length > 1) { + logger.warn( + `Duplicate interaction customId, first match wins | customId = ${customId} | modules = ${matches.map((entry) => entry.module.id).join(",")}` + ); + } + + return matches[0]; } async function handleCommand(interaction: ChatInputCommandInteraction) { @@ -42,28 +50,24 @@ async function handleCommand(interaction: ChatInputCommandInteraction) { `Command found | name = ${command.command.data.name} | module = ${command.module.id}` ); - if ( - command.module.id === coreModule.id || - ( - await moduleService.getModuleStateIn( - command.module.id, - interaction.guild! - ) - ).activated - ) { - const config = await configService.getConfigForModuleIn( - command.module, - interaction.guildId! - ); + // Slash commands only run in guilds: without one there is no module state + // nor config to load. + if (!interaction.guild || !interaction.guildId) { + logger.warn(`Command outside guild | name = ${interaction.commandName}`); + return; + } - logger.debug(`Executing command | name = ${interaction.commandName}`); - await command.command.execute(interaction, config); - } else { - const coreConfig = await configService.getConfigForModuleIn( - coreModule, - interaction.guildId! - ); + const coreConfig = await configService.getConfigForModuleIn( + coreModule, + interaction.guildId + ); + const enabled = + command.module.id === coreModule.id || + (await moduleService.getModuleStateIn(command.module.id, interaction.guild)) + .activated; + + if (!enabled) { logger.warn( `Command not enabled | name = ${interaction.commandName} | module = ${command.module.id}` ); @@ -73,6 +77,40 @@ async function handleCommand(interaction: ChatInputCommandInteraction) { }), flags: MessageFlags.Ephemeral, }); + return; + } + + if (command.command.requiresAdmin) { + if (!(await requireAdmin(interaction, coreConfig.t))) { + return; + } + } + + const config = await configService.getConfigForModuleIn( + command.module, + interaction.guildId + ); + + logger.debug(`Executing command | name = ${interaction.commandName}`); + try { + await command.command.execute(interaction, config); + } catch (err) { + logger.error({ err }, `Command failed | name = ${interaction.commandName}`); + const payload = { + content: coreConfig.t("command.failed", { + commandName: interaction.commandName, + }), + flags: MessageFlags.Ephemeral, + } as const; + try { + if (interaction.replied || interaction.deferred) { + await interaction.followUp(payload); + } else { + await interaction.reply(payload); + } + } catch { + logger.debug(`Error reply failed | name = ${interaction.commandName}`); + } } } @@ -86,42 +124,49 @@ async function handleComplete(interaction: AutocompleteInteraction) { return; } + if (!interaction.guild || !interaction.guildId) { + logger.warn( + `Autocomplete outside guild | name = ${interaction.commandName}` + ); + await interaction.respond([]); + return; + } + if ( - command.module.id === coreModule.id || - ( - await moduleService.getModuleStateIn( - command.module.id, - interaction.guild! - ) + command.module.id !== coreModule.id && + !( + await moduleService.getModuleStateIn(command.module.id, interaction.guild) ).activated ) { - const config = await configService.getConfigForModuleIn( - coreModule, - interaction.guildId! - ); - - if ( - command.module.id === coreModule.id || - ( - await moduleService.getModuleStateIn( - command.module.id, - interaction.guild! - ) - ).activated - ) { - logger.debug(`Handling autocomplete | name = ${interaction.commandName}`); - await command.command.complete?.(interaction, config); - } else { - logger.warn( - `Command not enabled | name = ${interaction.command?.name} | module = ${command.module.id}` - ); - await interaction.respond([]); - } - } else { logger.warn( `Command not enabled | name = ${interaction.commandName} | module = ${command.module.id}` ); await interaction.respond([]); + return; + } + + // The module's own config, like handleCommand: complete() is typed against + // the command's schema, so handing it the core config would lie at runtime. + const config = await configService.getConfigForModuleIn( + command.module, + interaction.guildId + ); + + logger.debug(`Handling autocomplete | name = ${interaction.commandName}`); + try { + await command.command.complete?.(interaction, config); + } catch (err) { + logger.error( + { err }, + `Autocomplete failed | name = ${interaction.commandName}` + ); + try { + await interaction.respond([]); + } catch { + logger.debug( + `Autocomplete fallback failed | name = ${interaction.commandName}` + ); + } } } @@ -162,7 +207,15 @@ async function handleInteraction(interaction: CompatibleInteraction) { interaction.guildId! ); - if (!handler.handler.check(interaction, config)) return; + // Read before check(): a rejecting type predicate narrows `interaction` + // to never below. + const checkedCustomId = interaction.customId; + if (!handler.handler.check(interaction, config)) { + logger.debug( + `Interaction check rejected | customId = ${checkedCustomId}` + ); + return; + } if (handler.handler.access === "admin") { const coreConfig = await configService.getConfigForModuleIn( diff --git a/src/lib/command.ts b/src/lib/command.ts index aa9d96f8..9a3d5454 100644 --- a/src/lib/command.ts +++ b/src/lib/command.ts @@ -24,6 +24,13 @@ export interface Command { | SlashCommandSubcommandsOnlyBuilder | SlashCommandOptionsOnlyBuilder; + /** + * When true, the command is only executed for guild administrators; the + * permission check is enforced centrally by the command dispatcher. + * Defense in depth on top of `setDefaultMemberPermissions`. + */ + requiresAdmin?: boolean; + /** * The function to execute when the command is invoked. * From bed4e1e87f57b3e992d90a066fdd502081a28174 Mon Sep 17 00:00:00 2001 From: RedsTom Date: Wed, 16 Sep 2026 22:58:25 +0200 Subject: [PATCH 07/16] fix(core): resilient boot and optional lifecycle hooks A broken module import or a throwing onLoad no longer aborts the whole boot: the module is skipped with an error log. onLoad supports async and all three lifecycle hooks are optional, dropping the log-only boilerplate from the example modules. Co-Authored-By: Claude Code --- src/core/loaders/module-loader.test.ts | 23 +++++++++++++++++++ src/core/loaders/module-loader.ts | 15 +++++++----- src/index.ts | 14 +++++++++-- src/lib/module.ts | 9 ++++---- src/modules/test-config/test-config.module.ts | 8 ------- .../thread-creator/thread-creator.module.ts | 12 ---------- 6 files changed, 49 insertions(+), 32 deletions(-) create mode 100644 src/core/loaders/module-loader.test.ts diff --git a/src/core/loaders/module-loader.test.ts b/src/core/loaders/module-loader.test.ts new file mode 100644 index 00000000..05b10e02 --- /dev/null +++ b/src/core/loaders/module-loader.test.ts @@ -0,0 +1,23 @@ +import { mkdtemp, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; + +const { loadModule } = await import("./module-loader.js"); + +async function tempModuleDir(): Promise { + return mkdtemp(join(tmpdir(), "omnibot-mod-")); +} + +describe("loadModule", () => { + it("returns null when no entry point exists", async () => { + await expect(loadModule(await tempModuleDir())).resolves.toBeNull(); + }); + + it("skips a module whose import fails instead of throwing", async () => { + const dir = await tempModuleDir(); + await writeFile(join(dir, "broken.module.js"), "this is not valid js {{{"); + + await expect(loadModule(dir)).resolves.toBeNull(); + }); +}); diff --git a/src/core/loaders/module-loader.ts b/src/core/loaders/module-loader.ts index 0ef0a4aa..96d93c94 100644 --- a/src/core/loaders/module-loader.ts +++ b/src/core/loaders/module-loader.ts @@ -105,12 +105,15 @@ export async function loadModule(modulePath: string): Promise { const moduleFilePath = path.resolve(modulePath, moduleEntryPoint); - const imported: { default: Declared } = await import( - pathToFileURL(moduleFilePath).href - ); - - if (!imported) { - logger.warn(`\tFailed to import module | path = ${moduleFilePath}`); + let imported: { default: Declared }; + try { + imported = await import(pathToFileURL(moduleFilePath).href); + } catch (error) { + // One broken module must not take down the whole boot. + logger.error( + { err: error }, + `\tFailed to import module, skipping | path = ${moduleFilePath}` + ); return null; } diff --git a/src/index.ts b/src/index.ts index 392b984b..c21a16c7 100644 --- a/src/index.ts +++ b/src/index.ts @@ -42,11 +42,21 @@ export const client = new Client({ client.once(Events.ClientReady, async (readyClient) => { for (const module of modules) { - module.onLoad(readyClient, module.registry); + try { + await module.onLoad?.(readyClient, module.registry); + } catch (error) { + // One failing module must not prevent the others (and the core) + // from loading. + logger.error( + { err: error }, + `Module onLoad failed, skipping | id = ${module.id}` + ); + continue; + } loadModuleEvents(readyClient, module); } - coreModule.onLoad(readyClient, coreModule.registry); + await coreModule.onLoad?.(readyClient, coreModule.registry); await syncCommands(readyClient, modules); diff --git a/src/lib/module.ts b/src/lib/module.ts index 758d3477..a314abd2 100644 --- a/src/lib/module.ts +++ b/src/lib/module.ts @@ -53,11 +53,12 @@ export interface ModuleDeclaration { devOnly?: boolean; /** - * Called when the module is initialized at startup. + * Called when the module is initialized at startup. May be async; a + * throwing onLoad skips the module but never aborts the boot. * * @param client The Discord client instance. */ - onLoad: (client: Client, registry: Registry) => void; + onLoad?: (client: Client, registry: Registry) => void | Promise; /** * Called when the module is installed in a guild. @@ -65,7 +66,7 @@ export interface ModuleDeclaration { * @param client The Discord client instance. * @param guild The guild where the module is being installed. */ - onInstall: (client: Client, guild: Guild, registry: Registry) => void; + onInstall?: (client: Client, guild: Guild, registry: Registry) => void; /** * Called when the module is uninstalled from a guild. @@ -73,7 +74,7 @@ export interface ModuleDeclaration { * @param client The Discord client instance. * @param guild The guild from which the module is being uninstalled. */ - onUninstall: (client: Client, guild: Guild, registry: Registry) => void; + onUninstall?: (client: Client, guild: Guild, registry: Registry) => void; } /** diff --git a/src/modules/test-config/test-config.module.ts b/src/modules/test-config/test-config.module.ts index 1640bff7..d7ddde4c 100644 --- a/src/modules/test-config/test-config.module.ts +++ b/src/modules/test-config/test-config.module.ts @@ -110,12 +110,4 @@ export default defineModule({ onLoad() { logger.info("Module Test Config chargé (mode développement)"); }, - - onInstall(_client, guild) { - logger.info(`Module Test Config activé sur le serveur ${guild.id}`); - }, - - onUninstall(_client, guild) { - logger.info(`Module Test Config désactivé sur le serveur ${guild.id}`); - }, }); diff --git a/src/modules/thread-creator/thread-creator.module.ts b/src/modules/thread-creator/thread-creator.module.ts index 38730318..d49d5779 100644 --- a/src/modules/thread-creator/thread-creator.module.ts +++ b/src/modules/thread-creator/thread-creator.module.ts @@ -27,16 +27,4 @@ export default defineModule({ logger.info("Module Thread Creator chargé avec succès"); }, - - onInstall(_client, guild) { - logger.info( - `Module Thread Creator installé sur le serveur "${guild.name}" (${guild.id})` - ); - }, - - onUninstall(_client, guild) { - logger.info( - `Module Thread Creator désinstallé du serveur "${guild.name}" (${guild.id})` - ); - }, }); From b21b7e2b1488db31281311eed9cbd08832745cc9 Mon Sep 17 00:00:00 2001 From: RedsTom Date: Wed, 16 Sep 2026 22:58:29 +0200 Subject: [PATCH 08/16] fix(core): reject duplicate interaction customIds at registration Also drops the declared-but-never-propagated configType field from event listeners. Co-Authored-By: Claude Code --- src/lib/listener.ts | 5 ----- src/lib/registry.test.ts | 36 ++++++++++++++++++++++++++++++++++++ src/lib/registry.ts | 21 +++++++++++++++------ 3 files changed, 51 insertions(+), 11 deletions(-) create mode 100644 src/lib/registry.test.ts diff --git a/src/lib/listener.ts b/src/lib/listener.ts index 15278b48..0e375153 100644 --- a/src/lib/listener.ts +++ b/src/lib/listener.ts @@ -14,11 +14,6 @@ export interface EventListener< */ eventType: EventType; - /** - * The configuration schema for the module that registered the listener. - */ - configType?: ConfigType; - /** * The function to execute when the event is triggered. * diff --git a/src/lib/registry.test.ts b/src/lib/registry.test.ts new file mode 100644 index 00000000..a650c0cf --- /dev/null +++ b/src/lib/registry.test.ts @@ -0,0 +1,36 @@ +import type { ButtonInteraction } from "discord.js"; +import { describe, expect, it } from "vitest"; +import { declareInteractionHandler } from "./interaction.js"; +import { Registry } from "./registry.js"; + +describe("Registry", () => { + it("rejects duplicate interaction customIds", () => { + const registry = new Registry(); + const make = () => + declareInteractionHandler({ + customId: "dup", + check: (_interaction): _interaction is ButtonInteraction => true, + execute: async () => {}, + }); + + registry.register(make()); + expect(() => registry.register(make())).toThrow( + "Duplicate interaction customId" + ); + }); + + it("accepts distinct customIds", () => { + const registry = new Registry(); + const make = (customId: string) => + declareInteractionHandler({ + customId, + check: (_interaction): _interaction is ButtonInteraction => true, + execute: async () => {}, + }); + + registry.register(make("one")); + registry.register(make("two")); + + expect(registry.interactionHandlers).toHaveLength(2); + }); +}); diff --git a/src/lib/registry.ts b/src/lib/registry.ts index 2976861b..7e9c6699 100644 --- a/src/lib/registry.ts +++ b/src/lib/registry.ts @@ -94,13 +94,22 @@ export class Registry { handler as Declared> ); break; - case DeclarationType.Interaction: - this._interactionHandlers.push( - handler as Declared< - InteractionHandler - > - ); + case DeclarationType.Interaction: { + const interaction = handler as Declared< + InteractionHandler + >; + if ( + this._interactionHandlers.some( + (existing) => existing.customId === interaction.customId + ) + ) { + throw new Error( + `Duplicate interaction customId | customId = ${interaction.customId}` + ); + } + this._interactionHandlers.push(interaction); break; + } default: throw new Error(`Unknown declaration type | type = ${handler.type}`); } From 80ba27782b448e3dd52f1c77c24aa9cf4d683890 Mon Sep 17 00:00:00 2001 From: RedsTom Date: Wed, 16 Sep 2026 22:58:54 +0200 Subject: [PATCH 09/16] test(core): shared testing helpers for module authors New #lib/testing.js: makeTestConfig, fakeGuild/fakeChannel/fakeMessage, initTestI18n and silenceLogs, so module tests stop reinventing mocks (and stop accidentally booting the bot). The guild-create listener test is migrated onto them as proof. Co-Authored-By: Claude Code --- .../listeners/guild-create.listener.test.ts | 56 +++------ src/lib/testing.test.ts | 79 ++++++++++++ src/lib/testing.ts | 114 ++++++++++++++++++ 3 files changed, 207 insertions(+), 42 deletions(-) create mode 100644 src/lib/testing.test.ts create mode 100644 src/lib/testing.ts diff --git a/src/core/listeners/guild-create.listener.test.ts b/src/core/listeners/guild-create.listener.test.ts index 9bc441e3..e5a20fd9 100644 --- a/src/core/listeners/guild-create.listener.test.ts +++ b/src/core/listeners/guild-create.listener.test.ts @@ -1,6 +1,11 @@ import { MessageFlags } from "discord.js"; import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; -import { addTranslations, initI18n } from "#lib/i18n.js"; +import { + fakeChannel, + fakeGuild, + initTestI18n, + silenceLogs, +} from "#lib/testing.js"; const { mockModules } = vi.hoisted(() => ({ mockModules: [] as any[] })); vi.mock("#index.js", () => ({ modules: mockModules, client: {} })); @@ -29,49 +34,16 @@ vi.mock("#core/loaders/command-loader.js", () => ({ const { default: listener, resolveWelcomeChannel } = await import("./guild-create.listener.js"); -function fakeChannel(sendImpl?: (args: unknown) => Promise) { - return { - id: "chan-1", - isTextBased: () => true, - isSendable: () => true, - send: vi.fn(sendImpl ?? (async () => ({}))), - }; -} - -function fakeGuild(overrides: Record = {}) { - const channel = overrides.channel ?? fakeChannel(); - const ownerSend = overrides.ownerSend ?? vi.fn(async () => ({})); - return { - guild: { - id: "guild-1", - preferredLocale: "fr", - systemChannelId: "chan-1", - systemChannel: channel, - channels: { fetch: vi.fn(async () => channel) }, - members: { - me: { - permissionsIn: vi.fn(() => ({ has: () => true })), - }, - }, - fetchOwner: vi.fn(async () => ({ send: ownerSend })), - client: { tag: "fake-client" }, - ...overrides.guild, - } as any, - channel, - ownerSend, - }; -} +const welcomeBundle = { + "guild.welcome.title": "title", + "guild.welcome.body": "body", + "guild.welcome.hint": "hint", + "guild.welcome.dmPrefix": "prefix ", +}; beforeAll(async () => { - await initI18n(); - for (const lng of ["en", "fr"]) { - addTranslations(lng, "core", { - "guild.welcome.title": "title", - "guild.welcome.body": "body", - "guild.welcome.hint": "hint", - "guild.welcome.dmPrefix": "prefix ", - }); - } + await initTestI18n("core", { en: welcomeBundle, fr: welcomeBundle }); + silenceLogs(); }); beforeEach(() => { diff --git a/src/lib/testing.test.ts b/src/lib/testing.test.ts new file mode 100644 index 00000000..0321e187 --- /dev/null +++ b/src/lib/testing.test.ts @@ -0,0 +1,79 @@ +import { beforeAll, describe, expect, it } from "vitest"; +import { ConfigType, type ConfigSchema } from "./config.js"; +import { + fakeChannel, + fakeGuild, + fakeMessage, + initTestI18n, + makeTestConfig, + silenceLogs, +} from "./testing.js"; + +const schema = { + title: { + name: "Title", + description: "A title", + type: ConfigType.STRING, + defaultValue: "hi", + }, + count: { + name: "Count", + description: "A count", + type: ConfigType.NUMBER, + }, +} satisfies ConfigSchema; + +beforeAll(async () => { + // String defaults resolve through i18n: init once, like any consumer. + await initTestI18n(); + silenceLogs(); +}); + +describe("makeTestConfig", () => { + it("serves stored values and schema defaults without a DB", () => { + const config = makeTestConfig("mod-x", schema, { count: 3 }); + + expect(config.get("count")).toBe(3); + expect(config.get("title")).toBe("hi"); + expect(config.isSet("count")).toBe(true); + expect(config.isSet("title")).toBe(false); + }); +}); + +describe("fakes", () => { + it("builds a guild with a sendable system channel", async () => { + const { guild, channel, ownerSend } = fakeGuild(); + + expect(guild.systemChannel).toBe(channel); + await channel.send("hello"); + expect(channel.send).toHaveBeenCalledWith("hello"); + expect(ownerSend).not.toHaveBeenCalled(); + }); + + it("lets callers override sending to simulate failures", async () => { + const channel = fakeChannel(async () => { + throw Object.assign(new Error("forbidden"), { code: 50013 }); + }); + + await expect(channel.send("hi")).rejects.toThrow("forbidden"); + }); + + it("builds a non-bot guild text message", () => { + const message = fakeMessage({ content: "ping" }); + + expect(message.author.bot).toBe(false); + expect(message.content).toBe("ping"); + expect(message.channel.isThread()).toBe(false); + }); +}); + +describe("initTestI18n", () => { + it("initializes once and registers bundles", async () => { + silenceLogs(); + await initTestI18n("core", { en: { "test.key": "value" } }); + await initTestI18n("core", { en: { "test.key": "value" } }); + + const { createT } = await import("./i18n.js"); + expect(createT("en", "core")("test.key")).toBe("value"); + }); +}); diff --git a/src/lib/testing.ts b/src/lib/testing.ts new file mode 100644 index 00000000..667801b8 --- /dev/null +++ b/src/lib/testing.ts @@ -0,0 +1,114 @@ +/** + * Test-only helpers for module authors. Import from `*.test.ts` files only — + * never from runtime code. + * + * What this file does NOT do: `vi.mock("#index.js")` and + * `vi.mock("#lib/database.js")` must still be declared at the top of each + * test file. Vitest hoists those calls above imports, so no helper can hide + * them. Forgetting the `#index.js` mock boots the real bot (Prisma + login), + * forgetting the database mock hits a real database. + */ +import { ChannelType } from "discord.js"; +import { vi } from "vitest"; +import { + ConfigProvider, + type ConfigData, + type ConfigSchema, +} from "./config.js"; +import { addTranslations, initI18n } from "./i18n.js"; +import baseLogger from "./logger.js"; +import type { Module } from "./module.js"; + +let i18nReady = false; + +/** `initI18n()` once per file, then registers the given bundles. */ +export async function initTestI18n( + namespace = "core", + resources: Record> = {} +): Promise { + if (!i18nReady) { + await initI18n(); + i18nReady = true; + } + for (const [lng, bundle] of Object.entries(resources)) { + addTranslations(lng, namespace, bundle); + } +} + +/** Quiet pino for the rest of the file (failed-path tests log a lot). */ +export function silenceLogs(): void { + baseLogger.level = "silent"; +} + +/** A `ConfigProvider` for `schema` with `values` stored, no DB involved. */ +export function makeTestConfig( + moduleId: string, + schema: TSchema, + values: Partial> = {}, + locale = "en" +): ConfigProvider { + const module = { id: moduleId, config: schema } as unknown as Module; + return new ConfigProvider(module, values as ConfigData, locale); +} + +/** Sendable text channel double. Override sending via `sendImpl`. */ +export function fakeChannel( + sendImpl: (args: unknown) => Promise = async () => ({}) +) { + return { + id: "chan-1", + isTextBased: () => true, + isSendable: () => true, + send: vi.fn(sendImpl), + }; +} + +export interface FakeGuildSet { + guild: any; + channel: ReturnType; + ownerSend: ReturnType; +} + +/** + * Guild double with a system channel, member permissions and owner DM. + * Override pieces via `{ channel, ownerSend, guild: { … } }`. + */ +export function fakeGuild(overrides: Record = {}): FakeGuildSet { + const channel = overrides["channel"] ?? fakeChannel(); + const ownerSend = overrides["ownerSend"] ?? vi.fn(async () => ({})); + return { + guild: { + id: "guild-1", + preferredLocale: "fr", + systemChannelId: "chan-1", + systemChannel: channel, + channels: { fetch: vi.fn(async () => channel) }, + members: { + me: { + permissionsIn: vi.fn(() => ({ has: () => true })), + }, + }, + fetchOwner: vi.fn(async () => ({ send: ownerSend })), + client: { tag: "fake-client" }, + ...overrides["guild"], + }, + channel, + ownerSend, + }; +} + +/** Minimal `messageCreate` payload for listener tests. */ +export function fakeMessage(overrides: Record = {}) { + return { + id: "msg-1", + content: "hello world", + author: { bot: false, displayName: "Ada", username: "ada" }, + guild: { id: "guild-1" }, + channel: { + id: "chan-1", + type: ChannelType.GuildText, + isThread: () => false, + }, + ...overrides, + }; +} From 0fcdff71074da555b20d0f6f5b3d7769003c01db Mon Sep 17 00:00:00 2001 From: RedsTom Date: Wed, 16 Sep 2026 22:59:26 +0200 Subject: [PATCH 10/16] refactor: env validator in TypeScript with shared dev constant validate-env-vars runs under tsx so it imports the single dev-mode definition from #lib/env.js instead of duplicating the development literal. dev/start scripts updated to match. Co-Authored-By: Claude Code --- ...alidate-env-vars.mjs => validate-env-vars.ts} | 16 ++++++++++------ package.json | 4 ++-- 2 files changed, 12 insertions(+), 8 deletions(-) rename bootstrap/{validate-env-vars.mjs => validate-env-vars.ts} (71%) diff --git a/bootstrap/validate-env-vars.mjs b/bootstrap/validate-env-vars.ts similarity index 71% rename from bootstrap/validate-env-vars.mjs rename to bootstrap/validate-env-vars.ts index 3b529ee8..9f15437a 100644 --- a/bootstrap/validate-env-vars.mjs +++ b/bootstrap/validate-env-vars.ts @@ -2,7 +2,13 @@ // missing env vars in the application code. // Let's just crash before starting if missing // values. -const REQUIRED_ENV_VARS = { +// +// Runs under tsx (see the `dev`/`start` scripts), so the single dev-mode +// definition in `#lib/env.js` can be imported instead of duplicating the +// "development" literal here. +import { isDevMode } from "#lib/env.js"; + +const REQUIRED_ENV_VARS: Record = { DISCORD_TOKEN: { sensitive: true, }, @@ -11,14 +17,12 @@ const REQUIRED_ENV_VARS = { }, // Required only in development: the guild where core commands are registered // instantly instead of globally (see command-loader). - ...(process.env.NODE_ENV === "development" - ? { DEV_GUILD_ID: { sensitive: false } } - : {}), + ...(isDevMode() ? { DEV_GUILD_ID: { sensitive: false } } : {}), }; // --- Functions --- -function exitIfMissing(envVarName) { +function exitIfMissing(envVarName: string): void { const envVar = process.env[envVarName]; if (envVar === undefined || envVar.length === 0) { console.error(`Missing required environment variable '${envVarName}'`); @@ -26,7 +30,7 @@ function exitIfMissing(envVarName) { } } -function displayEnvironmentVariables() { +function displayEnvironmentVariables(): void { console.log( "Starting the application with the following environment variables:" ); diff --git a/package.json b/package.json index 577ee338..8488704b 100644 --- a/package.json +++ b/package.json @@ -16,8 +16,8 @@ }, "scripts": { "build": "pnpm prisma:generate && tsc", - "dev": "NODE_ENV=development node --env-file=.env ./bootstrap/validate-env-vars.mjs && NODE_ENV=development node --conditions=development --env-file=.env --import tsx src/index.ts", - "start": "node ./bootstrap/validate-env-vars.mjs && node ./dist/index.js", + "dev": "NODE_ENV=development node --conditions=development --env-file=.env --import tsx ./bootstrap/validate-env-vars.ts && NODE_ENV=development node --conditions=development --env-file=.env --import tsx src/index.ts", + "start": "node --import tsx ./bootstrap/validate-env-vars.ts && node ./dist/index.js", "test": "vitest run", "test:unit": "vitest run --project unit", "test:integration": "vitest run --project integration", From 6619d1dfe15ad142f7364d20bb69dca36744be1f Mon Sep 17 00:00:00 2001 From: RedsTom Date: Wed, 16 Sep 2026 22:59:41 +0200 Subject: [PATCH 11/16] feat: scaffold new modules with pnpm new-module Generates a compiling, tested module skeleton (definition, config schema, namespaced example command, guarded example listener, en/fr i18n, commented Prisma model, test on the shared helpers). Co-Authored-By: Claude Code --- package.json | 1 + scripts/new-module.ts | 194 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 195 insertions(+) create mode 100644 scripts/new-module.ts diff --git a/package.json b/package.json index 8488704b..09461cd2 100644 --- a/package.json +++ b/package.json @@ -23,6 +23,7 @@ "test:integration": "vitest run --project integration", "prepare": "lefthook install -f || true", "format": "oxfmt --write && oxlint --fix --fix-suggestions", + "new-module": "tsx scripts/new-module.ts", "prisma:consolidate": "tsx scripts/consolidate-schema.ts", "prisma:generate": "pnpm prisma:consolidate && prisma generate --no-hints", "prisma:migrate": "pnpm prisma:consolidate && prisma migrate dev", diff --git a/scripts/new-module.ts b/scripts/new-module.ts new file mode 100644 index 00000000..70ed42ea --- /dev/null +++ b/scripts/new-module.ts @@ -0,0 +1,194 @@ +import fs from "node:fs/promises"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); + +function toCamel(id: string): string { + return id.replace(/-([a-z0-9])/g, (_, c: string) => c.toUpperCase()); +} + +function toPascal(id: string): string { + const camel = toCamel(id); + return camel.charAt(0).toUpperCase() + camel.slice(1); +} + +const MODULE_TS = `import { GatewayIntentBits } from "discord.js"; +import { defineModule } from "#lib/module.js"; +import helloCommand from "./commands/hello.command.js"; +import messageListener from "./listeners/message.listener.js"; +import { __CAMEL__ConfigSchema } from "./__ID__.config.js"; + +export default defineModule({ + id: "__ID__", + name: "__NAME__", + description: "TODO: describe what this module does.", + version: "0.1.0", + + config: __CAMEL__ConfigSchema, + + // Non-privileged intents only: no portal toggle needed. Add + // GatewayIntentBits.MessageContent only if you read message text (privileged). + intents: [GatewayIntentBits.Guilds, GatewayIntentBits.GuildMessages], + + onLoad(_client, registry) { + registry.register(helloCommand); + registry.register(messageListener); + }, +}); +`; + +const CONFIG_TS = `import { ConfigType, type ConfigSchema } from "#lib/config.js"; + +export const __CAMEL__ConfigSchema = { + announcement: { + name: "Announcement", + description: "Message posted by the hello command.", + type: ConfigType.STRING, + defaultValue: "Hello!", + }, +} satisfies ConfigSchema; + +export type __PASCAL__ConfigSchema = typeof __CAMEL__ConfigSchema; +`; + +const COMMAND_TS = `import { MessageFlags, SlashCommandBuilder } from "discord.js"; +import { declareCommand } from "#lib/command.js"; +import type { __PASCAL__ConfigSchema } from "../__ID__.config.js"; + +// Command names are matched first-come-first-served across ALL modules: +// always prefix with your module id. +export default declareCommand<__PASCAL__ConfigSchema>({ + data: new SlashCommandBuilder() + .setName("__ID__-hello") + .setDescription("Replies with the configured announcement."), + async execute(interaction, config) { + await interaction.reply({ + content: config.get("announcement"), + flags: MessageFlags.Ephemeral, + }); + }, +}); +`; + +const LISTENER_TS = `import { ChannelType } from "discord.js"; +import { declareEventListener } from "#lib/listener.js"; +import logger from "#lib/logger.js"; +import type { __PASCAL__ConfigSchema } from "../__ID__.config.js"; + +export default declareEventListener<"messageCreate", __PASCAL__ConfigSchema>({ + eventType: "messageCreate", + async execute(message, config) { + if (message.author.bot) return; + if (!message.guild) return; + if (message.channel.type !== ChannelType.GuildText) return; + // Listeners run with \`undefined\` config outside guilds: never drop this guard. + if (!config) return; + + logger.debug( + \`Saw message | guild = \${message.guild.id} | announcement = \${config.get("announcement")}\` + ); + }, +}); +`; + +const I18N_EN = `{ + "modules.__ID__.name": "__NAME__", + "modules.__ID__.description": "TODO: describe what this module does.", + "config.announcement.name": "Announcement", + "config.announcement.description": "Message posted by the hello command." +} +`; + +const I18N_FR = `{ + "modules.__ID__.name": "__NAME__", + "modules.__ID__.description": "TODO: décrivez ce que fait ce module.", + "config.announcement.name": "Annonce", + "config.announcement.description": "Message posté par la commande hello." +} +`; + +const MODEL_PRISMA = `// Database models for the __ID__ module. +// Uncomment and adapt the example below, then run: +// pnpm prisma:generate && pnpm prisma:migrate +// Model names must be unique across ALL modules (everything is consolidated +// into a single schema). +// +// model __Pascal__Thing { +// id String @id @default(cuid()) +// guildId String +// createdAt DateTime @default(now()) +// } +`; + +const TEST_TS = `import { describe, expect, it } from "vitest"; +import { initTestI18n, makeTestConfig } from "#lib/testing.js"; +import { __CAMEL__ConfigSchema } from "./__ID__.config.js"; + +describe("__ID__ config", () => { + it("serves schema defaults without a DB", async () => { + await initTestI18n(); + const config = makeTestConfig("__ID__", __CAMEL__ConfigSchema, {}); + + expect(config.get("announcement")).toBe("Hello!"); + expect(config.isSet("announcement")).toBe(false); + }); +}); +`; + +async function main(): Promise { + const [id, ...nameParts] = process.argv.slice(2); + + if (!id || !/^[a-z0-9-]+$/.test(id)) { + console.error("Usage: pnpm new-module [Display Name]"); + console.error(" must match /^[a-z0-9-]+$/ (e.g. my-module)"); + process.exit(1); + } + + const name = nameParts.join(" ") || toPascal(id); + const camel = toCamel(id); + const pascal = toPascal(id); + const fill = (template: string) => + template + .replaceAll("__ID__", id) + .replaceAll("__CAMEL__", camel) + .replaceAll("__PASCAL__", pascal) + .replaceAll("__NAME__", name); + + const dir = path.join(__dirname, "..", "src", "modules", id); + try { + await fs.stat(dir); + console.error(`Refusing to overwrite existing directory: ${dir}`); + process.exit(1); + } catch { + // Missing directory: the expected case, keep going. + } + + const files: Record = { + [`${id}.module.ts`]: MODULE_TS, + [`${id}.config.ts`]: CONFIG_TS, + [`commands/hello.command.ts`]: COMMAND_TS, + [`listeners/message.listener.ts`]: LISTENER_TS, + [`i18n/en.json`]: I18N_EN, + [`i18n/fr.json`]: I18N_FR, + [`models/${id}.prisma`]: MODEL_PRISMA, + [`${id}.test.ts`]: TEST_TS, + }; + + for (const [relative, template] of Object.entries(files)) { + const fullPath = path.join(dir, relative); + await fs.mkdir(path.dirname(fullPath), { recursive: true }); + await fs.writeFile(fullPath, fill(template)); + console.log(` created ${path.relative(process.cwd(), fullPath)}`); + } + + console.log(`\nModule "${id}" scaffolded. Next steps:`); + console.log(` 1. Fill the TODO description in ${id}.module.ts`); + console.log(` 2. pnpm test:unit --project unit src/modules/${id}`); + console.log(` 3. pnpm dev, then enable it via /modules on your dev guild`); +} + +main().catch((error) => { + console.error(error); + process.exit(1); +}); From 3f3c1d3e82c702c5db02a11dba3967f6af66a222 Mon Sep 17 00:00:00 2001 From: RedsTom Date: Wed, 16 Sep 2026 22:59:44 +0200 Subject: [PATCH 12/16] feat(prisma): fail fast on duplicate model names The consolidator now reports the model and both files instead of failing late at generate time. Covered by a unit test; scripts tests join the unit project. Co-Authored-By: Claude Code --- scripts/consolidate-schema.test.ts | 34 +++++++++++++++++++ scripts/consolidate-schema.ts | 52 ++++++++++++++++++++++++++++-- vitest.config.ts | 2 +- 3 files changed, 85 insertions(+), 3 deletions(-) create mode 100644 scripts/consolidate-schema.test.ts diff --git a/scripts/consolidate-schema.test.ts b/scripts/consolidate-schema.test.ts new file mode 100644 index 00000000..78f0e8a6 --- /dev/null +++ b/scripts/consolidate-schema.test.ts @@ -0,0 +1,34 @@ +import { describe, expect, it } from "vitest"; +import { findDuplicatePrismaModel } from "./consolidate-schema.js"; + +describe("findDuplicatePrismaModel", () => { + it("returns null when every model name is unique", () => { + expect( + findDuplicatePrismaModel([ + { path: "a.prisma", content: "model Alpha {\n id String @id\n}" }, + { path: "b.prisma", content: "model Beta {\n id String @id\n}" }, + ]) + ).toBeNull(); + }); + + it("names the model and both files on collision", () => { + expect( + findDuplicatePrismaModel([ + { path: "a.prisma", content: "model Alpha {\n id String @id\n}" }, + { + path: "b.prisma", + content: "// comment\nmodel Alpha {\n id String @id\n}", + }, + ]) + ).toEqual({ model: "Alpha", first: "a.prisma", second: "b.prisma" }); + }); + + it("ignores commented-out models", () => { + expect( + findDuplicatePrismaModel([ + { path: "a.prisma", content: "// model Alpha {\n// }" }, + { path: "b.prisma", content: "model Alpha {\n id String @id\n}" }, + ]) + ).toBeNull(); + }); +}); diff --git a/scripts/consolidate-schema.ts b/scripts/consolidate-schema.ts index 8da4fe6d..4480ac0f 100644 --- a/scripts/consolidate-schema.ts +++ b/scripts/consolidate-schema.ts @@ -42,6 +42,40 @@ async function findPrismaFiles(dir: string): Promise { return files; } +export interface PrismaSource { + path: string; + content: string; +} + +export interface DuplicateModel { + model: string; + first: string; + second: string; +} + +/** + * Everything is merged into a single schema, so a model name must be unique + * across ALL module files. Prisma itself only reports this late at + * `generate` time with a cryptic error — fail here with both file paths. + */ +export function findDuplicatePrismaModel( + files: PrismaSource[] +): DuplicateModel | null { + const origins = new Map(); + for (const file of files) { + for (const line of file.content.split("\n")) { + const match = line.match(/^\s*model\s+(\w+)/); + if (!match?.[1]) continue; + const first = origins.get(match[1]); + if (first) { + return { model: match[1], first, second: file.path }; + } + origins.set(match[1], file.path); + } + } + return null; +} + async function consolidateSchema() { const srcDir = path.join(__dirname, "..", "src"); const schemaPath = path.join(srcDir, "prisma", "schema.prisma"); @@ -53,6 +87,7 @@ async function consolidateSchema() { // Lire le contenu du header.prisma let consolidatedContent = ""; + const sources: PrismaSource[] = []; // Ajouter le contenu de tous les fichiers prisma trouvés for (const file of prismaFiles) { @@ -73,11 +108,19 @@ async function consolidateSchema() { .trim(); consolidatedContent += cleanContent + "\n"; + sources.push({ path: relativePath, content: cleanContent }); } catch (error) { console.warn(`Erreur lors de la lecture de ${file}:`, error); } } + const duplicate = findDuplicatePrismaModel(sources); + if (duplicate) { + throw new Error( + `Duplicate Prisma model "${duplicate.model}" in "${duplicate.first}" and "${duplicate.second}": model names must be unique across all modules.` + ); + } + // Écrire le schéma consolidé await fs.writeFile(schemaPath, consolidatedContent); @@ -90,5 +133,10 @@ async function consolidateSchema() { }); } -// Exécuter la consolidation -consolidateSchema().catch(console.error); +// Exécuter la consolidation (seulement en run direct, pas à l'import en test) +if (process.argv[1]?.endsWith("consolidate-schema.ts")) { + consolidateSchema().catch((error) => { + console.error(error); + process.exitCode = 1; + }); +} diff --git a/vitest.config.ts b/vitest.config.ts index cd896461..bd8880e5 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -13,7 +13,7 @@ export default defineConfig({ test: { name: "unit", environment: "node", - include: ["src/**/*.test.ts"], + include: ["src/**/*.test.ts", "scripts/**/*.test.ts"], exclude: [...configDefaults.exclude, integrationGlob], }, }, From 9f8d603fbc7d2865e6766a90a24943eef74a0cd7 Mon Sep 17 00:00:00 2001 From: RedsTom Date: Wed, 16 Sep 2026 23:00:08 +0200 Subject: [PATCH 13/16] feat: log level via env with debug fallback LOG_LEVEL accepts fatal/error/warn/info/debug/trace/silent and falls back to debug otherwise. Documented in AGENTS.md. Co-Authored-By: Claude Code --- AGENTS.md | 1 + src/lib/logger.ts | 17 ++++++++++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 9d4d740a..b036192e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -60,3 +60,4 @@ Copy `.env.example` to `.env`: - `DISCORD_TOKEN` — bot token from Discord Developer Portal - `DATABASE_URL` — PostgreSQL connection string (default matches `compose.yaml`) +- `LOG_LEVEL` — optional log level (`fatal`/`error`/`warn`/`info`/`debug`/`trace`/`silent`, default `debug`); invalid values fall back to `debug` diff --git a/src/lib/logger.ts b/src/lib/logger.ts index de9b2216..75b37dfd 100644 --- a/src/lib/logger.ts +++ b/src/lib/logger.ts @@ -1,7 +1,22 @@ import pino from "pino"; +function resolveLevel(): pino.LevelWithSilent { + switch (process.env["LOG_LEVEL"]) { + case "fatal": + case "error": + case "warn": + case "info": + case "debug": + case "trace": + case "silent": + return process.env["LOG_LEVEL"]; + default: + return "debug"; + } +} + const base = pino({ - level: "debug", + level: resolveLevel(), transport: { target: "pino-pretty", options: { From 664612a3917aa447d69e2a2e1fa33dec5795fdde Mon Sep 17 00:00:00 2001 From: RedsTom Date: Wed, 16 Sep 2026 23:00:28 +0200 Subject: [PATCH 14/16] docs: module scaffold and publishing checklist Document pnpm new-module and the shared test helpers, plus a pre-production checklist (version bump, prefixed names, DM guards, requiresAdmin, prisma order). Co-Authored-By: Claude Code --- docs/site/en/guide/creating-a-module.md | 26 +++++++++++++++++++++++++ docs/site/fr/guide/creating-a-module.md | 26 ++++++++++++++++++++++++- 2 files changed, 51 insertions(+), 1 deletion(-) diff --git a/docs/site/en/guide/creating-a-module.md b/docs/site/en/guide/creating-a-module.md index c60f5a52..4fbf2235 100644 --- a/docs/site/en/guide/creating-a-module.md +++ b/docs/site/en/guide/creating-a-module.md @@ -2,6 +2,18 @@ A module in OmniBot is a self-contained functional unit that can be installed and uninstalled per Discord server. Each module can contain commands, event listeners, interaction handlers, services, configuration, and database models. +## Scaffolding a Module + +Don't start from scratch. Generate a working module skeleton (definition, config schema, one command, one listener, `i18n/en+fr.json`, a commented Prisma model, and a test): + +```bash +pnpm new-module my-module "My Module" +``` + +The generated module compiles and its test passes unmodified. Fill the `TODO` description, then enable it via `/modules` on your dev guild. No registration step exists: modules are auto-discovered from `src/modules/` at startup. + +For tests, use the shared helpers from `#lib/testing.js` (`makeTestConfig`, `fakeGuild`, `fakeMessage`, `initTestI18n`) instead of hand-rolled mocks. Note that `vi.mock("#index.js")` and `vi.mock("#lib/database.js")` must still be declared per test file — Vitest hoists them, so no helper can hide them. + ## Module Structure ``` @@ -199,6 +211,20 @@ Keys are looked up in the module's own namespace first, then fall back to core t The guild's locale is configured via the core module's settings (`/config core > locale`). When a locale file doesn't exist for the selected language, the system falls back to English. +## Publishing Checklist + +Before your module reaches production guilds: + +- **Bump `version`** when you add, rename, or change a command — production re-registers guild commands only on a version change (dev re-syncs every boot, so this is easy to miss locally) +- **Set `DEV_GUILD_ID`** in `.env` — without it, dev mode registers no commands at all +- **Prefix command names and `customId`s** with your module id — both are matched first-come-first-served across all modules, and a `:` inside an argument shifts `customId` parsing +- **Guard listeners with `if (!config) return`** — listeners run with `undefined` config outside guilds (DMs) +- **Put `requiresAdmin: true`** on every sensitive command and interaction handler — fail-open otherwise +- **Never put an object in an entity `defaultValue`** (`USER`/`ROLE`/`CHANNEL`/`CATEGORY` defaults must stay unset; ids are stored, objects are hydrated) +- **Declare `ENUM` `options` `as const`** for literal-union typing of `config.get()` +- **Prisma order**: write `models/*.prisma`, then `pnpm prisma:generate`, then `pnpm prisma:migrate` — and keep model names unique across all modules +- **No top-level side effects** — `devOnly` modules are still imported in production; only their registration is skipped + ## Best Practices - **Keep `onLoad` lean** — register artifacts and log, move logic to services diff --git a/docs/site/fr/guide/creating-a-module.md b/docs/site/fr/guide/creating-a-module.md index 9ac81cd5..f8459b53 100644 --- a/docs/site/fr/guide/creating-a-module.md +++ b/docs/site/fr/guide/creating-a-module.md @@ -2,7 +2,17 @@ Un module dans OmniBot est une unité fonctionnelle autonome qui peut être installée et désinstallée par serveur Discord. Chaque module peut contenir des commandes, des écouteurs d'événements, des gestionnaires d'interactions, des services, une configuration et des modèles de base de données. -## Structure d'un module +## Générer un module + +Ne partez pas de zéro. Générez un squelette fonctionnel (définition, schéma config, une commande, un listener, `i18n/en+fr.json`, un modèle Prisma commenté, et un test) : + +```bash +pnpm new-module mon-module "Mon Module" +``` + +Le module généré compile et son test passe sans retouche. Remplissez la description `TODO`, puis activez-le via `/modules` sur votre serveur dev. Aucune étape d'enregistrement : les modules sont auto-découverts depuis `src/modules/` au démarrage. + +Pour les tests, utilisez les helpers partagés de `#lib/testing.js` (`makeTestConfig`, `fakeGuild`, `fakeMessage`, `initTestI18n`) plutôt que des mocks maison. Notez que `vi.mock("#index.js")` et `vi.mock("#lib/database.js")` doivent toujours être déclarés par fichier de test — Vitest les hisse, aucun helper ne peut les masquer. ``` src/modules/mon-module/ @@ -199,6 +209,20 @@ Les clés sont d'abord cherchées dans le namespace du module, puis dans celui d La locale du serveur est configurée via les paramètres du module Cœur (`/config core > locale`). Quand un fichier de locale n'existe pas pour la langue sélectionnée, le système utilise l'anglais par défaut. +## Liste de vérification avant publication + +Avant que votre module n'atteigne des serveurs de production : + +- **Incrémentez `version`** quand vous ajoutez, renommez ou modifiez une commande — la production ne ré-enregistre les commandes que lors d'un changement de version (le dev resynchronise à chaque démarrage, facile à rater en local) +- **Renseignez `DEV_GUILD_ID`** dans `.env` — sans lui, le mode dev n'enregistre aucune commande +- **Préfixez noms de commandes et `customId`** avec votre id de module — les deux sont résolus premier-arrivé-premier-servi entre tous les modules, et un `:` dans un argument décale le parsing des `customId` +- **Gardez `if (!config) return`** dans les listeners — hors serveurs (MP), ils tournent avec une config `undefined` +- **Mettez `requiresAdmin: true`** sur chaque commande et handler sensible — sinon exposé à tout le monde +- **Jamais d'objet en `defaultValue` d'entité** (`USER`/`ROLE`/`CHANNEL`/`CATEGORY` restent non renseignés ; les ids sont stockés, les objets sont réhydratés) +- **Déclarez les `options` des `ENUM` avec `as const`** pour typer `config.get()` en union littérale +- **Ordre Prisma** : écrire `models/*.prisma`, puis `pnpm prisma:generate`, puis `pnpm prisma:migrate` — et gardez des noms de modèles uniques entre modules +- **Pas de side-effect au top-level** — les modules `devOnly` sont quand même importés en production ; seul leur enregistrement est sauté + ## Bonnes pratiques - **Gardez `onLoad` léger** — enregistrez les artefacts et loggez, déplacez la logique dans les services From 7f9198fe0526d9a63c6fcccf43884c9906e05364 Mon Sep 17 00:00:00 2001 From: RedsTom Date: Fri, 2 Oct 2026 16:10:46 +0200 Subject: [PATCH 15/16] fix(core): drop redundant inline admin checks, fix test client mock --- src/core/commands/config.command.ts | 5 ++--- src/core/commands/module.command.ts | 8 +++----- src/core/loaders/command-loader.integration.test.ts | 2 +- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/core/commands/config.command.ts b/src/core/commands/config.command.ts index 356c1c49..316eb93c 100644 --- a/src/core/commands/config.command.ts +++ b/src/core/commands/config.command.ts @@ -8,7 +8,6 @@ import coreModule from "#core/core.module.js"; import configService from "#core/services/config.service.js"; import moduleService from "#core/services/module.service.js"; import { configurationMessage } from "#core/utils/core-messages.js"; -import { requireAdmin } from "#core/utils/require-admin.js"; import { modules } from "#index.js"; import { declareCommand } from "#lib/command.js"; import { Colors } from "#utils/colors.js"; @@ -33,13 +32,13 @@ export default declareCommand({ requiresAdmin: true, async execute(interaction) { + // Admin access is enforced centrally by the command dispatcher via + // `requiresAdmin` (defense in depth: `setDefaultMemberPermissions` above). const coreConfig = await configService.getConfigForModuleIn( coreModule, interaction.guildId! ); - if (!(await requireAdmin(interaction, coreConfig.t))) return; - const moduleId = interaction.options.getString("module", true); const module = [...modules, coreModule].find((m) => m.id === moduleId); diff --git a/src/core/commands/module.command.ts b/src/core/commands/module.command.ts index 3da89048..890ba563 100644 --- a/src/core/commands/module.command.ts +++ b/src/core/commands/module.command.ts @@ -7,7 +7,6 @@ import coreModule from "#core/core.module.js"; import configService from "#core/services/config.service.js"; import moduleService from "#core/services/module.service.js"; import { modulesMessage } from "#core/utils/core-messages.js"; -import { requireAdmin } from "#core/utils/require-admin.js"; import { declareCommand } from "#lib/command.js"; const PERMISSION_ADMINISTRATOR = 0x8; @@ -23,15 +22,14 @@ export default declareCommand({ requiresAdmin: true, async execute(interaction) { - // Checked before deferring: requireAdmin replies, which needs a fresh - // interaction. + // Admin access is enforced centrally by the command dispatcher via + // `requiresAdmin` — checked before deferring (a reply needs a fresh + // interaction). const coreConfig = await configService.getConfigForModuleIn( coreModule, interaction.guildId! ); - if (!(await requireAdmin(interaction, coreConfig.t))) return; - const defer = await interaction.deferReply({ flags: MessageFlags.Ephemeral, }); diff --git a/src/core/loaders/command-loader.integration.test.ts b/src/core/loaders/command-loader.integration.test.ts index 04d689e1..e6760eae 100644 --- a/src/core/loaders/command-loader.integration.test.ts +++ b/src/core/loaders/command-loader.integration.test.ts @@ -5,7 +5,7 @@ import type { Module } from "#lib/module.js"; // Avoid booting the bot / Prisma when importing the command loader's module graph. vi.mock("#index.js", () => ({ modules: [], - client: { user: { id: "app-1" }, token: "token" }, + client: { user: { id: "app-1" }, token: "token", isReady: () => true }, })); vi.mock("#lib/database.js", () => ({ default: {}, Prisma: {} })); From c67a52911315aa3eb1281a9f7e4ee00bb7b0aeea Mon Sep 17 00:00:00 2001 From: RedsTom Date: Fri, 2 Oct 2026 16:37:34 +0200 Subject: [PATCH 16/16] style: fix formatting in unified config.service test --- src/core/services/config.service.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/core/services/config.service.test.ts b/src/core/services/config.service.test.ts index f45ec2e6..989d4d08 100644 --- a/src/core/services/config.service.test.ts +++ b/src/core/services/config.service.test.ts @@ -82,7 +82,10 @@ describe("ConfigService on a guild without stored configuration", () => { }); it("does not fail when another process created the row in the meantime", async () => { - const first = configService.getConfigForModuleIn(coreModule, "racing-guild"); + const first = configService.getConfigForModuleIn( + coreModule, + "racing-guild" + ); rows.set("racing-guild", { core: { locale: "fr" } }); const provider = await first;