diff --git a/CHANGELOG.md b/CHANGELOG.md index cc66282..5ada553 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,21 @@ All notable changes to this project are documented here. ## [Unreleased] +### Changed + +**Command module registry** + +- Slash commands are `CommandModule`s (`src/commands/types.ts`: `command`, + `description`, `operatorOnly`, `allowlisted`, `hidden`, `handle(ctx, deps)`) + listed in menu order by `COMMAND_MODULES` in `src/commands/index.ts`. + `registerCommandHandlers` and `registerCommands` iterate that registry, so + `/help` and the Telegram command menu follow registry order, and the + operator-only and `ALLOWED_CHAT_IDS` gates are applied centrally from each + module's flags. `/contracts` and `/preview` are written in the full module + shape as examples. Replies and message text are unchanged (#327). +- `CONTRIBUTING.md` documents adding a command as a module; + `tests/command-registry.test.mjs` covers order and authorization. + ### Added **Selectable bot language** diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c8072f6..2571d28 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -158,33 +158,47 @@ The REPL context is preloaded with: Commands are self-contained modules under `src/commands/`; `src/bot.ts` only walks the registry, so a new command never needs to touch a shared handler table (and so -rarely conflicts with another in-flight branch): +rarely conflicts with another in-flight branch). The module shape is +`CommandModule` in `src/commands/types.ts`: -1. Create `src/commands/.ts` and export a module from `defineCommand({ … })`: +| Field | Meaning | +| -------------- | ----------------------------------------------------------------------- | +| `command` | Telegram command name, without the slash | +| `description` | English menu line; defaults to its `COMMAND_DESCRIPTIONS` entry | +| `operatorOnly` | Only the configured operator; others are silently ignored | +| `allowlisted` | Only chats in `ALLOWED_CHAT_IDS`; others are silently ignored | +| `hidden` | Registered and usable, but not in the menu or `/help` (e.g. `/export`) | +| `handle` | `async (ctx, deps) => …`, called only after the gates above pass | + +1. Create `src/commands/.ts` and export a module from `createCommandModule({ … })` + (`src/commands/contracts.ts` and `src/commands/preview.ts` are small examples): ```ts - export const widgetCommand = defineCommand({ + export const widgetCommand = createCommandModule({ command: "widget", - chatAllowlist: true, // optional: only chats in ALLOWED_CHAT_IDS - operatorOnly: false, // optional: operator-only, silently ignored otherwise - hidden: false, // optional: registered but not advertised in the menu - handler: async (ctx, deps) => { + description: COMMAND_DESCRIPTIONS.widget, // optional: this is the default + operatorOnly: false, // optional, default false + allowlisted: true, // optional, default false + hidden: false, // optional, default false + handle: async (ctx, deps) => { await ctx.reply(widgetMessage(deps.config), TELEGRAM_OPTIONS); }, }); ``` The handler receives `(ctx, deps)`; `deps` is the same object `createBot` took - (config, poller status, pause/resume, audit, logs). Authorization is applied by - the registrar from `operatorOnly`/`chatAllowlist` — do not repeat it in the - handler. -2. Add one import line to `src/commands/index.ts`, in menu order. + (config, poller status, pause/resume, audit, logs). Authorization is applied + centrally by the registrar from `operatorOnly`/`allowlisted`, with the usual + silent ignore and log line — do not repeat it in the handler. +2. Add the module to `COMMAND_MODULES` in `src/commands/index.ts`, in menu order. + That array's order is the order of the Telegram menu (`setMyCommands`) and `/help`. 3. If the command is advertised, add its description to `COMMAND_DESCRIPTIONS` (both locales) in `src/i18n.ts` and its entry to `COMMAND_DETAILS` in `src/commands/help-detail.ts`. `/help`, the Telegram menu and the detail pages are generated from those tables. -4. Cover the handler in `tests/` like any other command; `tests/command-modules.test.mjs` - checks the registry invariants (unique names, menu parity, central authorization). +4. Cover the handler in `tests/` like any other command. `tests/command-registry.test.mjs` + pins catalogue order and the central authorization gates, and + `tests/command-modules.test.mjs` checks the registry invariants (unique names, menu parity). ## Where things live diff --git a/README.md b/README.md index 10c39a0..e74c9b3 100644 --- a/README.md +++ b/README.md @@ -139,9 +139,11 @@ looks healthy but notifies nobody. command metadata used to register handlers. Without `OPERATOR_TELEGRAM_USER_ID`, the help and menu omit `/audit`, `/pause`, and `/resume`; configured operator commands are labelled "Operator only". Help uses static metadata, so it remains -available when the poller or RPC is unhealthy. Adding a command requires its -metadata and handler in `src/bot.ts`; TypeScript checks that every registry entry -has a handler. This changes no cursor files or deployment settings. +available when the poller or RPC is unhealthy. Each command is a self-contained +module under `src/commands/`, listed in menu order by `COMMAND_MODULES` in +`src/commands/index.ts`; operator-only and `ALLOWED_CHAT_IDS` checks come from the +module's flags (see "Adding a slash command" in `CONTRIBUTING.md`). This changes no +cursor files or deployment settings. Commands from a user other than `OPERATOR_TELEGRAM_USER_ID` receive no control response and cannot mutate poller state — this includes `/audit`, whose report diff --git a/src/bot.ts b/src/bot.ts index f5e09ee..2de897b 100644 --- a/src/bot.ts +++ b/src/bot.ts @@ -273,13 +273,13 @@ function dispatch( ctx: CommandContext, deps: BotDeps, ): Promise { - if (module.operatorOnly === true && !operatorAllowed(ctx, deps.config, module.command)) { + if (module.operatorOnly && !operatorAllowed(ctx, deps.config, module.command)) { return Promise.resolve(); } - if (module.chatAllowlist === true && !chatAllowed(ctx, deps.config, module.command)) { + if (module.allowlisted && !chatAllowed(ctx, deps.config, module.command)) { return Promise.resolve(); } - return module.handler(ctx, deps); + return module.handle(ctx, deps); } /** diff --git a/src/commands/about.ts b/src/commands/about.ts index 4fadc58..2281f2b 100644 --- a/src/commands/about.ts +++ b/src/commands/about.ts @@ -5,7 +5,7 @@ import { escapeMd } from "../notifications/format.js"; import { networkLabel, type BotConfig } from "../config.js"; -import { defineCommand } from "./registry.js"; +import { createCommandModule } from "./registry.js"; import { replyMarkdown } from "./runtime.js"; const REPOSITORY_URL = "https://github.com/example/stellar-bot"; @@ -21,9 +21,9 @@ export function aboutMessage(config: BotConfig, version: string): string { ].join("\n"); } -export const aboutCommand = defineCommand({ +export const aboutCommand = createCommandModule({ command: "about", - handler: async (ctx, deps) => { + handle: async (ctx, deps) => { await replyMarkdown(ctx, aboutMessage(deps.config, deps.version ?? "unknown")); }, }); diff --git a/src/commands/audit.ts b/src/commands/audit.ts index 6a1720b..28ddc9a 100644 --- a/src/commands/audit.ts +++ b/src/commands/audit.ts @@ -12,7 +12,7 @@ import { } from "../audit.js"; import { messagesFor, type Locale } from "../i18n.js"; import { safeErrorMessage } from "../notifications/format.js"; -import { defineCommand } from "./registry.js"; +import { createCommandModule } from "./registry.js"; /** How many recent audit lines `/audit` renders. A chat message is not a file. */ const AUDIT_TAIL = 10; @@ -33,10 +33,10 @@ export function renderAuditForTelegram( return `${header}\n\n${report}\n\n${messagesFor(locale).auditCliHint}`; } -export const auditCommand = defineCommand({ +export const auditCommand = createCommandModule({ command: "audit", operatorOnly: true, - handler: async (ctx, deps) => { + handle: async (ctx, deps) => { const { config } = deps; try { const file = deps.auditFile ?? config.auditFile; diff --git a/src/commands/contracts.ts b/src/commands/contracts.ts index 48b7c16..f4f544b 100644 --- a/src/commands/contracts.ts +++ b/src/commands/contracts.ts @@ -8,8 +8,9 @@ import { escapeMd } from "../notifications/format.js"; import { networkLabel, type BotConfig } from "../config.js"; +import { COMMAND_DESCRIPTIONS } from "../i18n.js"; import { contractExplorerUrl } from "../stellar/client.js"; -import { defineCommand } from "./registry.js"; +import { createCommandModule } from "./registry.js"; import { TELEGRAM_OPTIONS } from "./runtime.js"; export function contractsMessage(config: BotConfig): string { @@ -44,10 +45,13 @@ export function contractsMessage(config: BotConfig): string { return lines.join("\n"); } -export const contractsCommand = defineCommand({ +export const contractsCommand = createCommandModule({ command: "contracts", - chatAllowlist: true, - handler: async (ctx, deps) => { + description: COMMAND_DESCRIPTIONS.contracts, + operatorOnly: false, + // Deployment details: only answered in ALLOWED_CHAT_IDS. + allowlisted: true, + handle: async (ctx, deps) => { await ctx.reply(contractsMessage(deps.config), TELEGRAM_OPTIONS); }, }); diff --git a/src/commands/health.ts b/src/commands/health.ts index 3046cd9..d16ad01 100644 --- a/src/commands/health.ts +++ b/src/commands/health.ts @@ -8,7 +8,7 @@ import { escapeMd } from "../notifications/format.js"; import { formatProvenanceSummary, networkLabel, type BotConfig } from "../config.js"; import { buildHealthReport, chainClockLabel } from "../health.js"; import type { PollerStatus } from "../poller.js"; -import { defineCommand } from "./registry.js"; +import { createCommandModule } from "./registry.js"; import { TELEGRAM_OPTIONS } from "./runtime.js"; function ago(timestamp: number | null, nowMs: number = Date.now()): string { @@ -102,12 +102,12 @@ export function healthMessage( return lines.join("\n"); } -export const healthCommand = defineCommand({ +export const healthCommand = createCommandModule({ // Config-only, so this never fails on account of poller or RPC state — // unlike /status, it has nothing to report failure on. command: "health", - chatAllowlist: true, - handler: async (ctx, deps) => { + allowlisted: true, + handle: async (ctx, deps) => { await ctx.reply(healthMessage(deps.config, deps.status()), TELEGRAM_OPTIONS); }, }); diff --git a/src/commands/help.ts b/src/commands/help.ts index 50028d9..6fe3093 100644 --- a/src/commands/help.ts +++ b/src/commands/help.ts @@ -12,7 +12,7 @@ import { escapeMd } from "../notifications/format.js"; import { messagesFor } from "../i18n.js"; import type { BotConfig } from "../config.js"; import { helpDetailMessage } from "./help-detail.js"; -import { defineCommand, visibleCommandMetadata } from "./registry.js"; +import { createCommandModule, visibleCommandMetadata } from "./registry.js"; import { commandArgument, replyMarkdown } from "./runtime.js"; /** The help overview: title, intro, and one line per advertised command. */ @@ -29,16 +29,16 @@ export function helpMessage(config?: BotConfig): string { ].join("\n"); } -export const startCommand = defineCommand({ +export const startCommand = createCommandModule({ command: "start", - handler: async (ctx, deps) => { + handle: async (ctx, deps) => { await replyMarkdown(ctx, helpMessage(deps.config)); }, }); -export const helpCommand = defineCommand({ +export const helpCommand = createCommandModule({ command: "help", - handler: async (ctx, deps) => { + handle: async (ctx, deps) => { const arg = commandArgument(ctx); await replyMarkdown( ctx, diff --git a/src/commands/index.ts b/src/commands/index.ts index eec2730..377ec23 100644 --- a/src/commands/index.ts +++ b/src/commands/index.ts @@ -1,38 +1,61 @@ /** * The command catalogue — the one place that knows which commands exist. * - * Every module below calls `defineCommand` when it loads, and the import order - * here is the order Telegram's menu and `/help` list them in. Adding a command: + * {@link COMMAND_MODULES} is the ordered list of slash commands; its order is + * the order Telegram's menu and `/help` list them in. Adding a command: * - * 1. create `src/commands/.ts` and export a module from - * `defineCommand({ command, operatorOnly?, chatAllowlist?, hidden?, handler })` - * 2. add one import line here — in menu order — and re-export anything - * `src/bot.ts` or the tests need + * 1. create `src/commands/.ts` exporting a module from + * `createCommandModule({ command, description?, operatorOnly?, allowlisted?, hidden?, handle })` + * (see `CommandModule` in `./types.ts`) + * 2. add it to `COMMAND_MODULES` below — in menu order — and re-export + * anything `src/bot.ts` or the tests need * 3. if the command is advertised, add its description to * `COMMAND_DESCRIPTIONS` (both locales) in `src/i18n.ts` and its entry to * `COMMAND_DETAILS` in `src/commands/help-detail.ts` * - * `src/bot.ts` never needs editing: it registers whatever the registry holds. + * `src/bot.ts` never needs editing: it registers whatever the registry holds + * and applies `operatorOnly` / `allowlisted` centrally. */ -// Registration order = menu order. Keep this list in sync with the menu. -import "./help.js"; // registers /start and /help, in that order -import "./about.js"; -import "./status.js"; -import "./audit.js"; -import "./contracts.js"; -import "./health.js"; -import "./preview.js"; -import "./pause.js"; -import "./resume.js"; -import "./log-export.js"; // hidden: operator log export +import { aboutCommand } from "./about.js"; +import { auditCommand } from "./audit.js"; +import { contractsCommand } from "./contracts.js"; +import { healthCommand } from "./health.js"; +import { helpCommand, startCommand } from "./help.js"; +import { logExportCommand } from "./log-export.js"; +import { pauseCommand } from "./pause.js"; +import { previewCommand } from "./preview.js"; +import { registerCommandModules } from "./registry.js"; +import { resumeCommand } from "./resume.js"; +import { statusCommand } from "./status.js"; +import type { CommandModule } from "./types.js"; + +/** Every slash command, in menu order. */ +export const COMMAND_MODULES: readonly CommandModule[] = Object.freeze([ + startCommand, + helpCommand, + aboutCommand, + statusCommand, + auditCommand, + contractsCommand, + healthCommand, + previewCommand, + pauseCommand, + resumeCommand, + logExportCommand, // hidden: operator log export +]); + +registerCommandModules(COMMAND_MODULES); export { commandMetadata, commandModule, + createCommandModule, defineCommand, + registerCommandModules, registeredCommands, visibleCommandMetadata, + type CommandDefinition, type CommandDeps, type CommandHandler, type CommandMetadata, diff --git a/src/commands/log-export.ts b/src/commands/log-export.ts index 98fc1ac..22fa148 100644 --- a/src/commands/log-export.ts +++ b/src/commands/log-export.ts @@ -7,13 +7,13 @@ */ import { renderLogExport } from "../logger.js"; -import { defineCommand } from "./registry.js"; +import { createCommandModule } from "./registry.js"; -export const logExportCommand = defineCommand({ +export const logExportCommand = createCommandModule({ command: "export", hidden: true, operatorOnly: true, - handler: async (ctx, deps) => { + handle: async (ctx, deps) => { const { config, logs } = deps; if (!logs || logs.capacity() === 0) { await ctx.reply("Log export is disabled (LOG_BUFFER_LINES=0)."); diff --git a/src/commands/pause.ts b/src/commands/pause.ts index 41419f4..151a5f0 100644 --- a/src/commands/pause.ts +++ b/src/commands/pause.ts @@ -11,7 +11,7 @@ import { messagesFor, type Locale } from "../i18n.js"; import type { PollerPauseResult } from "../poller.js"; import { pauseConfirmKeyboard } from "./keyboards.js"; -import { defineCommand } from "./registry.js"; +import { createCommandModule } from "./registry.js"; import { TELEGRAM_OPTIONS } from "./runtime.js"; /** Operator-facing text for the pause confirmation prompt. */ @@ -31,10 +31,10 @@ export function pauseMessage(result: PollerPauseResult, locale?: Locale): string } } -export const pauseCommand = defineCommand({ +export const pauseCommand = createCommandModule({ command: "pause", operatorOnly: true, - handler: async (ctx) => { + handle: async (ctx) => { await ctx.reply(PAUSE_PROMPT, { ...TELEGRAM_OPTIONS, reply_markup: pauseConfirmKeyboard(), diff --git a/src/commands/preview.ts b/src/commands/preview.ts index af2b332..d9fb286 100644 --- a/src/commands/preview.ts +++ b/src/commands/preview.ts @@ -4,14 +4,18 @@ * look without waiting for a live event. */ +import { COMMAND_DESCRIPTIONS } from "../i18n.js"; import { previewMessage } from "../notifications/format.js"; -import { defineCommand } from "./registry.js"; +import { createCommandModule } from "./registry.js"; import { TELEGRAM_OPTIONS, commandArgument } from "./runtime.js"; -export const previewCommand = defineCommand({ +export const previewCommand = createCommandModule({ command: "preview", - chatAllowlist: true, - handler: async (ctx, deps) => { + description: COMMAND_DESCRIPTIONS.preview, + operatorOnly: false, + // Reveals channel formatting settings: only answered in ALLOWED_CHAT_IDS. + allowlisted: true, + handle: async (ctx, deps) => { const target = commandArgument(ctx) || "market"; await ctx.reply(previewMessage(deps.config, target), TELEGRAM_OPTIONS); }, diff --git a/src/commands/registry.ts b/src/commands/registry.ts index f15b789..a485c6c 100644 --- a/src/commands/registry.ts +++ b/src/commands/registry.ts @@ -1,112 +1,83 @@ /** * The slash-command registry. * - * A command is a self-contained module under `src/commands/`: it declares its - * name, who may run it, and its handler (plus, where useful, the message - * renderer that handler uses). Modules register themselves through - * {@link defineCommand} at load time; `src/commands/index.ts` is the single - * place that imports them, which fixes menu order. - * - * `src/bot.ts` only walks the registry, so adding a command no longer means - * editing a shared `handlers` record there. Authorization is declared, not - * repeated: the registrar applies {@link CommandModule.operatorOnly} and - * {@link CommandModule.chatAllowlist} before the handler runs, so a new - * operator command cannot forget its check. + * A command is a self-contained module under `src/commands/` (see + * {@link CommandModule} in `./types.ts`): it declares its name, menu + * description, who may run it, and its handler. `src/commands/index.ts` lists + * the modules in menu order and registers that list here; `src/bot.ts` only + * walks the registry, so adding a command no longer means editing a shared + * `handlers` record there. Authorization is declared, not repeated: the + * registrar applies `operatorOnly` and `allowlisted` before the handler runs. */ -import type { CommandContext, Context } from "grammy"; -import type { UserFromGetMe } from "grammy/types"; - -import type { AuditLog } from "../audit.js"; import type { BotConfig } from "../config.js"; import { COMMAND_DESCRIPTIONS, messagesFor } from "../i18n.js"; -import type { LogCapture } from "../logger.js"; -import type { PollerPauseResult, PollerResumeResult, PollerStatus } from "../poller.js"; -import type { BroadcastStore } from "./broadcast.js"; - -/** Everything a command handler may need, supplied once by `createBot`. */ -export interface CommandDeps { - config: BotConfig; - status: () => PollerStatus; - pause: () => PollerPauseResult; - resume: () => PollerResumeResult; - /** Bot version string, typically read from package.json. */ - version?: string | undefined; - /** Live in-memory audit window; renders immediately even before a flush. */ - audit?: AuditLog | undefined; - /** Where the audit JSONL file lives, for the file-backed report. */ - auditFile?: string | undefined; - /** - * Pre-populated bot info. When provided (e.g. in tests) grammy skips the - * getMe() call so `bot.handleUpdate()` works without a real Telegram token. - */ - botInfo?: UserFromGetMe | undefined; - /** Bounded redacted log ring backing /export. Absent = command disabled. */ - logs?: LogCapture | undefined; - /** In-memory broadcast confirmation store. */ - broadcastStore?: BroadcastStore | undefined; -} +import type { + CommandDefinition, + CommandHandler, + CommandMetadata, + CommandModule, +} from "./types.js"; -/** A slash command handler: grammy's command context plus the bot's deps. */ -export type CommandHandler = ( - ctx: CommandContext, - deps: CommandDeps, -) => Promise; - -/** One self-contained slash command. */ -export interface CommandModule { - /** - * Telegram command name, without the leading slash. Advertised commands must - * also have an entry in `COMMAND_DESCRIPTIONS` (and a `COMMAND_DETAILS` - * entry); hidden operator tools such as `/export` need neither. - */ - readonly command: string; - /** - * Operator-only commands answer only the configured operator and are hidden - * from the menu when no operator is configured. Non-operators are ignored - * silently: an error reply would confirm the command exists. - */ - readonly operatorOnly?: boolean; - /** - * Restricts the command to chats in `ALLOWED_CHAT_IDS`. Restricted commands - * are ignored silently for the same reason as operator commands. - */ - readonly chatAllowlist?: boolean; - /** - * Registered and usable, but not advertised in the Telegram command menu or - * `/help` (e.g. `/export`, an operator recovery tool). - */ - readonly hidden?: boolean; - readonly handler: CommandHandler; -} - -/** Menu and `/help` metadata for a registered command. */ -export interface CommandMetadata { - readonly command: string; - readonly description: string; - readonly operatorOnly: boolean; -} +export type { + CommandDefinition, + CommandDeps, + CommandHandler, + CommandMetadata, + CommandModule, +} from "./types.js"; const registered = new Map(); +const normalized = new WeakSet(); /** * The English description registered with Telegram's command menu. A missing - * entry is a boot-time programming error, not a silently blank menu line. + * entry for an advertised command is a boot-time programming error, not a + * silently blank menu line. */ -function descriptionFor(command: string): string { +function defaultDescription(command: string, hidden: boolean): string { const description = (COMMAND_DESCRIPTIONS as Readonly>)[command]; - if (description === undefined) { - throw new Error(`no COMMAND_DESCRIPTIONS entry for /${command}`); + if (description !== undefined) return description; + if (hidden) return ""; + throw new Error(`no COMMAND_DESCRIPTIONS entry for /${command}`); +} + +/** + * Build a {@link CommandModule} from what a command file writes: fills in the + * flag defaults and the menu description. Does not register anything — the + * catalogue in `src/commands/index.ts` decides what is registered, and in + * which order. + */ +export function createCommandModule(definition: CommandDefinition | CommandModule): CommandModule { + if (normalized.has(definition)) return definition as CommandModule; + + const handle: CommandHandler | undefined = definition.handle ?? definition.handler; + if (typeof handle !== "function") { + throw new Error(`/${definition.command} has no handler`); } - return description; + const hidden = definition.hidden === true; + const allowlisted = definition.allowlisted === true || definition.chatAllowlist === true; + const module: CommandModule = { + command: definition.command, + description: definition.description ?? defaultDescription(definition.command, hidden), + operatorOnly: definition.operatorOnly === true, + allowlisted, + hidden, + handle, + handler: handle, + chatAllowlist: allowlisted, + }; + normalized.add(module); + return module; } /** - * Register a command module. Called at module load from each file in this - * directory; importing a command module twice is harmless, registering two - * modules for the same command is a programming error and throws at boot. + * Register a command module. Registering the same module twice is harmless; + * registering two different modules for one command is a programming error + * and throws at boot. */ -export function defineCommand(module: CommandModule): CommandModule { +export function defineCommand(definition: CommandDefinition | CommandModule): CommandModule { + const module = createCommandModule(definition); const existing = registered.get(module.command); if (existing !== undefined && existing !== module) { throw new Error(`duplicate command registration: /${module.command}`); @@ -115,6 +86,13 @@ export function defineCommand(module: CommandModule): CommandModule { return module; } +/** Register an ordered list of modules; list order becomes menu order. */ +export function registerCommandModules( + modules: readonly (CommandDefinition | CommandModule)[], +): readonly CommandModule[] { + return modules.map((module) => defineCommand(module)); +} + /** Every registered command, in registration (menu) order. */ export function registeredCommands(): readonly CommandModule[] { return [...registered.values()]; @@ -132,11 +110,11 @@ export function commandModule(command: string): CommandModule | undefined { */ export function commandMetadata(): CommandMetadata[] { return registeredCommands() - .filter((module) => module.hidden !== true) + .filter((module) => !module.hidden) .map((module) => ({ command: module.command, - description: descriptionFor(module.command), - operatorOnly: module.operatorOnly === true, + description: module.description, + operatorOnly: module.operatorOnly, })); } @@ -149,13 +127,12 @@ export function visibleCommandMetadata(config?: BotConfig): CommandMetadata[] { const descriptions = messagesFor(config?.locale).commandDescriptions; const operatorConfigured = config === undefined || config.operatorTelegramUserId !== null; return registeredCommands() - .filter((module) => module.hidden !== true) - .filter((module) => operatorConfigured || module.operatorOnly !== true) + .filter((module) => !module.hidden) + .filter((module) => operatorConfigured || !module.operatorOnly) .map((module) => ({ command: module.command, description: - (descriptions as Readonly>)[module.command] ?? - descriptionFor(module.command), - operatorOnly: module.operatorOnly === true, + (descriptions as Readonly>)[module.command] ?? module.description, + operatorOnly: module.operatorOnly, })); } diff --git a/src/commands/resume.ts b/src/commands/resume.ts index ebf7607..af0c536 100644 --- a/src/commands/resume.ts +++ b/src/commands/resume.ts @@ -5,7 +5,7 @@ import { messagesFor, type Locale } from "../i18n.js"; import type { PollerResumeResult } from "../poller.js"; -import { defineCommand } from "./registry.js"; +import { createCommandModule } from "./registry.js"; import { TELEGRAM_OPTIONS } from "./runtime.js"; export function resumeMessage(result: PollerResumeResult, locale?: Locale): string { @@ -20,10 +20,10 @@ export function resumeMessage(result: PollerResumeResult, locale?: Locale): stri } } -export const resumeCommand = defineCommand({ +export const resumeCommand = createCommandModule({ command: "resume", operatorOnly: true, - handler: async (ctx, deps) => { + handle: async (ctx, deps) => { const resume = deps.resume ?? (() => "stopped" as const); await ctx.reply(resumeMessage(resume(), deps.config.locale), TELEGRAM_OPTIONS); }, diff --git a/src/commands/status.ts b/src/commands/status.ts index 1b495fe..4f4b893 100644 --- a/src/commands/status.ts +++ b/src/commands/status.ts @@ -14,7 +14,7 @@ import { networkLabel, type BotConfig } from "../config.js"; import { chainClockLabel } from "../health.js"; import { formatFeatureFlags } from "../notifications/featureFlags.js"; import type { PollerStatus } from "../poller.js"; -import { defineCommand } from "./registry.js"; +import { createCommandModule } from "./registry.js"; import { TELEGRAM_OPTIONS } from "./runtime.js"; function ago(timestamp: number | null, nowMs: number = Date.now()): string { @@ -149,10 +149,10 @@ export function lastEventMessage(config: BotConfig, status: PollerStatus): strin return lines.join("\n").trimEnd(); } -export const statusCommand = defineCommand({ +export const statusCommand = createCommandModule({ command: "status", - chatAllowlist: true, - handler: async (ctx, deps) => { + allowlisted: true, + handle: async (ctx, deps) => { const startedAt = performance.now(); let sent = false; try { diff --git a/src/commands/types.ts b/src/commands/types.ts new file mode 100644 index 0000000..011bbd2 --- /dev/null +++ b/src/commands/types.ts @@ -0,0 +1,121 @@ +/** + * The shape of a slash-command module. + * + * Every command under `src/commands/` exports one {@link CommandModule}. The + * module carries everything the bot needs to advertise and run it — its name, + * its menu description, who may run it, and its handler — so `src/bot.ts` + * only iterates the ordered catalogue in `src/commands/index.ts` and never + * needs a per-command edit. + * + * Authorization is data, not code: the registrar reads `operatorOnly` and + * `allowlisted` and applies the shared gates before `handle` runs, so a new + * command cannot forget its check. + */ + +import type { CommandContext, Context } from "grammy"; +import type { UserFromGetMe } from "grammy/types"; + +import type { AuditLog } from "../audit.js"; +import type { BotConfig } from "../config.js"; +import type { LogSampler } from "../log-sampler.js"; +import type { LogCapture } from "../logger.js"; +import type { PollerPauseResult, PollerResumeResult, PollerStatus } from "../poller.js"; +import type { BroadcastStore } from "./broadcast.js"; +import type { CommandRateLimiter } from "./rate-limit.js"; + +/** Everything a command handler may need, supplied once by `createBot`. */ +export interface CommandDeps { + config: BotConfig; + status: () => PollerStatus; + pause: () => PollerPauseResult; + resume: () => PollerResumeResult; + /** Bot version string, typically read from package.json. */ + version?: string | undefined; + /** Live in-memory audit window; renders immediately even before a flush. */ + audit?: AuditLog | undefined; + /** Where the audit JSONL file lives, for the file-backed report. */ + auditFile?: string | undefined; + /** + * Pre-populated bot info. When provided (e.g. in tests) grammy skips the + * getMe() call so `bot.handleUpdate()` works without a real Telegram token. + */ + botInfo?: UserFromGetMe | undefined; + /** Bounded redacted log ring backing /export. Absent = command disabled. */ + logs?: LogCapture | undefined; + /** In-memory broadcast confirmation store. */ + broadcastStore?: BroadcastStore | undefined; + /** Per-chat command rate limiter. Defaults to a fresh limiter per bot. */ + rateLimiter?: CommandRateLimiter | undefined; + /** Sampler for the "rate limit exceeded" warning. */ + rateLimitSampler?: LogSampler | undefined; + /** Injected clock (epoch ms) for the default rate limiter and sampler. */ + now?: (() => number) | undefined; +} + +/** A slash command handler: grammy's command context plus the bot's deps. */ +export type CommandHandler = ( + ctx: CommandContext, + deps: CommandDeps, +) => Promise; + +/** One self-contained slash command, as the registry stores it. */ +export interface CommandModule { + /** + * Telegram command name, without the leading slash. Advertised commands also + * need a `COMMAND_DETAILS` entry in `src/commands/help-detail.ts`. + */ + readonly command: string; + /** + * English menu description (Telegram `setMyCommands` and `/help`). A locale's + * `commandDescriptions` entry overrides it at render time. Empty for hidden + * commands, which are never advertised. + */ + readonly description: string; + /** + * Operator-only commands answer only the configured operator and are hidden + * from the menu when no operator is configured. Non-operators are ignored + * silently: an error reply would confirm the command exists. + */ + readonly operatorOnly: boolean; + /** + * Restricts the command to chats in `ALLOWED_CHAT_IDS`. Restricted commands + * are ignored silently for the same reason as operator commands. + */ + readonly allowlisted: boolean; + /** + * Registered and usable, but not advertised in the Telegram command menu or + * `/help` (e.g. `/export`, an operator recovery tool). + */ + readonly hidden: boolean; + /** Runs the command. Called only after the declared gates pass. */ + handle(ctx: CommandContext, deps: CommandDeps): Promise; + /** @deprecated Alias of {@link CommandModule.handle}, kept for existing callers. */ + readonly handler: CommandHandler; + /** @deprecated Alias of {@link CommandModule.allowlisted}, kept for existing callers. */ + readonly chatAllowlist: boolean; +} + +/** + * What a command file writes. Flags default to `false`; `description` defaults + * to the command's `COMMAND_DESCRIPTIONS` entry. `handler` and `chatAllowlist` + * are accepted as older spellings of `handle` and `allowlisted`. + */ +export interface CommandDefinition { + readonly command: string; + readonly description?: string; + readonly operatorOnly?: boolean; + readonly allowlisted?: boolean; + readonly hidden?: boolean; + readonly handle?: CommandHandler; + /** @deprecated Use `handle`. */ + readonly handler?: CommandHandler; + /** @deprecated Use `allowlisted`. */ + readonly chatAllowlist?: boolean; +} + +/** Menu and `/help` metadata for a registered command. */ +export interface CommandMetadata { + readonly command: string; + readonly description: string; + readonly operatorOnly: boolean; +} diff --git a/tests/command-registry.test.mjs b/tests/command-registry.test.mjs new file mode 100644 index 0000000..2a681a0 --- /dev/null +++ b/tests/command-registry.test.mjs @@ -0,0 +1,370 @@ +/** + * The command module registry (issue #327). + * + * Commands are self-contained `CommandModule`s listed in menu order by + * `COMMAND_MODULES` (`src/commands/index.ts`). These tests register the + * modules on a fake bot and pin: + * - the module shape and the catalogue's names and order + * - that handlers, `/help` and `setMyCommands` all follow registry order + * - that `operatorOnly` / `allowlisted` are enforced centrally from module + * flags, with the same silent-ignore behaviour and log lines as before + * - that `/contracts` and `/preview` — the first modules moved — reply + * exactly as their message builders render + */ + +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + contractsMessage, + helpMessage, + previewMessage, + registerCommandHandlers, + registerCommands, +} from "../dist/bot.js"; +import { + COMMAND_MODULES, + contractsCommand, + createCommandModule, + defineCommand, + previewCommand, + registeredCommands, +} from "../dist/commands/index.js"; +import { COMMAND_DESCRIPTIONS } from "../dist/i18n.js"; + +const OPTIONS = { + parse_mode: "MarkdownV2", + link_preview_options: { is_disabled: true }, +}; + +const REGISTRY_ORDER = [ + "start", + "help", + "about", + "status", + "audit", + "contracts", + "health", + "preview", + "pause", + "resume", + "export", +]; +const ADVERTISED = REGISTRY_ORDER.filter((command) => command !== "export"); +const OPERATOR_ONLY = ["audit", "pause", "resume", "export"]; +const ALLOWLISTED = ["status", "contracts", "health", "preview"]; + +const OPERATOR_ID = 42; +const STRANGER_ID = 99; +const ALLOWED_CHAT = -100111; +const OTHER_CHAT = -100222; + +function baseConfig(overrides = {}) { + return { + marketContractId: "C" + "A".repeat(55), + squadContractId: "C" + "B".repeat(55), + rpcUrl: "https://example.invalid/rpc", + horizonUrl: "https://example.invalid/horizon", + networkPassphrase: "Test SDF Network ; September 2015", + explorerBaseUrl: "https://example.invalid/explorer", + botToken: "123456789:TEST-ONLY-TOKEN-NEVER-USE", + chatId: String(ALLOWED_CHAT), + allowedChatIds: [String(ALLOWED_CHAT)], + operatorTelegramUserId: String(OPERATOR_ID), + pollIntervalMs: 30_000, + startLookbackLedgers: 60, + cursorFile: "/tmp/unused-cursor.json", + maxNotificationsPerCycle: 20, + healthHost: "127.0.0.1", + healthPort: 0, + healthStaleMs: 90_000, + channelPreviewMode: false, + ...overrides, + }; +} + +function baseStatus() { + return { + running: true, + paused: false, + startedAt: 1, + cycles: 0, + lastPollAt: null, + lastSuccessAt: null, + latestLedger: null, + oldestLedger: null, + notificationsSent: 0, + notificationsFailed: 0, + eventsSkipped: 0, + consecutiveFailures: 0, + lastError: null, + targets: [], + pagesScanned: 0, + emptyPages: 0, + lastCyclePages: 0, + lastCycleEmptyPages: 0, + cursorRewinds: 0, + restartGaps: 0, + }; +} + +/** Never rate-limits, so authorization is the only gate under test. */ +const OPEN_LIMITER = { consume: () => true, now: () => Date.now() }; + +function deps(overrides = {}) { + return { + config: baseConfig(), + status: () => baseStatus(), + pause: () => "paused", + resume: () => "resumed", + rateLimiter: OPEN_LIMITER, + ...overrides, + }; +} + +/** A bot-shaped recorder: command registrations in order, plus the menu. */ +function fakeBot() { + const handlers = new Map(); + const menus = []; + return { + handlers, + menus, + bot: { + command: (name, handler) => handlers.set(name, handler), + api: { setMyCommands: async (commands) => menus.push(commands) }, + }, + }; +} + +function context({ userId = OPERATOR_ID, chatId = ALLOWED_CHAT, updateId = 7, text } = {}) { + const replies = []; + return { + replies, + ctx: { + from: { id: userId }, + chat: { id: chatId, type: "group" }, + update: { update_id: updateId }, + message: text === undefined ? undefined : { text }, + reply: async (...args) => replies.push(args), + }, + }; +} + +async function withWarnings(fn) { + const warnings = []; + const original = console.warn; + console.warn = (message) => warnings.push(String(message)); + try { + await fn(); + } finally { + console.warn = original; + } + return warnings; +} + +// ── Module shape and catalogue order ───────────────────────────────────────── + +test("COMMAND_MODULES lists every command module in menu order", () => { + assert.deepEqual( + COMMAND_MODULES.map(({ command }) => command), + REGISTRY_ORDER, + ); + assert.ok(Object.isFrozen(COMMAND_MODULES), "the catalogue is not mutable at runtime"); +}); + +test("every module has the CommandModule shape", () => { + for (const module of COMMAND_MODULES) { + assert.equal(typeof module.command, "string", `/${module.command} command`); + assert.equal(typeof module.description, "string", `/${module.command} description`); + assert.equal(typeof module.operatorOnly, "boolean", `/${module.command} operatorOnly`); + assert.equal(typeof module.allowlisted, "boolean", `/${module.command} allowlisted`); + assert.equal(typeof module.hidden, "boolean", `/${module.command} hidden`); + assert.equal(typeof module.handle, "function", `/${module.command} handle`); + if (!module.hidden) { + assert.equal(module.description, COMMAND_DESCRIPTIONS[module.command]); + } + } + assert.deepEqual( + COMMAND_MODULES.filter(({ operatorOnly }) => operatorOnly).map(({ command }) => command), + OPERATOR_ONLY, + ); + assert.deepEqual( + COMMAND_MODULES.filter(({ allowlisted }) => allowlisted).map(({ command }) => command), + ALLOWLISTED, + ); +}); + +test("/contracts and /preview are self-contained allowlisted modules", () => { + for (const [module, command] of [ + [contractsCommand, "contracts"], + [previewCommand, "preview"], + ]) { + assert.equal(module.command, command); + assert.equal(module.description, COMMAND_DESCRIPTIONS[command]); + assert.equal(module.operatorOnly, false); + assert.equal(module.allowlisted, true); + assert.equal(module.hidden, false); + assert.ok(COMMAND_MODULES.includes(module), `/${command} is in the catalogue`); + } +}); + +test("the registry holds the catalogue modules themselves, in catalogue order", () => { + const registered = registeredCommands().slice(0, COMMAND_MODULES.length); + assert.equal(registered.length, COMMAND_MODULES.length); + registered.forEach((module, index) => assert.equal(module, COMMAND_MODULES[index])); +}); + +// ── Registration follows registry order ────────────────────────────────────── + +test("registerCommandHandlers registers module commands in registry order", () => { + const { handlers, bot } = fakeBot(); + registerCommandHandlers(bot, deps()); + assert.deepEqual( + [...handlers.keys()].slice(0, REGISTRY_ORDER.length), + REGISTRY_ORDER, + ); +}); + +for (const operatorTelegramUserId of [String(OPERATOR_ID), null]) { + test(`setMyCommands and /help list module commands in registry order (operator=${operatorTelegramUserId})`, async () => { + const config = baseConfig({ operatorTelegramUserId }); + const { handlers, menus, bot } = fakeBot(); + registerCommandHandlers(bot, deps({ config })); + await registerCommands(bot, config); + + const expected = + operatorTelegramUserId === null + ? ADVERTISED.filter((command) => !OPERATOR_ONLY.includes(command)) + : ADVERTISED; + assert.equal(menus.length, 1); + assert.deepEqual(menus[0].map(({ command }) => command), expected); + for (const { command, description } of menus[0]) { + assert.equal(description, COMMAND_DESCRIPTIONS[command]); + assert.ok(handlers.has(command), `/${command} has a handler`); + } + + const helpLines = helpMessage(config) + .split("\n") + .filter((line) => line.startsWith("/")) + .map((line) => line.split(" ")[0].slice(1)); + assert.deepEqual(helpLines, expected); + }); +} + +// ── Central authorization from module flags ────────────────────────────────── + +for (const command of OPERATOR_ONLY) { + test(`/${command} (operatorOnly) ignores non-operators silently and logs the attempt`, async () => { + const { handlers, bot } = fakeBot(); + registerCommandHandlers(bot, deps()); + const stranger = context({ userId: STRANGER_ID, updateId: 7 }); + const warnings = await withWarnings(() => handlers.get(command)(stranger.ctx)); + assert.deepEqual(stranger.replies, []); + assert.deepEqual(warnings, [`[bot] ignored unauthorized /${command} on update 7`]); + }); +} + +for (const command of ALLOWLISTED) { + test(`/${command} (allowlisted) ignores chats outside ALLOWED_CHAT_IDS silently and logs it`, async () => { + const { handlers, bot } = fakeBot(); + registerCommandHandlers(bot, deps()); + const outsider = context({ chatId: OTHER_CHAT }); + const warnings = await withWarnings(() => handlers.get(command)(outsider.ctx)); + assert.deepEqual(outsider.replies, []); + assert.deepEqual(warnings, [ + `[bot] /${command} denied for chat ${OTHER_CHAT} (not in ALLOWED_CHAT_IDS)`, + ]); + }); +} + +test("public commands answer anyone, anywhere", async () => { + const { handlers, bot } = fakeBot(); + registerCommandHandlers(bot, deps()); + for (const command of ["start", "help", "about"]) { + const stranger = context({ userId: STRANGER_ID, chatId: OTHER_CHAT }); + await handlers.get(command)(stranger.ctx); + assert.equal(stranger.replies.length >= 1, true, `/${command} replied`); + } +}); + +test("the operator passes operatorOnly gates", async () => { + const { handlers, bot } = fakeBot(); + let resumed = 0; + registerCommandHandlers(bot, deps({ resume: () => { resumed += 1; return "resumed"; } })); + const operator = context({ userId: OPERATOR_ID }); + await handlers.get("resume")(operator.ctx); + assert.equal(resumed, 1); + assert.equal(operator.replies.length, 1); +}); + +test("flags on a new module are enforced without any code in the module", async () => { + const calls = []; + defineCommand( + createCommandModule({ + command: "test-registry-gated", + hidden: true, + operatorOnly: true, + allowlisted: true, + handle: async (ctx) => { + calls.push(ctx.from.id); + }, + }), + ); + const { handlers, bot } = fakeBot(); + registerCommandHandlers(bot, deps()); + const handler = handlers.get("test-registry-gated"); + assert.equal(typeof handler, "function", "a defined module is registered on the bot"); + + await withWarnings(async () => { + await handler(context({ userId: STRANGER_ID, chatId: ALLOWED_CHAT }).ctx); + await handler(context({ userId: OPERATOR_ID, chatId: OTHER_CHAT }).ctx); + }); + assert.deepEqual(calls, [], "both gates apply"); + + await handler(context({ userId: OPERATOR_ID, chatId: ALLOWED_CHAT }).ctx); + assert.deepEqual(calls, [OPERATOR_ID]); +}); + +// ── Moved commands reply exactly as before ─────────────────────────────────── + +test("/contracts replies with exactly the contracts message", async () => { + const { handlers, bot } = fakeBot(); + const d = deps(); + registerCommandHandlers(bot, d); + const { ctx, replies } = context(); + await handlers.get("contracts")(ctx); + assert.deepEqual(replies, [[contractsMessage(d.config), OPTIONS]]); +}); + +test("/preview replies with the market sample by default and honours its argument", async () => { + const { handlers, bot } = fakeBot(); + const d = deps(); + registerCommandHandlers(bot, d); + + const bare = context({ text: "/preview" }); + await handlers.get("preview")(bare.ctx); + assert.deepEqual(bare.replies, [[previewMessage(d.config, "market"), OPTIONS]]); + + const squad = context({ text: "/preview squad" }); + await handlers.get("preview")(squad.ctx); + assert.deepEqual(squad.replies, [[previewMessage(d.config, "squad"), OPTIONS]]); +}); + +// ── createCommandModule ────────────────────────────────────────────────────── + +test("createCommandModule fills flag defaults and the menu description", () => { + const module = createCommandModule({ command: "contracts", handle: async () => undefined }); + assert.equal(module.description, COMMAND_DESCRIPTIONS.contracts); + assert.equal(module.operatorOnly, false); + assert.equal(module.allowlisted, false); + assert.equal(module.hidden, false); + assert.equal(createCommandModule(module), module, "normalising a module is idempotent"); +}); + +test("createCommandModule rejects a module without a handler or an advertised description", () => { + assert.throws(() => createCommandModule({ command: "test-no-handler" }), /no handler/); + assert.throws( + () => createCommandModule({ command: "test-no-description", handle: async () => undefined }), + /no COMMAND_DESCRIPTIONS entry for \/test-no-description/, + ); +}); diff --git a/tests/rollback.test.mjs b/tests/rollback.test.mjs index 223ab98..6321ab6 100644 --- a/tests/rollback.test.mjs +++ b/tests/rollback.test.mjs @@ -321,7 +321,7 @@ test("poller treats a missing cursor file as a cold start (no crash)", async () assert.ok(status.cycles >= 1, "at least one cycle should have run"); assert.ok(!status.lastError, "should have no errors on a clean RPC stub"); - await rm(dir, { recursive: true }); + await rm(dir, { recursive: true, force: true, maxRetries: 5, retryDelay: 50 }); }); test("poller treats a corrupt cursor file as a cold start (no crash)", async () => { @@ -345,7 +345,7 @@ test("poller treats a corrupt cursor file as a cold start (no crash)", async () const status = poller.status(); assert.ok(status.cycles >= 1, "poller should continue after corrupt cursor"); - await rm(dir, { recursive: true }); + await rm(dir, { recursive: true, force: true, maxRetries: 5, retryDelay: 50 }); }); test("poller resumes from a valid v1 cursor file written by the previous release", async () => { @@ -393,7 +393,7 @@ test("poller resumes from a valid v1 cursor file written by the previous release assert.equal(marketCursor, cursor, "market should resume from saved cursor"); assert.equal(squadCursor, cursor, "squad should resume from saved cursor"); - await rm(dir, { recursive: true }); + await rm(dir, { recursive: true, force: true, maxRetries: 5, retryDelay: 50 }); }); test("cursor file written after a cycle is valid JSON with a version field", async () => { @@ -418,7 +418,7 @@ test("cursor file written after a cycle is valid JSON with a version field", asy assert.ok(parsed.targets?.market, "market target must be persisted"); assert.ok(parsed.targets?.squad, "squad target must be persisted"); - await rm(dir, { recursive: true }); + await rm(dir, { recursive: true, force: true, maxRetries: 5, retryDelay: 50 }); }); // ═══════════════════════════════════════════════════════════════════════════════ @@ -457,7 +457,7 @@ test("an RPC failure in one cycle does not stop the poller", async () => { assert.ok(status.cycles >= 2, `expected ≥2 cycles after RPC recovery, got ${status.cycles}`); - await rm(dir, { recursive: true }); + await rm(dir, { recursive: true, force: true, maxRetries: 5, retryDelay: 50 }); }); test("RPC failure leaves the cursor unchanged so the next cycle resumes correctly", async () => { @@ -494,7 +494,7 @@ test("RPC failure leaves the cursor unchanged so the next cycle resumes correctl "RPC failure must not overwrite the cursor with null or a stale value"); } - await rm(dir, { recursive: true }); + await rm(dir, { recursive: true, force: true, maxRetries: 5, retryDelay: 50 }); }); // ═══════════════════════════════════════════════════════════════════════════════ @@ -527,7 +527,7 @@ test("a Telegram send failure does not stop the poller or prevent cursor advance // notificationsFailed is only incremented when an event was actually sent, // so it may be 0 if the stub emits no events — that is acceptable. - await rm(dir, { recursive: true }); + await rm(dir, { recursive: true, force: true, maxRetries: 5, retryDelay: 50 }); }); // ═══════════════════════════════════════════════════════════════════════════════ @@ -596,7 +596,7 @@ test("cursor file schema is backward-compatible: extra unknown keys are ignored" "poller must not throw when reading a cursor file with extra fields", ); - await rm(dir, { recursive: true }); + await rm(dir, { recursive: true, force: true, maxRetries: 5, retryDelay: 50 }); }); test("cursor file written by the bot is valid JSON that JSON.parse accepts without loss", async () => { @@ -619,5 +619,5 @@ test("cursor file written by the bot is valid JSON that JSON.parse accepts witho assert.doesNotThrow(() => JSON.parse(raw), "cursor file must be valid JSON with no BigInt or undefined values"); - await rm(dir, { recursive: true }); + await rm(dir, { recursive: true, force: true, maxRetries: 5, retryDelay: 50 }); });