From 4dea91dfedc2d089667530cff1ecfd67d06f688d Mon Sep 17 00:00:00 2001 From: Eeetan Date: Fri, 3 Jul 2026 10:02:26 -0400 Subject: [PATCH] Defer Discord replies with per-command ephemerality; make team list/roster public MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Acknowledge interactions with an early deferReply so slow (cold Neon) handlers don't blow the 3s deadline, and drive the defer's visibility from a new `ephemeral` hint on defineCommand (subcommand overrides command; fail-safe to ephemeral). safeReply now editReplies the deferred message and clears the spinner when there's nothing to say. Flip /team list and /team roster to public (ephemeral: false) — they're shared reference the whole channel benefits from — at both the defer layer (team.js) and the neutral payload (messages.js). Co-Authored-By: Claude Opus 4.8 --- discord-bot/src/adapters/discord.js | 43 ++++++- discord-bot/src/commands/team.js | 2 + discord-bot/src/defineCommand.js | 7 +- discord-bot/src/messages.js | 8 +- discord-bot/test/adapters-discord.test.js | 140 +++++++++++++++++++++- discord-bot/test/defineCommand.test.js | 36 ++++++ discord-bot/test/messages.test.js | 16 ++- 7 files changed, 243 insertions(+), 9 deletions(-) diff --git a/discord-bot/src/adapters/discord.js b/discord-bot/src/adapters/discord.js index 00286cd..3b113d1 100644 --- a/discord-bot/src/adapters/discord.js +++ b/discord-bot/src/adapters/discord.js @@ -41,10 +41,42 @@ export function payloadToDiscordReply(payload) { return out; } +// Resolve the visibility hint for an interaction BEFORE the handler runs, so we +// can defer with the right ephemerality. Mirrors the router's auth resolution: +// an active subcommand's hint wins, otherwise the command-level hint, otherwise +// fail-safe to ephemeral (private). +export function resolveEphemeral(command, subcommandName) { + const sub = subcommandName + ? command.subcommands.find((s) => s.name === subcommandName) + : null; + return sub?.ephemeral ?? command.ephemeral ?? true; +} + async function safeReply(interaction, payload) { const dpayload = payloadToDiscordReply(payload); - if (!dpayload) return; - const method = interaction.replied || interaction.deferred ? 'followUp' : 'reply'; + if (!dpayload) { + // Nothing to say. If we deferred, clear the "thinking…" state so the user + // isn't left staring at a spinner. + if (interaction.deferred && !interaction.replied) { + await interaction + .deleteReply() + .catch((e) => console.error('deleteReply failed:', e.message)); + } + return; + } + + // After deferReply(), the first response must edit the deferred message. + // Ephemerality was already locked in at defer time, so editReply ignores the + // ephemeral flag — strip it to avoid passing an unsupported option. + if (interaction.deferred && !interaction.replied) { + const { flags, ...editable } = dpayload; + await interaction + .editReply(editable) + .catch((e) => console.error('reply failed:', e.message)); + return; + } + + const method = interaction.replied ? 'followUp' : 'reply'; await interaction[method](dpayload).catch((e) => console.error('reply failed:', e.message), ); @@ -57,6 +89,13 @@ export function wireDiscordClient(client, { commands, appContext }) { if (!command) return; try { const intent = interactionToIntent(interaction, command); + // Acknowledge within Discord's 3s deadline BEFORE running the handler. + // The handler may hit a cold-starting (asleep) Neon database that takes + // several seconds to boot; without an early defer the interaction token + // expires and the reply fails ("Unknown interaction") even though the DB + // work succeeds. Deferring extends the response window to 15 minutes. + const ephemeral = resolveEphemeral(command, intent.subcommand); + await interaction.deferReply(ephemeral ? { flags: MessageFlags.Ephemeral } : {}); const payload = await dispatch(intent, { commands, appContext }); await safeReply(interaction, payload); } catch (err) { diff --git a/discord-bot/src/commands/team.js b/discord-bot/src/commands/team.js index 6874594..318089f 100644 --- a/discord-bot/src/commands/team.js +++ b/discord-bot/src/commands/team.js @@ -45,6 +45,7 @@ export default defineCommand({ name: 'list', description: 'List teams', auth: 'linked', + ephemeral: false, // shared reference — post publicly so the channel can see it options: [ { name: 'active_only', type: 'boolean', required: false, description: 'Only active teams (default true)' }, ], @@ -118,6 +119,7 @@ export default defineCommand({ name: 'roster', description: "Show a team's current roster", auth: 'linked', + ephemeral: false, // shared reference — post publicly so the channel can see it options: [ { name: 'team', type: 'string', required: true, description: 'Team slug' }, { name: 'as_of', type: 'string', required: false, description: 'ISO date (default today)' }, diff --git a/discord-bot/src/defineCommand.js b/discord-bot/src/defineCommand.js index d0e2a19..fa9f3ba 100644 --- a/discord-bot/src/defineCommand.js +++ b/discord-bot/src/defineCommand.js @@ -2,7 +2,7 @@ * Neutral command definition. Consumed by the Discord adapter and the web * adapter. No discord.js dependency here. */ -export function defineCommand({ name, description, auth, beta, options, handler, subcommands }) { +export function defineCommand({ name, description, auth, beta, ephemeral, options, handler, subcommands }) { if (!name || typeof name !== 'string') { throw new Error('defineCommand: `name` (string) is required'); } @@ -20,6 +20,9 @@ export function defineCommand({ name, description, auth, beta, options, handler, name: sub.name, description: sub.description ?? '', auth: sub.auth ?? auth ?? 'linked', + // Visibility hint used to defer the Discord reply before the (possibly + // slow) handler runs. Inherits the command-level value unless overridden. + ephemeral: sub.ephemeral ?? ephemeral ?? true, options: sub.options ?? [], handler: sub.handler, }; @@ -29,6 +32,8 @@ export function defineCommand({ name, description, auth, beta, options, handler, description: description ?? '', auth: auth ?? 'linked', beta: beta ?? false, + // Default to ephemeral: bot replies are personal directory/team info. + ephemeral: ephemeral ?? true, options: options ?? [], handler, subcommands: normalizedSubs, diff --git a/discord-bot/src/messages.js b/discord-bot/src/messages.js index f6341b8..597db8f 100644 --- a/discord-bot/src/messages.js +++ b/discord-bot/src/messages.js @@ -127,7 +127,9 @@ export function renderListTeamsResult(result) { return FALLBACK; } })(); - return { content, ephemeral: true }; + // Public: shared reference. Visibility is locked at defer time in the Discord + // adapter (see team.js `list` subcommand); this keeps the neutral payload consistent. + return { content, ephemeral: false }; } export function renderRenameTeamResult(result) { @@ -208,7 +210,9 @@ export function renderRosterResult(result) { return FALLBACK; } })(); - return { content, ephemeral: true }; + // Public: shared reference. Visibility is locked at defer time in the Discord + // adapter (see team.js `roster` subcommand); this keeps the neutral payload consistent. + return { content, ephemeral: false }; } export function renderMyTeamsResult(result) { diff --git a/discord-bot/test/adapters-discord.test.js b/discord-bot/test/adapters-discord.test.js index a8a9397..b527228 100644 --- a/discord-bot/test/adapters-discord.test.js +++ b/discord-bot/test/adapters-discord.test.js @@ -2,7 +2,56 @@ import { test } from 'node:test'; import assert from 'node:assert/strict'; import { MessageFlags } from 'discord.js'; import { defineCommand } from '../src/defineCommand.js'; -import { interactionToIntent, payloadToDiscordReply } from '../src/adapters/discord.js'; +import { + interactionToIntent, + payloadToDiscordReply, + resolveEphemeral, + wireDiscordClient, +} from '../src/adapters/discord.js'; + +// A minimal fake discord.js interaction that tracks the response lifecycle the +// way the real one does: deferReply() flips `deferred`, reply() flips `replied`. +function fakeInteraction({ commandName, subcommand = null, calls }) { + const interaction = { + commandName, + user: { id: '1', username: 'alex' }, + deferred: false, + replied: false, + isChatInputCommand: () => true, + options: { + getString: () => null, + getBoolean: () => null, + getUser: () => null, + getSubcommand: () => subcommand, + }, + async deferReply(opts) { + calls.push({ method: 'deferReply', opts }); + this.deferred = true; + }, + async reply(payload) { + calls.push({ method: 'reply', payload }); + this.replied = true; + }, + async editReply(payload) { + calls.push({ method: 'editReply', payload }); + }, + async followUp(payload) { + calls.push({ method: 'followUp', payload }); + }, + }; + return interaction; +} + +// Minimal EventEmitter-ish client stub capturing the interactionCreate handler. +function fakeClient() { + let handler = null; + return { + on: (event, fn) => { + if (event === 'interactionCreate') handler = fn; + }, + emit: (interaction) => handler(interaction), + }; +} const linkLike = defineCommand({ name: 'link', @@ -75,3 +124,92 @@ test('payloadToDiscordReply propagates empty-string content', () => { const out = payloadToDiscordReply({ content: '' }); assert.equal(out.content, ''); }); + +test('resolveEphemeral falls back to command-level hint', () => { + const cmd = defineCommand({ + name: 'whoami', + description: 'x', + ephemeral: true, + handler: async () => ({ content: 'ok' }), + }); + assert.equal(resolveEphemeral(cmd, null), true); +}); + +test('resolveEphemeral prefers the active subcommand hint', () => { + const cmd = defineCommand({ + name: 'team', + description: 'x', + ephemeral: false, + subcommands: [ + { name: 'roster', ephemeral: true, handler: async () => ({ content: 'ok' }) }, + ], + handler: async () => ({ content: 'top' }), + }); + assert.equal(resolveEphemeral(cmd, 'roster'), true); + assert.equal(resolveEphemeral(cmd, null), false); +}); + +test('wireDiscordClient defers BEFORE running the handler (slow DB safe)', async () => { + const calls = []; + let handlerRan = false; + let deferredWhenHandlerRan = null; + const command = defineCommand({ + name: 'whoami', + description: 'x', + auth: 'public', + ephemeral: true, + handler: async () => { + handlerRan = true; + // Simulate a slow Neon cold-start query; capture defer state at this point. + deferredWhenHandlerRan = calls.some((c) => c.method === 'deferReply'); + return { content: 'record', ephemeral: true }; + }, + }); + const commands = new Map([['whoami', command]]); + const client = fakeClient(); + wireDiscordClient(client, { commands, appContext: {} }); + + const interaction = fakeInteraction({ commandName: 'whoami', calls }); + await client.emit(interaction); + + assert.equal(handlerRan, true); + assert.equal(deferredWhenHandlerRan, true, 'handler must run only AFTER deferReply'); + assert.equal(calls[0].method, 'deferReply', 'deferReply must be the very first call'); +}); + +test('wireDiscordClient defers ephemerally per the resolved hint', async () => { + const calls = []; + const command = defineCommand({ + name: 'whoami', + description: 'x', + auth: 'public', + ephemeral: true, + handler: async () => ({ content: 'record' }), + }); + const client = fakeClient(); + wireDiscordClient(client, { commands: new Map([['whoami', command]]), appContext: {} }); + + await client.emit(fakeInteraction({ commandName: 'whoami', calls })); + + const defer = calls.find((c) => c.method === 'deferReply'); + assert.equal(defer.opts.flags, MessageFlags.Ephemeral); +}); + +test('wireDiscordClient edits the deferred reply with the payload', async () => { + const calls = []; + const command = defineCommand({ + name: 'whoami', + description: 'x', + auth: 'public', + ephemeral: true, + handler: async () => ({ content: 'the record' }), + }); + const client = fakeClient(); + wireDiscordClient(client, { commands: new Map([['whoami', command]]), appContext: {} }); + + await client.emit(fakeInteraction({ commandName: 'whoami', calls })); + + const edit = calls.find((c) => c.method === 'editReply'); + assert.ok(edit, 'a deferred reply should be delivered via editReply, not a second reply'); + assert.equal(edit.payload.content, 'the record'); +}); diff --git a/discord-bot/test/defineCommand.test.js b/discord-bot/test/defineCommand.test.js index 9d07c5d..8b45e14 100644 --- a/discord-bot/test/defineCommand.test.js +++ b/discord-bot/test/defineCommand.test.js @@ -41,6 +41,42 @@ test('defineCommand throws on missing handler', () => { ); }); +test('defineCommand defaults ephemeral to true', () => { + const cmd = defineCommand({ + name: 'foo', + description: 'Foo', + handler: async () => ({ content: 'ok' }), + }); + assert.equal(cmd.ephemeral, true); +}); + +test('defineCommand preserves explicit ephemeral: false', () => { + const cmd = defineCommand({ + name: 'foo', + description: 'Foo', + ephemeral: false, + handler: async () => ({ content: 'ok' }), + }); + assert.equal(cmd.ephemeral, false); +}); + +test('subcommands inherit and override ephemeral', () => { + const cmd = defineCommand({ + name: 'foo', + description: 'x', + ephemeral: false, + subcommands: [ + { name: 'inherits', description: 'i', handler: async () => ({ content: 'ok' }) }, + { name: 'overrides', description: 'o', ephemeral: true, handler: async () => ({ content: 'ok' }) }, + ], + handler: async () => ({ content: 'top' }), + }); + const inherits = cmd.subcommands.find((s) => s.name === 'inherits'); + const overrides = cmd.subcommands.find((s) => s.name === 'overrides'); + assert.equal(inherits.ephemeral, false); // inherited from parent + assert.equal(overrides.ephemeral, true); // own value wins +}); + test('defineCommand normalizes subcommands', () => { const cmd = defineCommand({ name: 'foo', diff --git a/discord-bot/test/messages.test.js b/discord-bot/test/messages.test.js index 27c46f7..5f7ed89 100644 --- a/discord-bot/test/messages.test.js +++ b/discord-bot/test/messages.test.js @@ -233,16 +233,14 @@ test('renderMyTeamsResult covers outcomes', () => { assert.match(renderMyTeamsResult({ outcome: 'DIRECTORY_DOWN' }).content, /unavailable|try again/i); }); -test('all messages.js render functions return ReplyPayload with ephemeral: true', () => { +test('personal/admin render functions return ReplyPayload with ephemeral: true', () => { const payloads = [ renderLinkResult({ outcome: 'DIRECTORY_DOWN' }), renderSeedResult({ outcome: 'DIRECTORY_DOWN' }), renderCreateTeamResult({ outcome: 'DIRECTORY_DOWN' }), - renderListTeamsResult({ outcome: 'DIRECTORY_DOWN' }), renderRenameTeamResult({ outcome: 'DIRECTORY_DOWN' }), renderAddMemberResult({ outcome: 'DIRECTORY_DOWN' }), renderRemoveMemberResult({ outcome: 'DIRECTORY_DOWN' }), - renderRosterResult({ outcome: 'DIRECTORY_DOWN' }), renderMyTeamsResult({ outcome: 'DIRECTORY_DOWN' }), ]; for (const p of payloads) { @@ -251,3 +249,15 @@ test('all messages.js render functions return ReplyPayload with ephemeral: true' assert.equal(p.embeds, undefined); } }); + +test('shared-reference render functions (list, roster) are public (ephemeral: false)', () => { + const payloads = [ + renderListTeamsResult({ outcome: 'DIRECTORY_DOWN' }), + renderRosterResult({ outcome: 'DIRECTORY_DOWN' }), + ]; + for (const p of payloads) { + assert.equal(typeof p.content, 'string'); + assert.equal(p.ephemeral, false); + assert.equal(p.embeds, undefined); + } +});