diff --git a/bot/src/discord/dynamicCommands.js b/bot/src/discord/dynamicCommands.js index 3587a73..3632df1 100644 --- a/bot/src/discord/dynamicCommands.js +++ b/bot/src/discord/dynamicCommands.js @@ -145,6 +145,30 @@ function render(envelope) { return { embeds: [embed] } } +/** + * Deliver the envelope at the privacy the HANDLER asked for, not the privacy the + * deferral guessed. + * + * When the two agree — the ordinary case — this is one `editReply`. When the + * handler wants a private answer to a publicly deferred command, the deferred + * reply is deleted and the answer arrives as an ephemeral follow-up: the + * interaction token stays valid, so this is a supported path rather than a + * trick, and the cost is a "thinking…" that appears and vanishes. + * + * There is no reverse case. A command deferred ephemerally is one whose answers + * are all about the caller's own account, and nothing it returns should become + * public because a handler forgot a flag. + */ +async function reply(interaction, envelope, deferredEphemeral) { + const payload = render(envelope) + if (!envelope.ephemeral || deferredEphemeral) { + await interaction.editReply(payload) + return + } + await interaction.deleteReply() + await interaction.followUp({ ...payload, ephemeral: true }) +} + /** * Defer, dispatch, edit. * @@ -160,11 +184,17 @@ async function execute(interaction) { const definition = pulled.find((c) => c.name === interaction.commandName) if (!definition) return false - // Ephemerality has to be decided BEFORE the answer exists, because it is a - // property of the deferral. A command declared for linked users only is - // answered privately by default — its answer is about the caller's own - // account — and everything else defers publicly; a handler that wants the - // opposite says so, and the follow-up carries it. + // **Ephemerality is fixed at the DEFERRAL, which happens before the answer + // exists.** That is Discord's rule, not a choice here, and it is the whole + // reason this needs care: the handler decides privacy per answer — a refusal + // is private, a guild summary is not — and by the time it says so the reply is + // already public or already not. + // + // So: defer for the common case (public, or private for a command that only + // ever speaks about the caller's own account), and if the envelope disagrees, + // reconcile below. Getting this wrong is not cosmetic — the live walk caught it + // posting "guild information is not shown to your account" into the channel, + // which announces a member's access level to everyone in it. const ephemeral = definition.access === 'linked' await interaction.deferReply({ ephemeral }) @@ -178,18 +208,20 @@ async function execute(interaction) { // A transport failure and a handler failure are the same sentence to the // member and different lines in the log: one is the app being unreachable, // the other is a module's code. + // A refusal is ALWAYS private, whatever the command's usual privacy: "you do + // not have access to that" is about one member and belongs to one member. if (!res.ok) { log.warn('command dispatch failed', { command: definition.name, error: res.error }) - await interaction.editReply({ content: refusal({ reason: 'error' }) }) + await reply(interaction, { text: refusal({ reason: 'error' }), ephemeral: true }, ephemeral) return true } if (!res.data || !res.data.ok) { - await interaction.editReply({ content: refusal(res.data || {}) }) + await reply(interaction, { text: refusal(res.data || {}), ephemeral: true }, ephemeral) return true } const envelope = res.data.response || {} - await interaction.editReply(render(envelope)) + await reply(interaction, envelope, ephemeral) // The private aside beside a public answer (§9 answer 5). Skipped when the // reply was already private — the member would just be told the same thing diff --git a/bot/test/dynamicCommands.test.js b/bot/test/dynamicCommands.test.js index 58a0232..a7882b4 100644 --- a/bot/test/dynamicCommands.test.js +++ b/bot/test/dynamicCommands.test.js @@ -53,6 +53,7 @@ function fakeInteraction({ commandName = 'guild', options = {}, userId = '555' } }, deferReply: async (payload) => calls.push(['defer', payload]), editReply: async (payload) => calls.push(['edit', payload]), + deleteReply: async () => calls.push(['delete']), followUp: async (payload) => calls.push(['followUp', payload]), } } @@ -195,6 +196,46 @@ test('a notice is not repeated when the answer was already private', async () => // Every failure path EDITS. Replying to a deferred interaction throws, so a // refusal that used reply() would turn a clean "no" into an unhandled error. +// Ephemerality is fixed at the DEFERRAL, which happens before the handler has +// said anything — so honouring a per-answer flag needs the deferred reply +// withdrawn. The live walk caught the version that ignored it posting "guild +// information is not shown to your account" into the channel, which announces a +// member's access level to everyone in it. +test('a handler asking for privacy gets it, even though the deferral was public', async () => { + answers([definition()]) + await dynamic.pull() + appInternal.dispatchCommand = async () => ({ + ok: true, data: { ok: true, response: { text: 'just for you', ephemeral: true } }, + }) + const interaction = fakeInteraction() + await dynamic.execute(interaction) + assert.deepEqual(interaction.calls.map(([kind]) => kind), ['defer', 'delete', 'followUp']) + assert.deepEqual(interaction.calls.at(-1)[1], { content: 'just for you', ephemeral: true }) +}) + +test('an already-private deferral just edits — no second message', async () => { + answers([definition({ access: 'linked' })]) + await dynamic.pull() + appInternal.dispatchCommand = async () => ({ + ok: true, data: { ok: true, response: { text: 'private', ephemeral: true } }, + }) + const interaction = fakeInteraction() + await dynamic.execute(interaction) + assert.deepEqual(interaction.calls.map(([kind]) => kind), ['defer', 'edit']) +}) + +// "You do not have access to that" is about one member and belongs to one +// member, whatever the command's usual privacy. +test('a refusal is always private', async () => { + answers([definition()]) + await dynamic.pull() + appInternal.dispatchCommand = async () => ({ ok: true, data: { ok: false, reason: 'forbidden' } }) + const interaction = fakeInteraction() + await dynamic.execute(interaction) + assert.deepEqual(interaction.calls.map(([kind]) => kind), ['defer', 'delete', 'followUp']) + assert.equal(interaction.calls.at(-1)[1].ephemeral, true) +}) + test('a refusal is phrased by the bot and edited into the deferred reply', async () => { answers([definition({ access: 'linked' })]) await dynamic.pull() @@ -204,7 +245,9 @@ test('a refusal is phrased by the bot and edited into the deferred reply', async const interaction = fakeInteraction() await dynamic.execute(interaction) assert.match(interaction.calls.at(-1)[1].content, /Link your Discord account/) - assert.equal(interaction.calls.filter(([kind]) => kind === 'edit').length, 1) + // Deferred ephemerally (access: 'linked'), so the refusal is one edit and no + // withdrawal — replying twice to a deferred interaction is what throws. + assert.deepEqual(interaction.calls.map(([kind]) => kind), ['defer', 'edit']) }) test('an unreachable app is the same sentence to the member and a different line in the log', async () => { @@ -214,6 +257,7 @@ test('an unreachable app is the same sentence to the member and a different line const interaction = fakeInteraction() await dynamic.execute(interaction) assert.match(interaction.calls.at(-1)[1].content, /Something went wrong/) + assert.equal(interaction.calls.at(-1)[1].ephemeral, true) }) test('an interaction for a command the app no longer serves is left alone', async () => {