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/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/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/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/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/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/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/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/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 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/package.json b/package.json index 577ee338..09461cd2 100644 --- a/package.json +++ b/package.json @@ -16,13 +16,14 @@ }, "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", "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/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/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); +}); diff --git a/src/core/commands/config.command.ts b/src/core/commands/config.command.ts index 37bcace0..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"; @@ -29,14 +28,17 @@ export default declareCommand({ .setRequired(true) .setAutocomplete(true) ), + + 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 ee37b4de..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; @@ -20,16 +19,17 @@ export default declareCommand({ .setDefaultMemberPermissions(PERMISSION_ADMINISTRATOR) .setContexts([InteractionContextType.Guild]), + 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/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..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.", @@ -87,5 +88,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..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.", @@ -87,5 +88,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..e5a20fd9 --- /dev/null +++ b/src/core/listeners/guild-create.listener.test.ts @@ -0,0 +1,180 @@ +import { MessageFlags } from "discord.js"; +import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { + fakeChannel, + fakeGuild, + initTestI18n, + silenceLogs, +} from "#lib/testing.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"); + +const welcomeBundle = { + "guild.welcome.title": "title", + "guild.welcome.body": "body", + "guild.welcome.hint": "hint", + "guild.welcome.dmPrefix": "prefix ", +}; + +beforeAll(async () => { + await initTestI18n("core", { en: welcomeBundle, fr: welcomeBundle }); + silenceLogs(); +}); + +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/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/core/loaders/command-loader.integration.test.ts b/src/core/loaders/command-loader.integration.test.ts index 60988163..e6760eae 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", isReady: () => true }, +})); 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, 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 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/core/services/config.service.test.ts b/src/core/services/config.service.test.ts index d73b032e..989d4d08 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,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(module, "racing-guild"); + const first = configService.getConfigForModuleIn( + coreModule, + "racing-guild" + ); rows.set("racing-guild", { core: { locale: "fr" } }); const provider = await first; @@ -84,3 +93,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; } } diff --git a/src/core/services/module.service.test.ts b/src/core/services/module.service.test.ts index 90d82149..68d2d64b 100644 --- a/src/core/services/module.service.test.ts +++ b/src/core/services/module.service.test.ts @@ -1,27 +1,28 @@ -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 // 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: {}, })); 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; 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 +33,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 +81,7 @@ describe("reconcileActivatedVersions", () => { await moduleService.reconcileActivatedVersions(module); - expect(update).not.toHaveBeenCalled(); + expect(upsert).not.toHaveBeenCalled(); }); }); @@ -93,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 1e58ae8b..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; } @@ -173,14 +177,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, 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/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. * 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/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: { 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/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}`); } 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, + }; +} 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})` - ); - }, }); 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], }, },