Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 41 additions & 2 deletions discord-bot/src/adapters/discord.js
Original file line number Diff line number Diff line change
Expand Up @@ -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),
);
Expand All @@ -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) {
Expand Down
2 changes: 2 additions & 0 deletions discord-bot/src/commands/team.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)' },
],
Expand Down Expand Up @@ -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)' },
Expand Down
7 changes: 6 additions & 1 deletion discord-bot/src/defineCommand.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
Expand All @@ -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,
};
Expand All @@ -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,
Expand Down
8 changes: 6 additions & 2 deletions discord-bot/src/messages.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
140 changes: 139 additions & 1 deletion discord-bot/test/adapters-discord.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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');
});
36 changes: 36 additions & 0 deletions discord-bot/test/defineCommand.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
16 changes: 13 additions & 3 deletions discord-bot/test/messages.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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);
}
});
Loading