feat(teams): the slash-command seam, and the first command through it

Phase 7 of TEAMS.md. `api.registerSlashCommands` stops throwing: a module registers a command's DEFINITION and its HANDLER together, the bot pulls the definitions over the internal listener and runs none of our code, and the handler executes here — forced by the bot container having no `modules` volume, and the right boundary anyway. Registration validates what Discord would reject as a batch (names, description lengths, the four option types, required-before-optional), because the bot registers the whole set in one PUT and a single bad entry costs every command including the bot's own. Commands are not namespaced under their owner — there is no dot in Discord's name grammar — so collisions are first-come with the holder named. The dispatcher is the access boundary: `linked` has no Discord equivalent, so the platform-side permission default can only ever be advertising. It resolves the actor by `auth_providers.kind` rather than the id slug, treats a banned account as unlinked, bounds a handler under the bot's own timeout, and keeps `ok` outside the envelope so a handler cannot forge it. Liveness is asked at both the pull and the dispatch. The registries have no removal path, so a module an operator disables at runtime would otherwise keep a live handler behind a command Discord still advertises. Co-Authored-By: Claude <noreply@anthropic.com>
2026-08-18 18:53:34 -05:00
parent b1d3b87cd6
commit cecd72915f
21 changed files with 1459 additions and 27 deletions
--- a/server/test/slashCommands.test.js
+++ b/server/test/slashCommands.test.js
@@ -0,0 +1,261 @@
+// ── Slash commands: registration, the actor, and the dispatcher ────────────
+//
+// TEAMS.md §7.1, phase 7. Three things are worth a test here and none of them
+// can be exercised by hand without a Discord guild:
+//
+//   1. **Registration rejects a definition Discord would reject as a batch.** The
+//      bot registers the whole set in one PUT, so one bad option type costs every
+//      command — including the bot's own. That has to fail at `register()`.
+//   2. **The dispatcher is the access boundary**, not the Discord-side permission
+//      default, which cannot express "has a linked account" at all.
+//   3. **A handler's failure is its own.** A throw, a hang, and a disabled owner
+//      each produce an answer the bot can render, and never an exception.
+process.env.DB_HOST = '127.0.0.1'
+process.env.DB_PORT = '59999'
+
+const { test, beforeEach, after } = require('node:test')
+const assert = require('node:assert/strict')
+
+const registries = require('../src/modules/registries')
+const slash = require('../src/utils/slashCommands')
+const loader = require('../src/modules/loader')
+const authProviders = require('../src/model/authProviders/authProviders.model')
+const userIdentities = require('../src/model/userIdentities/userIdentities.model')
+const users = require('../src/model/users/users.model')
+const db = require('../src/utils/db')
+
+after(() => db.close())
+
+const originals = {
+  isLoaded: loader.isLoaded,
+  list: loader.list,
+  version: loader.version,
+  providerList: authProviders.list,
+  findIdentity: userIdentities.findByProviderSubject,
+  getUser: users.getById,
+}
+
+// The loader is stubbed rather than driven: `load()` needs a modules directory
+// on disk, and every property below is about what the registry and the
+// dispatcher do with a state, not about how the state was reached.
+function loaded(state = 'started', id = 'uo') {
+  loader.isLoaded = () => true
+  loader.list = () => [{ id, state }]
+  loader.version = () => 42
+}
+
+beforeEach(() => {
+  registries._reset()
+  Object.assign(loader, { isLoaded: originals.isLoaded, list: originals.list, version: originals.version })
+  Object.assign(authProviders, { list: originals.providerList })
+  Object.assign(userIdentities, { findByProviderSubject: originals.findIdentity })
+  Object.assign(users, { getById: originals.getUser })
+})
+
+const cmd = (over = {}) => ({
+  name: 'guild',
+  description: 'Show a guild',
+  handler: async () => ({ text: 'ok' }),
+  ...over,
+})
+
+/** Register a batch as `owner`; returns the error message, or null on success. */
+function tryRegister(owner, commands) {
+  const api = registries.stage(owner)
+  try {
+    api.registerSlashCommands(commands)
+    registries.apply(api.staged)
+    return null
+  } catch (err) {
+    return err.message
+  }
+}
+
+// ── Registration ───────────────────────────────────────────────────────────
+
+test('a registered command is served without its handler', () => {
+  assert.equal(tryRegister('uo', [cmd()]), null)
+  const [definition] = registries.slashCommandDefinitions()
+  assert.equal(definition.name, 'guild')
+  assert.equal(definition.owner, 'uo')
+  assert.equal(definition.access, 'everyone', 'access defaults rather than being undefined on the wire')
+  assert.equal(definition.handler, undefined, 'the handler never crosses the process boundary')
+  assert.equal(typeof registries.slashCommand('guild').handler, 'function')
+})
+
+// Discord's name grammar has no dot in it, so the owner-prefix rule every other
+// id in registries.js follows cannot be expressed here. Collisions are the
+// substitute, and they have to name the holder.
+test('two registrants cannot hold the same command name', () => {
+  tryRegister('uo', [cmd()])
+  assert.match(tryRegister('rust', [cmd()]), /already registered by "uo"/)
+  // The same name twice inside ONE batch, which the held-by check above cannot
+  // catch — nothing is committed yet when the second entry is validated.
+  registries._reset()
+  assert.match(tryRegister('uo', [cmd(), cmd()]), /registered twice/)
+})
+
+test('a name Discord would reject is refused at registration', () => {
+  assert.match(tryRegister('uo', [cmd({ name: 'uo.guild' })]), /bad command name/)
+  assert.match(tryRegister('uo', [cmd({ name: 'Guild' })]), /bad command name/)
+  assert.match(tryRegister('uo', [cmd({ name: 'g'.repeat(33) })]), /bad command name/)
+  assert.match(tryRegister('uo', [cmd({ description: '' })]), /description of 1-100/)
+  assert.match(tryRegister('uo', [cmd({ handler: 'nope' })]), /has no handler/)
+  assert.match(tryRegister('uo', [cmd({ access: 'members' })]), /unknown access "members"/)
+})
+
+// §7.1.1 keeps the option schema small on purpose: subcommand groups,
+// autocomplete, attachments and modals are the features whose semantics do not
+// survive a second platform.
+test('only the four option types survive registration', () => {
+  const opt = (over) => cmd({ options: [{ name: 'x', type: 'string', description: 'd', ...over }] })
+  assert.equal(tryRegister('uo', [opt({})]), null)
+  registries._reset()
+  assert.match(tryRegister('uo', [opt({ type: 'attachment' })]), /unsupported type "attachment"/)
+  assert.match(tryRegister('uo', [opt({ type: 'boolean', choices: [{ name: 'a', value: 1 }] })]), /choices need string or integer/)
+  assert.match(tryRegister('uo', [opt({ choices: [] })]), /empty choices list/)
+})
+
+// Discord rejects the whole batch for this, so it cannot be left to be
+// discovered at the next `ready`.
+test('a required option after an optional one is refused', () => {
+  const options = [
+    { name: 'a', type: 'string', description: 'd', required: false },
+    { name: 'b', type: 'string', description: 'd', required: true },
+  ]
+  assert.match(tryRegister('uo', [cmd({ options })]), /required option after an optional one/)
+})
+
+// The validate-then-commit rule the rest of registries.js follows: a batch that
+// fails leaves nothing behind, or the bot would pull a half-registered set.
+test('a batch that fails registers none of it', () => {
+  tryRegister('uo', [cmd(), cmd({ name: 'bad name' })])
+  assert.deepEqual(registries.slashCommandDefinitions(), [])
+})
+
+// ── Liveness ───────────────────────────────────────────────────────────────
+
+// The registries have no removal path — nothing takes a registration back — so a
+// module an operator disables at runtime would otherwise keep answering.
+test('a disabled module’s command disappears from the pull and stops dispatching', async () => {
+  tryRegister('uo', [cmd()])
+  loaded('started')
+  assert.equal(slash.definitions().commands.length, 1)
+  assert.equal(slash.definitions().version, 42)
+
+  loaded('disabled')
+  assert.deepEqual(slash.definitions().commands, [])
+  assert.deepEqual(await slash.dispatch({ command: 'guild' }), { ok: false, reason: 'unknown' })
+})
+
+test('before load() there is nothing registered and that is an answer, not an error', () => {
+  loader.isLoaded = () => false
+  assert.deepEqual(slash.definitions(), { version: 0, commands: [] })
+})
+
+// ── The actor ──────────────────────────────────────────────────────────────
+
+function identity({ providerId = 'discord', kind = 'discord', role = 'player', status = 'active' } = {}) {
+  authProviders.list = async () => [{ id: providerId, kind }]
+  userIdentities.findByProviderSubject = async (provider, subject) =>
+    (provider === providerId && subject === '555' ? { user_id: 9 } : null)
+  users.getById = async (id) => (id === 9 ? { id: 9, role, status } : null)
+}
+
+// `auth_providers.id` is an operator-chosen slug; `kind` is the enum. Resolving
+// by id would report "not linked" for every user on a deployment that named its
+// Discord provider anything else.
+test('the Discord provider is found by kind, whatever the operator named it', async () => {
+  identity({ providerId: 'our-discord' })
+  const actor = await slash.resolveActor({ platform: 'discord', platformUserId: '555' })
+  assert.deepEqual(actor, {
+    platform: 'discord', platformUserId: '555', guildId: null, userId: 9, role: 'player', isLinked: true, isStaff: false,
+  })
+})
+
+test('a non-Discord kind on a provider named "discord" does not link anyone', async () => {
+  identity({ kind: 'oidc' })
+  const actor = await slash.resolveActor({ platform: 'discord', platformUserId: '555' })
+  assert.equal(actor.isLinked, false)
+})
+
+// Otherwise Discord would be the one surface a ban does not reach.
+test('a banned or disabled account resolves as unlinked', async () => {
+  identity({ status: 'banned' })
+  const actor = await slash.resolveActor({ platform: 'discord', platformUserId: '555' })
+  assert.equal(actor.isLinked, false)
+  assert.equal(actor.userId, null)
+})
+
+test('staff is the same two roles every other Team surface means by it', async () => {
+  identity({ role: 'moderator' })
+  assert.equal((await slash.resolveActor({ platform: 'discord', platformUserId: '555' })).isStaff, true)
+  identity({ role: 'editor' })
+  assert.equal((await slash.resolveActor({ platform: 'discord', platformUserId: '555' })).isStaff, false)
+})
+
+// ── Dispatch ───────────────────────────────────────────────────────────────
+
+test('access is enforced by the dispatcher, not by Discord’s permission model', async () => {
+  identity({ role: 'player' })
+  loaded()
+  tryRegister('uo', [cmd({ name: 'linked-only', access: 'linked' }), cmd({ name: 'staff-only', access: 'staff' })])
+
+  const unlinked = { command: 'linked-only', platformUserId: 'nobody' }
+  assert.deepEqual(await slash.dispatch(unlinked), {
+    ok: false, reason: 'forbidden', access: 'linked', isLinked: false,
+  })
+  // A linked player clears `linked` and not `staff` — the distinction Discord's
+  // default_member_permissions cannot express at all.
+  assert.equal((await slash.dispatch({ command: 'linked-only', platformUserId: '555' })).ok, true)
+  assert.equal((await slash.dispatch({ command: 'staff-only', platformUserId: '555' })).reason, 'forbidden')
+})
+
+test('a handler that throws costs its own command and answers renderably', async () => {
+  loaded()
+  tryRegister('uo', [cmd({ handler: async () => { throw new Error('boom') } })])
+  assert.deepEqual(await slash.dispatch({ command: 'guild' }), { ok: false, reason: 'error' })
+})
+
+test('a handler that hangs is bounded, not waited on', async () => {
+  loaded()
+  tryRegister('uo', [cmd({ handler: () => new Promise(() => {}) })])
+  const started = Date.now()
+  const res = await slash.dispatch({ command: 'guild' })
+  assert.deepEqual(res, { ok: false, reason: 'error' })
+  assert.ok(Date.now() - started < slash.HANDLER_TIMEOUT_MS + 500)
+})
+
+// `ok` is core's verdict on whether the handler produced an answer, so it lives
+// outside the envelope where a handler cannot write it.
+test('a handler cannot forge the success flag', async () => {
+  loaded()
+  tryRegister('uo', [cmd({ handler: async () => ({ ok: false, text: 'hi' }) })])
+  const res = await slash.dispatch({ command: 'guild' })
+  assert.equal(res.ok, true)
+  assert.equal(res.response.ok, undefined)
+  assert.equal(res.response.text, 'hi')
+})
+
+// An oversized reply fails inside the bot's editReply, where the module that
+// produced it cannot be seen — the caller would get "that command failed" for a
+// handler that worked.
+test('the envelope is clamped to what Discord will render', () => {
+  const out = slash.envelope({
+    text: 'x'.repeat(3000),
+    fields: Array.from({ length: 30 }, (_, i) => ({ name: `n${i}`, value: 'v'.repeat(2000) })),
+    url: 'javascript:alert(1)',
+    notice: 'link your account',
+  }, 'guild')
+  assert.equal(out.text.length, 2000)
+  assert.equal(out.fields.length, 25)
+  assert.equal(out.fields[0].value.length, 1024)
+  assert.equal(out.url, undefined, 'only absolute http(s) survives')
+  assert.equal(out.notice, 'link your account')
+})
+
+test('a handler that returns nothing has simply said nothing', async () => {
+  loaded()
+  tryRegister('uo', [cmd({ handler: async () => undefined })])
+  assert.deepEqual(await slash.dispatch({ command: 'guild' }), { ok: true, response: { ephemeral: false } })
+})