diff --git a/backend/__tests__/service/discordInstallBind.test.js b/backend/__tests__/service/discordInstallBind.test.js index 66b59f0a99..7309e75ca8 100644 --- a/backend/__tests__/service/discordInstallBind.test.js +++ b/backend/__tests__/service/discordInstallBind.test.js @@ -48,6 +48,7 @@ jest.mock('../../models/Pod', () => ({ findById: jest.fn(), find: jest.fn() })); jest.mock('../../services/discordService', () => jest.fn().mockImplementation(() => ({ initialize: jest.fn().mockResolvedValue(true), connect: jest.fn().mockResolvedValue(true), + disconnect: jest.fn().mockResolvedValue(true), }))); jest.mock('../../services/connectorSecrets', () => ({ put: jest.fn().mockResolvedValue('secret-ref-1'), @@ -328,3 +329,73 @@ describe('binding a Discord channel takes authority over the pod', () => { expect((await bindAs('member-token', proof)).status).toBe(201); }); }); + +// F161 (CodeRabbit 4475816316): "Add the bot again" on a row in error starts a +// fresh install. The new bind retires the caller's dropped row on the same pod, +// guild and channel the way Remove would, so the page doesn't show two rows. +describe('a fresh bind replaces the dropped row it reconnects', () => { + const OTHER_CHANNEL = '456789012345678901'; + const seedRow = async (over = {}) => { + const row = await Integration.create({ + podId: POD_ID, + type: 'discord', + createdBy: OWNER, + status: 'error', + config: { serverId: GUILD, channelId: CHANNEL, channelName: 'general' }, + ...over, + }); + await DiscordIntegration.create({ + integrationId: row._id, serverId: GUILD, serverName: 'Commonly Dev', channelId: row.config.channelId, channelName: 'general', webhookId: 'old-hook', + }); + return row; + }; + + const bindViaInstall = async () => { + const link = await request(app).get(`/api/discord/install-link/${POD_ID}`).set('Authorization', 'Bearer owner-token'); + const authorize = new URL(link.body.installUrl); + const back = new URL(authorize.searchParams.get('redirect_uri')); + const cb = await request(app) + .get(`${back.pathname}?code=the-code&state=${authorize.searchParams.get('state')}&guild_id=${GUILD}`); + const proof = new URL(cb.headers.location).searchParams.get('install'); + return request(app) + .post('/api/integrations') + .set('Authorization', 'Bearer owner-token') + .send({ + podId: POD_ID, + type: 'discord', + config: { serverId: GUILD, serverName: 'Commonly Dev', channelId: CHANNEL, channelName: 'general' }, + install: proof, + }); + }; + + test.each(['error', 'disconnected'])('a %s row on the same channel ends up inactive, its webhook record gone', async (status) => { + const old = await seedRow({ status }); + const bind = await bindViaInstall(); + expect(bind.status).toBe(201); + const retired = await Integration.findById(old._id).lean(); + expect(retired.isActive).toBe(false); + expect(retired.status).toBe('disconnected'); + expect(await DiscordIntegration.findOne({ integrationId: old._id })).toBeNull(); + const DiscordService = require('../../services/discordService'); + const retiredService = DiscordService.mock.results + .map((r, i) => ({ value: r.value, id: String(DiscordService.mock.calls[i][0]) })) + .find((r) => r.id === String(old._id)); + expect(retiredService.value.disconnect).toHaveBeenCalled(); + const live = await Integration.find({ podId: POD_ID, type: 'discord', isActive: true }).lean(); + expect(live.map((r) => String(r._id))).toEqual([String(bind.body.integration._id)]); + }); + + test('rows the bind does not replace stay as they are', async () => { + const otherChannel = await seedRow({ config: { serverId: GUILD, channelId: OTHER_CHANNEL } }); + const connected = await seedRow({ status: 'connected' }); + const someoneElses = await seedRow({ createdBy: MEMBER }); + const bind = await bindViaInstall(); + expect(bind.status).toBe(201); + for (const row of [otherChannel, connected, someoneElses]) { + // eslint-disable-next-line no-await-in-loop + const after = await Integration.findById(row._id).lean(); + expect(after.isActive).toBe(true); + expect(after.status).toBe(row.status); + } + }); +}); diff --git a/backend/__tests__/unit/routes/discord.management-auth.test.js b/backend/__tests__/unit/routes/discord.management-auth.test.js index 36cb43a8ea..a5d0845d36 100644 --- a/backend/__tests__/unit/routes/discord.management-auth.test.js +++ b/backend/__tests__/unit/routes/discord.management-auth.test.js @@ -172,6 +172,8 @@ describe('discord management route auth', () => { expect(res.status).toBe(200); expect(DiscordService).toHaveBeenCalledWith('integration-1'); + // F161 item 2: the route no longer claims the legacy set per guild; it is global. + expect(res.body.commands).toEqual([]); }); it('requires auth and admin access for register-all', async () => { diff --git a/backend/__tests__/unit/routes/discord.relayCommands.test.js b/backend/__tests__/unit/routes/discord.relayCommands.test.js index 0b2c03346e..7770fd27a1 100644 --- a/backend/__tests__/unit/routes/discord.relayCommands.test.js +++ b/backend/__tests__/unit/routes/discord.relayCommands.test.js @@ -52,6 +52,7 @@ const Integration = require('../../../models/Integration'); const Summary = require('../../../models/Summary'); const { runCommandForIntegration } = require('../../../services/discordMultiCommandService'); const discordRoutes = require('../../../routes/discord'); +const { resetSummaryCooldowns } = require('../../../services/discordRelayCommandService'); const app = express(); app.use('/api/discord/interactions', express.raw({ type: '*/*' })); @@ -110,6 +111,7 @@ beforeEach(() => { jest.clearAllMocks(); mockRows.length = 0; mockRows.push(row()); + resetSummaryCooldowns(); // The pod's creator: the bind rule /tldr re-checks (F143) passes for them. User.findById.mockResolvedValue({ _id: 'user-owner', role: 'user' }); }); @@ -171,6 +173,14 @@ describe('/mute and /unmute', () => { expect(res.body.data.flags).toBe(64); }); + // F161 item 7: the core's usage outcome, in Discord's words. + test.each(['90s', '1.5h'])('/mute %s answers with usage and writes nothing', async (value) => { + const res = await command('mute', { options: [{ name: 'duration', value }] }); + expect(Integration.findByIdAndUpdate).not.toHaveBeenCalled(); + expect(res.body.data.flags).toBe(64); + expect(res.body.data.content).toContain('Usage: /mute'); + }); + test('/unmute clears relayMutedUntil', async () => { await command('unmute'); expect(Integration.findByIdAndUpdate).toHaveBeenCalledWith('row-1', { $unset: { 'config.relayMutedUntil': '' } }); @@ -286,3 +296,94 @@ test('/help answers anywhere, ephemerally', async () => { expect(res.body.data.content).toContain('/mode'); expect(res.body.data.content).toContain('/tldr'); }); + +const settle = async () => { + await new Promise((resolve) => { setImmediate(resolve); }); + await new Promise((resolve) => { setImmediate(resolve); }); +}; + +// F161 item 3: /summary and /tldr run only on a connected row (F132's relay +// rule); a settings write on a row that is not connected says so. +describe('a bound row that is not connected', () => { + test.each(['disconnected', 'error', 'pending'])('/tldr on a %s row posts nothing', async (status) => { + mockRows[0] = { ...row(), status }; + Summary.findOne.mockReturnValue({ sort: () => Promise.resolve({ title: 'T', content: 'secret pod text' }) }); + const res = await command('tldr'); + expect(res.body.type).toBe(4); + expect(res.body.data.flags).toBe(64); + expect(res.body.data.content).not.toContain('secret pod text'); + expect(res.body.data.content).toContain("isn't active"); + }); + + test('/summary on a disconnected row runs nothing and answers at once', async () => { + mockRows[0] = { ...row({ webhookListenerEnabled: true }), status: 'disconnected' }; + const res = await command('summary'); + expect(res.body.type).toBe(4); + expect(res.body.data.flags).toBe(64); + expect(res.body.data.content).toContain("isn't active"); + await settle(); + expect(runCommandForIntegration).not.toHaveBeenCalled(); + }); + + test.each([ + ['mode', [{ name: 'mode', value: 'mirror' }]], + ['mute', [{ name: 'duration', value: '30m' }]], + ['unmute', undefined], + ])('/%s from the installer on a disconnected row writes nothing and says so', async (name, options) => { + mockRows[0] = { ...row(), status: 'disconnected' }; + const res = await command(name, { options }); + expect(Integration.findByIdAndUpdate).not.toHaveBeenCalled(); + expect(res.body.data.flags).toBe(64); + expect(res.body.data.content).toContain("isn't connected right now"); + }); +}); + +// F161 item 4: each /summary is an LLM run, so one row runs it at most once a minute. +test('/summary twice inside a minute runs once; the second is told to wait, ephemerally', async () => { + mockRows[0] = row({ webhookListenerEnabled: true }); + runCommandForIntegration.mockResolvedValue({ result: { success: true, content: 'ok' } }); + const first = await command('summary', { actor: STRANGER }); + expect(first.body.type).toBe(5); + await settle(); + const second = await command('summary', { actor: INSTALLER }); + expect(second.body.type).toBe(4); + expect(second.body.data.flags).toBe(64); + expect(second.body.data.content).toMatch(/less than a minute ago/); + await settle(); + expect(runCommandForIntegration).toHaveBeenCalledTimes(1); +}); + +test('/summary runs again once the minute has passed', async () => { + mockRows[0] = row({ webhookListenerEnabled: true }); + runCommandForIntegration.mockResolvedValue({ result: { success: true, content: 'ok' } }); + const realNow = Date.now; + try { + await command('summary'); + await settle(); + Date.now = () => realNow() + 61_000; + const again = await command('summary'); + expect(again.body.type).toBe(5); + await settle(); + expect(runCommandForIntegration).toHaveBeenCalledTimes(2); + } finally { + Date.now = realNow; + } +}); + +// F161 item 9: an admin pause takes the row out of every command's reach. +describe('a row under an admin pause', () => { + beforeEach(() => { mockRows[0] = row({ adminPause: { at: new Date(), by: 'admin-1' } }); }); + + test('the installer cannot change its settings', async () => { + const res = await command('mode', { options: [{ name: 'mode', value: 'mirror' }] }); + expect(Integration.findByIdAndUpdate).not.toHaveBeenCalled(); + expect(res.body.data.flags).toBe(64); + }); + + test('/tldr posts nothing', async () => { + Summary.findOne.mockReturnValue({ sort: () => Promise.resolve({ title: 'T', content: 'secret pod text' }) }); + const res = await command('tldr'); + expect(res.body.data.flags).toBe(64); + expect(res.body.data.content).not.toContain('secret pod text'); + }); +}); diff --git a/backend/__tests__/unit/routes/slack.webhook.commands.test.js b/backend/__tests__/unit/routes/slack.webhook.commands.test.js index c45054e29c..719d096cba 100644 --- a/backend/__tests__/unit/routes/slack.webhook.commands.test.js +++ b/backend/__tests__/unit/routes/slack.webhook.commands.test.js @@ -71,9 +71,11 @@ describe('Slack /commonly command replies', () => { ['mute', 60], ['mute 15', 15], ['mute 0', 60], - ['mute -5', 1], ['mute 99999', 24 * 60], - ['mute soon', 60], + ['mute 2h', 120], + // F161: the whole argument reaches the parse, not just its first word. + ['mute 2 hours', 120], + ['mute 30 min', 30], ])('%s mutes for %i minutes', async (text, minutes) => { bound(); const before = Date.now(); @@ -86,6 +88,20 @@ describe('Slack /commonly command replies', () => { expect(until).toBeLessThanOrEqual(Date.now() + minutes * 60000); }); + // F161: a duration the shared core cannot read is usage, not a guess. + test.each(['mute -5', 'mute soon', 'mute 90s'])('%s answers with usage and writes nothing', async (text) => { + bound(); + const res = await command(text); + expect(res.body).toEqual({ response_type: 'ephemeral', text: 'Usage: /commonly mute 30m|2h' }); + expect(Integration.updateOne).not.toHaveBeenCalled(); + }); + + test('mode still reads only its first word', async () => { + bound(); + const res = await command('mode attention please'); + expect(res.body).toEqual({ response_type: 'ephemeral', text: 'Relay mode: attention.' }); + }); + test('unmute clears only the owner mute', async () => { bound(); const res = await command('unmute'); diff --git a/backend/__tests__/unit/routes/telegram.webhook.test.js b/backend/__tests__/unit/routes/telegram.webhook.test.js index 4e9a5d189b..b1db1cc19b 100644 --- a/backend/__tests__/unit/routes/telegram.webhook.test.js +++ b/backend/__tests__/unit/routes/telegram.webhook.test.js @@ -337,6 +337,40 @@ describe('bridge command surface (/mode /status /mute /help)', () => { expect(until).toBeGreaterThan(Date.now()); }); + // F161 item 6: the whole argument reaches the parse, so `2 hours` is 120 minutes. + it('/mute 2 hours mutes for 120 minutes', async () => { + const before = Date.now(); + const res = await post('/mute 2 hours'); + expect(res.status).toBe(200); + const [, update] = Integration.findByIdAndUpdate.mock.calls[0]; + const until = new Date(update.$set['config.relayMutedUntil']).getTime(); + expect(until).toBeGreaterThanOrEqual(before + 120 * 60000); + expect(until).toBeLessThanOrEqual(Date.now() + 120 * 60000); + expect(telegramService.sendMessage).toHaveBeenCalledWith('bot-token', '42', expect.stringContaining('Muted for 120 min')); + }); + + it('/mode still reads only its first word', async () => { + await post('/mode attention please'); + expect(Integration.findByIdAndUpdate).toHaveBeenCalledWith( + 'integration-1', + { $set: { 'config.relayAllAgentMessages': false } }, + ); + }); + + // F161 item 7: a duration the parse cannot read is answered with usage. + it.each(['90s', '1.5h'])('/mute %s answers with usage and writes nothing', async (arg) => { + await post(`/mute ${arg}`); + expect(Integration.findByIdAndUpdate).not.toHaveBeenCalled(); + expect(telegramService.sendMessage).toHaveBeenCalledWith('bot-token', '42', expect.stringContaining('Usage: /mute')); + }); + + it('/help names the duration forms', async () => { + await post('/help'); + const text = telegramService.sendMessage.mock.calls[0][2]; + expect(text).toContain('30m'); + expect(text).toContain('2h'); + }); + it('/unmute only clears the owner mute and cannot clear an admin pause', async () => { const res = await post('/unmute'); diff --git a/backend/__tests__/unit/scripts/deployDiscordCommands.test.js b/backend/__tests__/unit/scripts/deployDiscordCommands.test.js index 495d320044..24cdc9d448 100644 --- a/backend/__tests__/unit/scripts/deployDiscordCommands.test.js +++ b/backend/__tests__/unit/scripts/deployDiscordCommands.test.js @@ -14,7 +14,10 @@ beforeEach(() => { jest.spyOn(console, 'log').mockImplementation(() => {}); }); -afterEach(() => jest.restoreAllMocks()); +afterEach(() => { + jest.restoreAllMocks(); + process.exitCode = undefined; +}); test('one PUT to the global commands endpoint, carrying all seven bridge commands', async () => { axios.put.mockResolvedValue({ status: 200 }); @@ -40,3 +43,135 @@ test('/mode offers exactly the two modes, and /mute takes a duration', () => { const mute = commands.find((c) => c.name === 'mute'); expect(mute.options[0]).toMatchObject({ name: 'duration', type: 3, required: false }); }); + +// F161 item 8: the bridge commands make sense only in a guild channel bound to a pod. +test('every bridge command is offered in guilds only', () => { + const commands = require('../../../config/discordApplicationCommands.json'); + BRIDGE.forEach((name) => { + expect(commands.find((c) => c.name === name).contexts).toEqual([0]); + }); +}); + +// F161 item 5: the boot-time PUT cannot hang boot, and a failure is an exit code. +describe('deploy at boot', () => { + beforeEach(() => { + jest.spyOn(console, 'error').mockImplementation(() => {}); + jest.spyOn(DiscordCommandDeployment.prototype, 'sleep').mockResolvedValue(); + }); + + test('the PUT carries a 10 s timeout', async () => { + axios.put.mockResolvedValue({ status: 200 }); + await new DiscordCommandDeployment().deploy(); + expect(axios.put.mock.calls[0][2].timeout).toBe(10_000); + }); + + test('main sets exit code 1 when every attempt fails', async () => { + axios.put.mockRejectedValue(Object.assign(new Error('timeout of 10000ms exceeded'), { code: 'ECONNABORTED' })); + await DiscordCommandDeployment.main(['deploy']); + expect(axios.put).toHaveBeenCalledTimes(3); + expect(process.exitCode).toBe(1); + }); + + test('main sets exit code 1 when credentials are missing', async () => { + delete process.env.DISCORD_BOT_TOKEN; + await DiscordCommandDeployment.main(['deploy']); + expect(axios.put).not.toHaveBeenCalled(); + expect(process.exitCode).toBe(1); + }); + + test('main leaves the exit code alone on success', async () => { + axios.put.mockResolvedValue({ status: 200 }); + await DiscordCommandDeployment.main(['deploy']); + expect(process.exitCode).toBeUndefined(); + }); +}); + +// F161 (CodeRabbit 4475816306): axios's `timeout` stops counting once headers +// arrive, so a body that trickles forever would still hold boot. Each attempt +// has a wall-clock deadline over the whole request and response. +describe('the per-attempt deadline', () => { + beforeEach(() => { + jest.spyOn(console, 'error').mockImplementation(() => {}); + jest.spyOn(DiscordCommandDeployment.prototype, 'sleep').mockResolvedValue(); + jest.useFakeTimers(); + }); + afterEach(() => jest.useRealTimers()); + + // A request that never settles unless its signal aborts it. + const hangUntilAborted = (_url, _body, options) => new Promise((_resolve, reject) => { + options.signal.addEventListener('abort', () => reject(Object.assign(new Error('canceled'), { code: 'ERR_CANCELED' }))); + }); + + test('a hanging response is aborted at 15 s, and three of them fail the deploy', async () => { + axios.put.mockImplementation(hangUntilAborted); + const done = new DiscordCommandDeployment().deploy(); + await jest.advanceTimersByTimeAsync(14_999); + expect(axios.put).toHaveBeenCalledTimes(1); + expect(axios.put.mock.calls[0][2].signal.aborted).toBe(false); + await jest.advanceTimersByTimeAsync(1); + expect(axios.put.mock.calls[0][2].signal.aborted).toBe(true); + await jest.advanceTimersByTimeAsync(30_000); + const result = await done; + expect(axios.put).toHaveBeenCalledTimes(3); + expect(result.success).toBe(false); + }); + + test('a settled attempt leaves no deadline timer behind', async () => { + axios.put.mockResolvedValue({ status: 200 }); + await new DiscordCommandDeployment().deploy(); + expect(jest.getTimerCount()).toBe(0); + }); +}); + +// F161 items 6 and 7: verify's exit status, and a registered command whose +// shape has gone stale (contexts or options) counts as a failure. +describe('verify', () => { + const commands = require('../../../config/discordApplicationCommands.json'); + const live = () => JSON.parse(JSON.stringify(commands)).map((c) => ({ id: `id-${c.name}`, application_id: 'app-1', version: '1', ...c })); + + beforeEach(() => { + jest.spyOn(console, 'error').mockImplementation(() => {}); + }); + + test('a matching set exits 0', async () => { + axios.get.mockResolvedValue({ status: 200, data: live() }); + await DiscordCommandDeployment.main(['verify']); + expect(process.exitCode).toBeUndefined(); + }); + + test('a missing command exits 1', async () => { + axios.get.mockResolvedValue({ status: 200, data: live().filter((c) => c.name !== 'tldr') }); + await DiscordCommandDeployment.main(['verify']); + expect(process.exitCode).toBe(1); + }); + + test('an API error exits 1', async () => { + axios.get.mockRejectedValue(new Error('503')); + await DiscordCommandDeployment.main(['verify']); + expect(process.exitCode).toBe(1); + }); + + test('a command registered without its guild-only context is stale', async () => { + const data = live().map((c) => (c.name === 'mute' ? { ...c, contexts: null } : c)); + axios.get.mockResolvedValue({ status: 200, data }); + const result = await new DiscordCommandDeployment().verifyDeployment(); + expect(result.success).toBe(false); + expect(result.staleCommands).toEqual(['mute']); + }); + + test('a command whose choices changed is stale', async () => { + const data = live().map((c) => (c.name === 'mode' + ? { ...c, options: [{ ...c.options[0], choices: [{ name: 'mirror', value: 'mirror' }] }] } + : c)); + axios.get.mockResolvedValue({ status: 200, data }); + const result = await new DiscordCommandDeployment().verifyDeployment(); + expect(result.staleCommands).toEqual(['mode']); + }); + + test('fields Discord adds on its own are not a difference', async () => { + const data = live().map((c) => ({ ...c, dm_permission: true, nsfw: false, integration_types: [0] })); + axios.get.mockResolvedValue({ status: 200, data }); + const result = await new DiscordCommandDeployment().verifyDeployment(); + expect(result.success).toBe(true); + }); +}); diff --git a/backend/__tests__/unit/services/chatRelayCore.commands.test.js b/backend/__tests__/unit/services/chatRelayCore.commands.test.js index 3439c8a8c0..83e4c9bdd4 100644 --- a/backend/__tests__/unit/services/chatRelayCore.commands.test.js +++ b/backend/__tests__/unit/services/chatRelayCore.commands.test.js @@ -25,12 +25,23 @@ describe('parseRelayCommand', () => { test.each([ ['30m', 30], ['30 min', 30], ['2h', 120], ['1 hour', 60], ['48h', 1440], ['45', 45], ['0m', 60], - [' 2h ', 120], ['0' + '\t'.repeat(50000) + 'x', 60], + [' 2h ', 120], ])('mute %p is %i minutes', (arg, minutes) => { const outcome = applyRelayCommand({ name: 'mute', arg }, integration, NOW); expect(outcome).toMatchObject({ kind: 'mute', minutes, until: new Date(NOW.getTime() + minutes * MIN) }); }); +// ReDoS guard (#3469): a long run of whitespace before an unreadable unit must +// stay linear. Since F161 an unreadable duration is usage, not the default. +test('a 50,000-tab duration is usage, answered quickly', () => { + const arg = '0' + '\t'.repeat(50000) + 'x'; + const started = process.hrtime.bigint(); + const outcome = applyRelayCommand({ name: 'mute', arg }, integration, NOW); + const elapsedMs = Number(process.hrtime.bigint() - started) / 1e6; + expect(outcome).toEqual({ kind: 'mute-usage' }); + expect(elapsedMs).toBeLessThan(50); +}); + describe('the authority gate', () => { test.each([ [{ name: 'mode', arg: 'mirror' }], diff --git a/backend/__tests__/unit/services/chatRelayCore.test.js b/backend/__tests__/unit/services/chatRelayCore.test.js index 83f6b05ecf..684d87c700 100644 --- a/backend/__tests__/unit/services/chatRelayCore.test.js +++ b/backend/__tests__/unit/services/chatRelayCore.test.js @@ -76,7 +76,8 @@ describe('applyRelayCommand', () => { }); test.each([ - [undefined, 60], ['15', 15], ['0', 60], ['-5', 1], ['99999', 1440], ['soon', 60], + [undefined, 60], ['', 60], ['15', 15], ['0', 60], ['99999', 1440], + ['30m', 30], ['2h', 120], ['2 hours', 120], [' 45 min ', 45], ])('mute %p is %i minutes from now', (arg, minutes) => { const outcome = applyRelayCommand({ name: 'mute', arg }, integration, NOW); const until = new Date(NOW.getTime() + minutes * MIN); @@ -85,6 +86,17 @@ describe('applyRelayCommand', () => { }); }); + // F161 item 7: a unit the parse cannot read is usage, not a guess. Before, + // `90s` fell through to parseInt and muted for 90 minutes. + test.each(['90s', '1.5h', 'soon', '-5', '2 days'])('mute %p is usage and writes nothing', (arg) => { + expect(applyRelayCommand({ name: 'mute', arg }, integration, NOW)).toEqual({ kind: 'mute-usage' }); + }); + + test('a refused caller still gets usage for a bad duration, since nothing would be written', () => { + expect(applyRelayCommand({ name: 'mute', arg: '90s' }, integration, NOW, { mayChangeSettings: false })) + .toEqual({ kind: 'mute-usage' }); + }); + test('unmute unsets only the owner mute', () => { expect(applyRelayCommand({ name: 'unmute' }, integration)) .toEqual({ kind: 'unmute', update: { $unset: { 'config.relayMutedUntil': '' } } }); diff --git a/backend/__tests__/unit/services/discordCommandService.test.js b/backend/__tests__/unit/services/discordCommandService.test.js index e558971b5d..aedfc81b69 100644 --- a/backend/__tests__/unit/services/discordCommandService.test.js +++ b/backend/__tests__/unit/services/discordCommandService.test.js @@ -162,3 +162,48 @@ describe('DiscordCommandService /commonly-summary on a shared channel', () => { ); }); + +// F161 item 1: /discord-enable and /discord-disable change what a shared +// channel feeds into the pod, so they take the rule the bridge commands use: +// only the installer, matched by the Discord id the bot-add proved. +describe('DiscordCommandService /discord-enable and /discord-disable authority', () => { + const INSTALLER = '345678901234567890'; + const STRANGER = '456789012345678901'; + const toggle = async (which, actorId, config = { discordUserId: INSTALLER }) => { + Integration.findOne.mockResolvedValue({ _id: 'integration123', config: { serverId: 'g', channelId: 'c', ...config } }); + Integration.findByIdAndUpdate.mockResolvedValue({}); + const service = new DiscordCommandService({ integrationId: 'integration123' }); + await service.initialize(); + return which === 'enable' ? service.handleEnableCommand(actorId) : service.handleDisableCommand(actorId); + }; + + beforeEach(() => jest.clearAllMocks()); + + it.each([['enable', true], ['disable', false]])('the installer may %s sync', async (which, flag) => { + const result = await toggle(which, INSTALLER); + expect(result.success).toBe(true); + expect(Integration.findByIdAndUpdate).toHaveBeenCalledWith('integration123', { 'config.webhookListenerEnabled': flag }); + }); + + it.each(['enable', 'disable'])('someone else in the channel is refused /discord-%s', async (which) => { + const result = await toggle(which, STRANGER); + expect(Integration.findByIdAndUpdate).not.toHaveBeenCalled(); + expect(result.success).toBe(false); + expect(result.content).toMatch(/only the person who connected this channel/i); + // No ping: the legacy reply carries no allowed_mentions guard. + expect(result.content).not.toContain('<@'); + }); + + it('a caller with no Discord id is refused', async () => { + const result = await toggle('enable', undefined); + expect(Integration.findByIdAndUpdate).not.toHaveBeenCalled(); + expect(result.success).toBe(false); + }); + + it('a row with no proven installer admits no one, and says how to fix it', async () => { + const result = await toggle('disable', INSTALLER, {}); + expect(Integration.findByIdAndUpdate).not.toHaveBeenCalled(); + expect(result.success).toBe(false); + expect(result.content).toMatch(/add the bot again/i); + }); +}); diff --git a/backend/__tests__/unit/services/discordMultiCommandService.test.js b/backend/__tests__/unit/services/discordMultiCommandService.test.js index 98a0f73e63..4ca7e968de 100644 --- a/backend/__tests__/unit/services/discordMultiCommandService.test.js +++ b/backend/__tests__/unit/services/discordMultiCommandService.test.js @@ -107,3 +107,16 @@ describe('discordMultiCommandService', () => { expect(results[1].result.content).toBe('Status B'); }); }); + +// F161 item 1: the multi-pod path hands the caller's Discord id to the toggles. +describe('discordMultiCommandService toggles carry the caller', () => { + it.each([['discord-enable', 'handleEnableCommand'], ['discord-disable', 'handleDisableCommand']])( + '%s passes actorId', + async (commandName, method) => { + const handler = jest.fn().mockResolvedValue({ success: true, content: 'ok' }); + DiscordCommandService.mockImplementation(() => ({ initialize: jest.fn().mockResolvedValue(true), [method]: handler })); + await runCommandForIntegration({ commandName, integration: { _id: 'i1' }, actorId: '345678901234567890' }); + expect(handler).toHaveBeenCalledWith('345678901234567890'); + }, + ); +}); diff --git a/backend/__tests__/unit/services/discordService.test.js b/backend/__tests__/unit/services/discordService.test.js index 4eaffdb7f1..817bda26eb 100644 --- a/backend/__tests__/unit/services/discordService.test.js +++ b/backend/__tests__/unit/services/discordService.test.js @@ -130,6 +130,36 @@ describe('DiscordService', () => { }); }); + // F161 item 1: the single-pod path hands the caller's Discord id to the toggles. + describe('handleSlashCommand passes the caller to the sync toggles', () => { + it.each([ + ['discord-enable', 'handleEnableCommand', { member: { user: { id: '345678901234567890' } } }], + ['discord-disable', 'handleDisableCommand', { user: { id: '345678901234567890' } }], + ])('%s', async (name, method, who) => { + const handler = jest.fn().mockResolvedValue({ success: true, content: 'ok' }); + discordService.commandService = { [method]: handler }; + await discordService.handleSlashCommand(name, { type: 2, data: { name }, ...who }); + expect(handler).toHaveBeenCalledWith('345678901234567890'); + }); + }); + + // F161 item 4: nothing is expected per guild, so a guild that still holds + // commands is not verified. + describe('verifyCommandRegistration', () => { + it('passes for a guild with no commands', async () => { + axios.get.mockResolvedValue({ status: 200, data: [] }); + const result = await discordService.verifyCommandRegistration('guild123'); + expect(result.success).toBe(true); + }); + + it('fails for a guild that still holds copies', async () => { + axios.get.mockResolvedValue({ status: 200, data: [{ name: 'discord-enable' }] }); + const result = await discordService.verifyCommandRegistration('guild123'); + expect(result.success).toBe(false); + expect(result.unexpectedCommands).toEqual(['discord-enable']); + }); + }); + describe('initialization', () => { it('should initialize successfully with valid integration', async () => { const result = await discordService.initialize(); @@ -450,6 +480,42 @@ describe('DiscordService', () => { }); describe('registerSlashCommands', () => { + const GLOBAL_SET = require('../../../config/discordApplicationCommands.json'); + const GLOBAL_URL = 'https://discord.com/api/v10/applications/test-client-id/commands'; + beforeEach(() => { + axios.get.mockImplementation(async (url) => (url === GLOBAL_URL ? { status: 200, data: GLOBAL_SET } : { status: 404 })); + }); + + // F161 item 5: clearing a guild's copies while the global set is missing + // (a boot deploy that failed) would leave that guild with no commands. + it('keeps the guild set when the global commands are not registered', async () => { + axios.get.mockResolvedValue({ status: 200, data: GLOBAL_SET.filter((c) => c.name !== 'mute') }); + const result = await discordService.registerSlashCommands('guild123'); + expect(result).toBe(false); + expect(axios.put).not.toHaveBeenCalled(); + expect(discordService.lastRegistrationError).toMatch(/global commands are not all registered/i); + expect(discordService.lastRegistrationError).toContain('mute'); + }); + + it('keeps the guild set when the global list cannot be read', async () => { + axios.get.mockRejectedValue(new Error('503')); + const result = await discordService.registerSlashCommands('guild123'); + expect(result).toBe(false); + expect(axios.put).not.toHaveBeenCalled(); + }); + + // F161 item 2: every command is global (config/discordApplicationCommands.json), + // so a guild registration carries none of them. The PUT is a bulk overwrite, + // so an empty one also clears the duplicates older guild registrations left. + it('registers nothing per guild that is already global', async () => { + axios.put.mockResolvedValue({ status: 200 }); + await discordService.registerSlashCommands('guild123'); + const [url, commands] = axios.put.mock.calls[0]; + expect(url).toMatch(/\/guilds\/guild123\/commands$/); + const globalNames = require('../../../config/discordApplicationCommands.json').map((c) => c.name); + expect(commands.filter((c) => globalNames.includes(c.name))).toEqual([]); + }); + it('should register commands successfully', async () => { axios.put.mockResolvedValue({ status: 200 }); diff --git a/backend/config/discordApplicationCommands.json b/backend/config/discordApplicationCommands.json index 3289b48dfe..503850623b 100644 --- a/backend/config/discordApplicationCommands.json +++ b/backend/config/discordApplicationCommands.json @@ -3,6 +3,7 @@ "name": "mode", "description": "Show this channel's relay mode, or switch it", "type": 1, + "contexts": [0], "options": [ { "name": "mode", @@ -19,12 +20,14 @@ { "name": "status", "description": "The pod, relay mode and mute state for this channel", - "type": 1 + "type": 1, + "contexts": [0] }, { "name": "mute", "description": "Pause relay to this channel (default 60 minutes, at most 24 hours)", "type": 1, + "contexts": [0], "options": [ { "name": "duration", @@ -37,22 +40,26 @@ { "name": "unmute", "description": "Resume relay to this channel", - "type": 1 + "type": 1, + "contexts": [0] }, { "name": "tldr", "description": "Post the pod's latest summary here", - "type": 1 + "type": 1, + "contexts": [0] }, { "name": "summary", "description": "Summarize this channel's recent chat into the pod", - "type": 1 + "type": 1, + "contexts": [0] }, { "name": "help", "description": "List the Commonly commands for this channel", - "type": 1 + "type": 1, + "contexts": [0] }, { "name": "commonly-summary", diff --git a/backend/routes/discord.ts b/backend/routes/discord.ts index c190f4b5c4..dc233503f3 100644 --- a/backend/routes/discord.ts +++ b/backend/routes/discord.ts @@ -26,7 +26,9 @@ import { listIntegrationsRateLimit, writeIntegrationsRateLimit } from '../middle import { platformIpRateLimit } from '../middleware/platformRateLimit'; import { invalidDiscordIdError, isDiscordSnowflake, isSupplied } from '../utils/discordBinding'; import { assertCanBindSurface, SurfaceBindRefusedError } from '../services/chatRelay/bindSurface'; -import { discordApplicationCommands, handleDiscordRelayCommand, isDiscordRelayCommand } from '../services/discordRelayCommandService'; +import { + discordApplicationCommands, handleDiscordRelayCommand, interactionActorId, isDiscordRelayCommand, +} from '../services/discordRelayCommandService'; import { mintDiscordInstallProof, mintDiscordInstallState, readDiscordInstallProof, readDiscordInstallState, } from '../utils/discordInstallProof'; @@ -221,7 +223,9 @@ router.post('/interactions', async (req: AuthReq & { body: Buffer }, res: Res) = return res.json({ type: 4, data: { content: '❌ Command not recognized.', flags: 64 } }); } - const commandResults = await runDiscordCommandForIntegrations({ commandName: data.name, integrations, guildId, channelId }) as Array<{ integration: typeof integrations[0]; result: { content: string; success: boolean; refused?: boolean } }>; + const commandResults = await runDiscordCommandForIntegrations({ + commandName: data.name, integrations, guildId, channelId, actorId: interactionActorId(interaction), + }) as Array<{ integration: typeof integrations[0]; result: { content: string; success: boolean; refused?: boolean } }>; const header = `🔗 ${integrations.length} pods linked to this Discord channel`; const blocks = commandResults.map(({ integration, result }) => { const podName = (integration.podId as { name?: string })?.name || integration.config?.channelName || `Pod ${integration.podId || integration._id}`; @@ -328,8 +332,12 @@ router.post('/register-commands/:integrationId', auth, async (req: AuthReq, res: const discordService = new DiscordService(integrationId); await discordService.initialize(); const success = await discordService.registerSlashCommands(guildId); - if (success) res.json({ message: 'Slash commands registered successfully', commands: ['/commonly-summary', '/discord-status', '/discord-enable', '/discord-disable'] }); - else res.status(500).json({ error: 'Failed to register slash commands' }); + // Every command is global (F161), so a guild registration clears the + // guild's duplicate copies and adds none. + if (success) res.json({ message: 'Guild command copies cleared; every command is registered globally', commands: [] }); + // The service's reason, e.g. the global set is not in place so the guild's + // commands were kept (F161). Its text names no credential. + else res.status(500).json({ error: discordService.lastRegistrationError || 'Failed to register slash commands' }); } catch (error) { console.error('Error registering slash commands:', error); res.status(500).json({ error: 'Failed to register slash commands' }); diff --git a/backend/routes/integrations.ts b/backend/routes/integrations.ts index afc69108d7..6dd3290297 100644 --- a/backend/routes/integrations.ts +++ b/backend/routes/integrations.ts @@ -493,6 +493,50 @@ router.get('/:podId', listIntegrationsRateLimit, auth, async (req: AuthReq, res: } }); +// F161: "Add the bot again" on a Discord row that dropped starts a fresh +// install, and the bind below writes a new row. The caller's own dropped rows +// on the same pod, guild and channel are then retired the way Remove retires a +// row: grants revoked, webhook deleted, platform record gone. The row itself is +// kept, inactive, rather than deleted. Best effort: a failure here leaves a +// stale row, never a failed bind. +const retireDroppedDiscordRows = async ( + keepId: unknown, podId: unknown, userId: string, serverId: string, channelId: string, +): Promise => { + // Every value comes from the request path: pin each to the shape it must + // have, so the filter can only ever hold plain strings. + const guild = String(serverId); + const channel = String(channelId); + const pod = String(podId); + if (!/^\d{17,20}$/.test(guild) || !/^\d{17,20}$/.test(channel) || !/^[a-f0-9]{24}$/i.test(pod)) return; + let dropped: Array<{ _id: unknown }> = []; + try { + dropped = await Integration.find({ + _id: { $ne: String(keepId) }, + type: 'discord', + isActive: true, + podId: pod, + createdBy: String(userId), + 'config.serverId': guild, + 'config.channelId': channel, + status: { $in: ['error', 'disconnected'] }, + }) as Array<{ _id: unknown }>; + } catch (error) { + console.warn(`Discord rebind: could not look up dropped rows: ${(error as Error).message}`); + return; + } + await Promise.all(dropped.map(async (row) => { + try { + await revokeConnectionGrants({ connection: row, revokedBy: userId }); + const service = new DiscordService(row._id); + if (await service.initialize()) await service.disconnect(); + await DiscordIntegration.findOneAndDelete({ integrationId: row._id }); + await Integration.findByIdAndUpdate(row._id, { $set: { isActive: false, status: 'disconnected' } }); + } catch (error) { + console.warn(`Discord rebind: could not retire integration ${String(row._id)}: ${(error as Error).message}`); + } + })); +}; + router.post('/', writeIntegrationsRateLimit, auth, async (req: AuthReq, res: Res) => { try { const { podId, type, config } = (req.body || {}) as { podId?: string; type?: unknown; config?: Record }; @@ -654,6 +698,9 @@ router.post('/', writeIntegrationsRateLimit, auth, async (req: AuthReq, res: Res if (!initialized) return res.status(500).json({ message: 'Failed to initialize integration' }); const connected = await service.connect(); if (!connected) console.warn('Integration initialized but failed to connect'); + await retireDroppedDiscordRows( + integration._id, podId, String(req.user?.id), String(config.serverId), String(config.channelId), + ); } res.status(201).json({ integration, platformIntegration }); } catch (error) { diff --git a/backend/routes/webhooks/slack.ts b/backend/routes/webhooks/slack.ts index 6d7996bbcc..44108c3807 100644 --- a/backend/routes/webhooks/slack.ts +++ b/backend/routes/webhooks/slack.ts @@ -156,10 +156,11 @@ const commandHelp = 'Use /commonly status, mode mirror|attention, mute [minutes] // with a subcommand. The subcommand is case-insensitive, the argument is taken // as typed, and anything unrecognised — an empty text included — is help. const parseSlackRelayCommand = (text: unknown): RelayCommand => { - const [command, argument] = String(text || '').trim().split(/\s+/, 2); + const [command, argument, ...rest] = String(text || '').trim().split(/\s+/); const normalized = command?.toLowerCase() || 'help'; if (normalized === 'mode') return { name: 'mode', arg: argument }; - if (normalized === 'mute') return { name: 'mute', arg: argument }; + // /mute reads its whole argument (`mute 2 hours`); the others read one word (F161). + if (normalized === 'mute') return { name: 'mute', arg: [argument, ...rest].filter(Boolean).join(' ') || undefined }; if (normalized === 'unmute') return { name: 'unmute' }; if (normalized === 'status') return { name: 'status' }; return { name: 'help' }; @@ -177,6 +178,8 @@ const slackCommandReply = (outcome: RelayCommandOutcome): string => { return `Relay mode: ${outcome.mode}.`; case 'mute': return `Muted for ${outcome.minutes} minutes.`; + case 'mute-usage': + return 'Usage: /commonly mute 30m|2h'; case 'unmute': return 'Relay unmuted.'; case 'status': diff --git a/backend/routes/webhooks/telegram.ts b/backend/routes/webhooks/telegram.ts index 18f9f9f7e7..575e8074a5 100644 --- a/backend/routes/webhooks/telegram.ts +++ b/backend/routes/webhooks/telegram.ts @@ -401,7 +401,7 @@ const HELP_TEXT = [ ' • mirror: every agent message in the pod is copied here', ' • attention: only escalations and the lead agent reach you', '/status — pod, mode, and mute state for this chat', - '/mute [minutes] — pause relay to this chat (default 60); /unmute to resume', + '/mute [30m | 2h] — pause relay to this chat (default 60 minutes; a bare number is minutes); /unmute to resume', '/tldr — latest pod summary', '/summary — summarize recent chat here into the pod', ].join('\n'); @@ -425,6 +425,8 @@ const telegramCommandReply = (outcome: RelayCommandOutcome, podName?: string): s : 'Mode: attention — only escalations and the lead agent reach you.'; case 'mute': return `Muted for ${outcome.minutes} min — nothing relays here until then. /unmute to resume.`; + case 'mute-usage': + return 'Usage: /mute 30m or /mute 2h (a bare number is minutes, at most 24 hours)'; case 'unmute': return 'Unmuted — relay resumes.'; case 'status': { @@ -524,7 +526,8 @@ router.post('/', telegramWebhookRateLimit, async (req: any, res: any) => { return res.sendStatus(200); } - const relayCommand = parseTelegramRelayCommand(command, args[0]); + // /mute reads its whole argument (`/mute 2 hours`); /mode reads one word. + const relayCommand = parseTelegramRelayCommand(command, command === MUTE_COMMAND ? args.join(' ') : args[0]); if (relayCommand) { await handleRelayCommand(chat, integration, relayCommand); return res.sendStatus(200); diff --git a/backend/scripts/deploy-discord-commands.js b/backend/scripts/deploy-discord-commands.js index 02b7ce6331..d821930bc6 100644 --- a/backend/scripts/deploy-discord-commands.js +++ b/backend/scripts/deploy-discord-commands.js @@ -1,6 +1,23 @@ const axios = require("axios"); const DISCORD_APPLICATION_COMMANDS = require("../config/discordApplicationCommands.json"); +// Each registration attempt, request and response together, ends by this. +const ATTEMPT_DEADLINE_MS = 15_000; + +// The parts of a command that change what Discord shows: where it is offered +// (`contexts`) and its options. Fields Discord adds itself (id, version, +// dm_permission, integration_types, ...) are not compared. +const optionShape = (options) => (options || []).map((o) => ({ + name: o.name, + type: o.type, + required: Boolean(o.required), + choices: (o.choices || []).map((c) => c.value), +})); +const commandShape = (cmd) => JSON.stringify({ + contexts: cmd.contexts || null, + options: optionShape(cmd.options), +}); + /** * Discord Command Deployment Script * Registers global slash commands with Discord API @@ -117,12 +134,25 @@ class DiscordCommandDeployment { try { const url = `https://discord.com/api/v10/applications/${clientId}/commands`; - const response = await axios.put(url, commands, { - headers: { - Authorization: `Bot ${botToken}`, - "Content-Type": "application/json", - }, - }); + // Bounded: this runs at boot (start.sh). axios's `timeout` covers the + // wait for headers only, so a body that trickles forever would still + // hold the server's start; the abort is a wall-clock deadline over the + // whole request and response. + const controller = new AbortController(); + const deadline = setTimeout(() => controller.abort(), ATTEMPT_DEADLINE_MS); + let response; + try { + response = await axios.put(url, commands, { + headers: { + Authorization: `Bot ${botToken}`, + "Content-Type": "application/json", + }, + timeout: 10_000, + signal: controller.signal, + }); + } finally { + clearTimeout(deadline); + } if (response.status === 200 || response.status === 201) { console.log(`✅ Discord API returned status ${response.status}`); @@ -208,17 +238,26 @@ class DiscordCommandDeployment { const missingCommands = expectedCommands.filter( (cmd) => !foundCommands.includes(cmd), ); - - if (missingCommands.length === 0) { + // Present but registered from an older file: a deploy is still due. + const staleCommands = DISCORD_APPLICATION_COMMANDS + .filter((cmd) => { + const found = registeredCommands.find((r) => r.name === cmd.name); + return found && commandShape(found) !== commandShape(cmd); + }) + .map((cmd) => cmd.name); + + if (missingCommands.length === 0 && staleCommands.length === 0) { console.log( `✅ All ${expectedCommands.length} commands verified globally`, ); return { success: true, registeredCommands: foundCommands }; } - console.log(`❌ Missing commands: ${missingCommands.join(", ")}`); + if (missingCommands.length) console.log(`❌ Missing commands: ${missingCommands.join(", ")}`); + if (staleCommands.length) console.log(`❌ Stale commands (contexts or options differ): ${staleCommands.join(", ")}`); return { success: false, missingCommands, + staleCommands, registeredCommands: foundCommands, }; } @@ -233,18 +272,23 @@ class DiscordCommandDeployment { /** * CLI interface */ -async function main() { - const args = process.argv.slice(2); +async function main(args = process.argv.slice(2)) { const command = args[0]; const deployment = new DiscordCommandDeployment(); + // A failure is an exit code, so start.sh can say so. start.sh does not + // stop the boot on it. + const failIf = (result) => { + if (!result || !result.success) process.exitCode = 1; + }; + switch (command) { case "deploy": - await deployment.deploy(); + failIf(await deployment.deploy()); break; case "verify": - await deployment.verifyDeployment(); + failIf(await deployment.verifyDeployment()); break; case "help": console.log("Discord Command Deployment Script"); @@ -267,13 +311,17 @@ async function main() { break; default: // Default: deploy - await deployment.deploy(); + failIf(await deployment.deploy()); } } // Run if called directly if (require.main === module) { - main().catch(console.error); + main().catch((error) => { + console.error(error); + process.exitCode = 1; + }); } module.exports = DiscordCommandDeployment; +module.exports.main = main; diff --git a/backend/services/chatRelay/chatRelayCore.ts b/backend/services/chatRelay/chatRelayCore.ts index 2247c77111..7bac4e7f4f 100644 --- a/backend/services/chatRelay/chatRelayCore.ts +++ b/backend/services/chatRelay/chatRelayCore.ts @@ -114,6 +114,8 @@ export type RelayCommandOutcome = lead: string | null; } | { kind: 'mute'; minutes: number; until: Date; update: RelayConfigUpdate } + /** A `/mute` duration the parse cannot read (`90s`, `1.5h`): nothing is written. */ + | { kind: 'mute-usage' } | { kind: 'unmute'; update: RelayConfigUpdate } /** A settings change from someone the platform could not prove may make it. */ | { kind: 'refused'; command: RelayCommand['name'] }; @@ -143,17 +145,18 @@ const MUTE_DURATION = /^(\d{1,5}) ?(m|min|mins|minutes?|h|hr|hrs|hours?)?$/i; const MUTE_ARG_MAX = 16; /** - * The minutes a `/mute` argument asks for: a bare number is minutes, and `30m` - * or `2h` carry their unit. Anything else falls back to parseInt, as before - * units were read, so `2h` no longer means two minutes but `soon` still means - * the default. + * The minutes a `/mute` argument asks for: none means the default, a bare + * number is minutes, and `30m` or `2h` carry their unit. Anything else is null, + * which the caller answers with usage: a guess at `90s` or `1.5h` would mute + * for a time nobody asked for (F161). */ -const muteMinutesOf = (arg?: string): number => { - const trimmed = String(arg || '').trim().slice(0, MUTE_ARG_MAX); - const match = MUTE_DURATION.exec(trimmed); - const asked = match - ? parseInt(match[1], 10) * (match[2] && /^h/i.test(match[2]) ? 60 : 1) - : parseInt(trimmed || String(DEFAULT_MUTE_MINUTES), 10); +const muteMinutesOf = (arg?: string): number | null => { + const raw = String(arg || '').trim(); + if (!raw) return DEFAULT_MUTE_MINUTES; + if (raw.length > MUTE_ARG_MAX) return null; + const match = MUTE_DURATION.exec(raw); + if (!match) return null; + const asked = parseInt(match[1], 10) * (match[2] && /^h/i.test(match[2]) ? 60 : 1); return Math.min(Math.max(asked || DEFAULT_MUTE_MINUTES, 1), MAX_MUTE_MINUTES); }; @@ -185,6 +188,7 @@ const decideRelayCommand = ( } if (command.name === 'mute') { const minutes = muteMinutesOf(command.arg); + if (minutes === null) return { kind: 'mute-usage' }; const until = new Date(now.getTime() + minutes * 60_000); return { kind: 'mute', minutes, until, update: { $set: { 'config.relayMutedUntil': until } } }; } diff --git a/backend/services/discordCommandService.ts b/backend/services/discordCommandService.ts index ef70a7a849..ec1a181b29 100644 --- a/backend/services/discordCommandService.ts +++ b/backend/services/discordCommandService.ts @@ -59,6 +59,7 @@ interface IntegrationDoc { serverId?: string; channelId?: string; webhookListenerEnabled?: boolean; + discordUserId?: string; }; populate(field: string, select: string): Promise; } @@ -74,6 +75,27 @@ interface DiscordSummaryData { summaryType: string; } +/** + * F161: channel sync changes what a SHARED channel feeds into the pod, so the + * toggles take the bridge commands' rule (discordRelayCommandService + * `mayChangeRelaySettings`): only the installer, matched by the Discord id the + * bot-add proved. A row bound before that proof existed records no id and + * admits no one; adding the bot again stamps it. Null means allowed. + */ +const syncToggleRefusal = (integration: IntegrationDoc, actorId?: string): CommandResult | null => { + // eslint-disable-next-line @typescript-eslint/no-require-imports, global-require + const { mayChangeRelaySettings } = require('./discordRelayCommandService'); + if (mayChangeRelaySettings(integration, String(actorId || ''))) return null; + // No `<@id>` here: the legacy reply carries no allowed_mentions guard. + return { + success: false, + content: integration.config?.discordUserId + ? 'Only the person who connected this channel to Commonly can turn channel sync on or off.' + : 'Only the person who connected this channel to Commonly can turn channel sync on or off, and this ' + + "connection doesn't record their Discord account. They can add the bot again from Commonly to fix that.", + }; +}; + class DiscordCommandService { private guildId: string | null; @@ -277,7 +299,7 @@ class DiscordCommandService { } } - async handleEnableCommand(): Promise { + async handleEnableCommand(actorId?: string): Promise { try { if (!this.integration) { await this.initialize(); @@ -287,6 +309,9 @@ class DiscordCommandService { return { success: false, content: '❌ Discord integration not found.' }; } + const refusal = syncToggleRefusal(this.integration, actorId); + if (refusal) return refusal; + await Integration.findByIdAndUpdate(this.integration._id, { 'config.webhookListenerEnabled': true, }); @@ -301,7 +326,7 @@ class DiscordCommandService { } } - async handleDisableCommand(): Promise { + async handleDisableCommand(actorId?: string): Promise { try { if (!this.integration) { await this.initialize(); @@ -311,6 +336,9 @@ class DiscordCommandService { return { success: false, content: '❌ Discord integration not found.' }; } + const refusal = syncToggleRefusal(this.integration, actorId); + if (refusal) return refusal; + await Integration.findByIdAndUpdate(this.integration._id, { 'config.webhookListenerEnabled': false, }); diff --git a/backend/services/discordMultiCommandService.ts b/backend/services/discordMultiCommandService.ts index a4412d5729..bf326cfcac 100644 --- a/backend/services/discordMultiCommandService.ts +++ b/backend/services/discordMultiCommandService.ts @@ -26,6 +26,8 @@ interface RunCommandOptions { integration: Integration; guildId?: string; channelId?: string; + /** The caller's Discord id; the sync toggles admit only the installer (F161). */ + actorId?: string; } interface RunMultiCommandOptions { @@ -33,6 +35,7 @@ interface RunMultiCommandOptions { integrations: Integration[]; guildId?: string; channelId?: string; + actorId?: string; } async function runCommandForIntegration({ @@ -40,6 +43,7 @@ async function runCommandForIntegration({ integration, guildId, channelId, + actorId, }: RunCommandOptions): Promise { if (commandName === 'discord-push') { const discordService = new DiscordService(integration._id); @@ -94,12 +98,12 @@ async function runCommandForIntegration({ case 'discord-enable': return { integration, - result: await commandService.handleEnableCommand(), + result: await commandService.handleEnableCommand(actorId), }; case 'discord-disable': return { integration, - result: await commandService.handleDisableCommand(), + result: await commandService.handleDisableCommand(actorId), }; default: return { @@ -117,6 +121,7 @@ async function runDiscordCommandForIntegrations({ integrations, guildId, channelId, + actorId, }: RunMultiCommandOptions): Promise { const results = await Promise.all( integrations.map((integration) => runCommandForIntegration({ @@ -124,6 +129,7 @@ async function runDiscordCommandForIntegrations({ integration, guildId, channelId, + actorId, })), ); diff --git a/backend/services/discordRelayCommandService.ts b/backend/services/discordRelayCommandService.ts index 33b44fae9a..d6a4d3d563 100644 --- a/backend/services/discordRelayCommandService.ts +++ b/backend/services/discordRelayCommandService.ts @@ -127,6 +127,13 @@ const optionValue = (interaction: DiscordInteraction, name: string): string | un }; const NOT_CONNECTED = "This channel isn't connected to a Commonly pod. Connect it from the pod's Connectors page."; +// A row bound here whose connection is not `connected` (F161): the relay's own +// rule (F132), so a command cannot reach a pod the bridge itself would not. +const NOT_ACTIVE = "This channel's connection to Commonly isn't active. Reconnect it from the pod's Connectors page."; +const SETTINGS_NOT_CONNECTED = "This channel isn't connected right now, so its relay settings can't change. " + + "Reconnect it from the pod's Connectors page."; + +const isConnected = (row: DiscordCommandRow): boolean => row.status === 'connected'; const HELP_TEXT = [ '**Commonly commands for this channel**', @@ -176,6 +183,8 @@ const discordCommandReply = (outcome: RelayCommandOutcome, row: DiscordCommandRo case 'mute': return `Muted for ${outcome.minutes} min. Nothing relays here until ${discordTime(outcome.until)}. ` + '/unmute to resume.'; + case 'mute-usage': + return 'Usage: /mute 30m or /mute 2h (a bare number is minutes, at most 24 hours)'; case 'unmute': return 'Unmuted. Relay resumes.'; case 'status': { @@ -237,8 +246,11 @@ const runSettingsCommand = async ( const outcome = applyRelayCommand(command, row, new Date(), { mayChangeSettings: mayChangeRelaySettings(row, actorId), }); - if ('update' in outcome) await Integration.findByIdAndUpdate(row._id, outcome.update); const podName = await podNameOf(row); + // Reads stay open on any bound row (/status names a dropped connection); + // a write waits for the row to be connected again. + if ('update' in outcome && !isConnected(row)) return { podName, text: SETTINGS_NOT_CONNECTED }; + if ('update' in outcome) await Integration.findByIdAndUpdate(row._id, outcome.update); return { podName, text: discordCommandReply(outcome, row, podName) }; })); return reply(joinPerPod(blocks)); @@ -294,34 +306,67 @@ const editDeferredReply = async (interaction: DiscordInteraction, content: strin ); }; +// /summary runs the summarizer, an LLM call, so one row runs it at most once a +// minute (F161). Claimed before the run, so two calls racing in the same +// second still make one. Per process: a second replica keeps its own clock, +// which at worst doubles the rate, never removes the limit. +export const SUMMARY_COOLDOWN_MS = 60_000; +const summaryRanAt = new Map(); + +const claimSummaryRun = (row: DiscordCommandRow): boolean => { + const key = String(row._id); + const now = Date.now(); + const last = summaryRanAt.get(key); + if (last !== undefined && now - last < SUMMARY_COOLDOWN_MS) return false; + summaryRanAt.set(key, now); + // Drop expired entries so the map stays the size of the last minute's rows. + summaryRanAt.forEach((at, id) => { if (now - at >= SUMMARY_COOLDOWN_MS) summaryRanAt.delete(id); }); + return true; +}; + +/** Test seam: forget every row's last /summary. */ +export const resetSummaryCooldowns = (): void => { summaryRanAt.clear(); }; + +const SUMMARY_COOLING = 'A summary ran here less than a minute ago. Try again shortly.'; + // /summary: the channel's buffered chat → a summary → Commonly Bot in the pod, // the path `/discord-push` has always taken. It calls the summarizer, so it // cannot promise Discord's 3 seconds and answers by deferring. -const deferSummary = (interaction: DiscordInteraction, rows: DiscordCommandRow[]): DiscordCommandHandled => ({ - response: { type: 5, data: { flags: EPHEMERAL } }, - after: async () => { - // eslint-disable-next-line @typescript-eslint/no-require-imports, global-require - const { runCommandForIntegration } = require('./discordMultiCommandService'); - const blocks = await Promise.all(rows.map(async (row) => { - const podName = await podNameOf(row); - if (!row.config?.webhookListenerEnabled) { - return { - podName, - text: 'Channel sync is off for this channel, so there is nothing buffered to summarize. ' - + 'Turn it on with /discord-enable.', - }; - } - try { - const { result } = await runCommandForIntegration({ commandName: 'discord-push', integration: row }); - return { podName, text: String(result?.content || '❌ The summary did not run.') }; - } catch (error) { - console.warn(`${LOG} /summary failed integration=${String(row._id)}: ${(error as Error).message}`); - return { podName, text: '❌ The summary did not run. Try again shortly.' }; - } - })); - await editDeferredReply(interaction, joinPerPod(blocks)); - }, -}); +const deferSummary = (interaction: DiscordInteraction, rows: DiscordCommandRow[]): DiscordCommandHandled => { + // Only rows with sync on run the summarizer, so only they take a slot. + const claimed = new Set(rows + .filter((row) => row.config?.webhookListenerEnabled && claimSummaryRun(row)) + .map((row) => String(row._id))); + if (!claimed.size && rows.some((row) => row.config?.webhookListenerEnabled)) { + return { response: reply(SUMMARY_COOLING) }; + } + return { + response: { type: 5, data: { flags: EPHEMERAL } }, + after: async () => { + // eslint-disable-next-line @typescript-eslint/no-require-imports, global-require + const { runCommandForIntegration } = require('./discordMultiCommandService'); + const blocks = await Promise.all(rows.map(async (row) => { + const podName = await podNameOf(row); + if (!row.config?.webhookListenerEnabled) { + return { + podName, + text: 'Channel sync is off for this channel, so there is nothing buffered to summarize. ' + + 'Turn it on with /discord-enable.', + }; + } + if (!claimed.has(String(row._id))) return { podName, text: SUMMARY_COOLING }; + try { + const { result } = await runCommandForIntegration({ commandName: 'discord-push', integration: row }); + return { podName, text: String(result?.content || '❌ The summary did not run.') }; + } catch (error) { + console.warn(`${LOG} /summary failed integration=${String(row._id)}: ${(error as Error).message}`); + return { podName, text: '❌ The summary did not run. Try again shortly.' }; + } + })); + await editDeferredReply(interaction, joinPerPod(blocks)); + }, + }; +}; /** * One bridge command → its interaction response. Only for names @@ -334,8 +379,12 @@ export const handleDiscordRelayCommand = async (interaction: DiscordInteraction) const channelId = String(interaction.channel_id || ''); const rows = await findBoundRows(guildId, channelId); if (!rows.length) return { response: reply(NOT_CONNECTED) }; - if (name === 'tldr') return { response: await runTldr(rows) }; - if (name === 'summary') return deferSummary(interaction, rows); + if (name === 'tldr' || name === 'summary') { + const connected = rows.filter(isConnected); + if (!connected.length) return { response: reply(NOT_ACTIVE) }; + if (name === 'tldr') return { response: await runTldr(connected) }; + return deferSummary(interaction, connected); + } return { response: await runSettingsCommand(interaction, rows) }; }; @@ -347,6 +396,8 @@ module.exports = { isBoundToChannel, mayChangeRelaySettings, findBoundRows, + resetSummaryCooldowns, + SUMMARY_COOLDOWN_MS, renderDiscordTldr, handleDiscordRelayCommand, }; diff --git a/backend/services/discordService.ts b/backend/services/discordService.ts index b27e201584..13899f1809 100644 --- a/backend/services/discordService.ts +++ b/backend/services/discordService.ts @@ -20,6 +20,8 @@ const { resolveDiscordWebhookUrl, hasDiscordWebhookUrl } = require('../utils/dis const connectorSecrets = require('./connectorSecrets'); // eslint-disable-next-line global-require const { DISCORD_WEBHOOK_URL } = require('./connectorSecretKinds'); +// eslint-disable-next-line global-require +const DISCORD_APPLICATION_COMMANDS = require('../config/discordApplicationCommands.json') as Array<{ name: string }>; interface FetchMessagesOptions { channelId?: string; @@ -127,6 +129,8 @@ interface VerifyCommandResult { success: boolean; registeredCommands?: string[]; missingCommands?: string[]; + /** Commands the guild holds that nothing expects there (F161). */ + unexpectedCommands?: string[]; totalExpected?: number; totalFound?: number; error?: string; @@ -156,6 +160,8 @@ class DiscordService { clientReady: boolean; commandService: unknown; integration: IntegrationDoc | null; + /** Why the last registerSlashCommands returned false, for the route to show. */ + lastRegistrationError: string | null = null; static async fetchMessages(options: FetchMessagesOptions = {}): Promise { const { @@ -821,7 +827,7 @@ class DiscordService { } } - async handleSlashCommand(commandName: string, _interaction: unknown): Promise { + async handleSlashCommand(commandName: string, interaction: unknown): Promise { try { if (!this.commandService) { return { @@ -833,11 +839,16 @@ class DiscordService { const cs = this.commandService as { handleSummaryCommand: () => Promise; handleStatusCommand: () => Promise; - handleEnableCommand: () => Promise; - handleDisableCommand: () => Promise; + handleEnableCommand: (actorId: string) => Promise; + handleDisableCommand: (actorId: string) => Promise; handlePushCommand: (service: DiscordService) => Promise; }; + // The caller's Discord id: `member.user` in a guild, `user` in a DM. The + // sync toggles check it against the installer's (F161). + const who = interaction as { member?: { user?: { id?: string } } | null; user?: { id?: string } | null } | null; + const actorId = String(who?.member?.user?.id || who?.user?.id || ''); + switch (commandName) { case 'commonly-summary': return await cs.handleSummaryCommand(); @@ -846,10 +857,10 @@ class DiscordService { return await cs.handleStatusCommand(); case 'discord-enable': - return await cs.handleEnableCommand(); + return await cs.handleEnableCommand(actorId); case 'discord-disable': - return await cs.handleDisableCommand(); + return await cs.handleDisableCommand(actorId); case 'discord-push': return await cs.handlePushCommand(this); @@ -882,28 +893,29 @@ class DiscordService { console.log(`🔧 Registering commands for guild: ${targetGuildId}`); - const commands = [ - { - name: 'commonly-summary', - description: 'Get the most recent summary from the linked chat pod', - type: 1, - }, - { - name: 'discord-status', - description: 'Show the status of Discord integration', - type: 1, - }, - { - name: 'discord-enable', - description: 'Enable webhook listener for Discord channel', - type: 1, - }, - { - name: 'discord-disable', - description: 'Disable webhook listener for Discord channel', - type: 1, - }, - ]; + // F161: every command, legacy and bridge, is registered globally at boot + // (scripts/deploy-discord-commands.js, config/discordApplicationCommands.json). + // A guild copy showed each legacy command twice in that guild's menu, so + // the guild set is empty, and this bulk overwrite clears the copies older + // registrations left behind. + const commands: Array<{ name: string }> = []; + + // Clearing the guild's copies is safe only while the global set is in + // place; after a boot deploy that failed, the copies are all the guild + // has. A read that fails counts as not in place. + const globalResponse = await axios.get( + `https://discord.com/api/v10/applications/${config.clientId}/commands`, + { headers: { Authorization: `Bot ${config.botToken}` }, timeout: 10_000 }, + ) as { status?: number; data?: Array<{ name?: string }> } | undefined; + const globalNames = new Set((globalResponse?.status === 200 && Array.isArray(globalResponse.data) + ? globalResponse.data : []).map((cmd) => cmd?.name)); + const missingGlobal = DISCORD_APPLICATION_COMMANDS + .map((cmd) => cmd.name) + .filter((name) => !globalNames.has(name)); + if (missingGlobal.length) { + throw new Error(`The global commands are not all registered (missing: ${missingGlobal.join(', ')}), ` + + "so this guild's commands were kept. Run npm run discord:deploy first."); + } const url = `https://discord.com/api/v10/applications/${config.clientId}/guilds/${targetGuildId}/commands`; @@ -933,6 +945,7 @@ class DiscordService { } catch (error) { const err = error as { message?: string }; console.error('❌ Failed to register commands:', err.message); + this.lastRegistrationError = err.message || 'Failed to register slash commands'; if (this.integration) { await Integration.findByIdAndUpdate(this.integrationId, { @@ -1071,20 +1084,19 @@ class DiscordService { if (response.status === 200) { const registeredCommands = response.data as Array<{ name: string }>; - const expectedCommands = [ - 'commonly-summary', - 'discord-status', - 'discord-enable', - 'discord-disable', - ]; + // Nothing is registered per guild any more (F161); see registerSlashCommands. + const expectedCommands: string[] = []; const foundCommands = registeredCommands.map((cmd) => cmd.name); const missingCommands = expectedCommands.filter( (cmd) => !foundCommands.includes(cmd), ); + // With nothing expected, a guild that still holds copies is the failure. + const unexpectedCommands = foundCommands.filter((cmd) => !expectedCommands.includes(cmd)); return { - success: missingCommands.length === 0, + success: missingCommands.length === 0 && unexpectedCommands.length === 0, + unexpectedCommands, registeredCommands: foundCommands, missingCommands, totalExpected: expectedCommands.length, diff --git a/docs/discord/DISCORD.md b/docs/discord/DISCORD.md index cc9e7fff8b..752a6fe6b3 100644 --- a/docs/discord/DISCORD.md +++ b/docs/discord/DISCORD.md @@ -77,7 +77,11 @@ Show the status of your Discord integration. ``` #### **`/discord-enable`** -Enable Discord webhook listener for activity aggregation. +Enable Discord webhook listener for activity aggregation. Only the person who +connected the channel may run it or `/discord-disable`: their Discord id must +equal the row's `config.discordUserId`, the rule the bridge commands use. A row +connected before that id was recorded admits no one until the bot is added +again from Commonly (F161). ``` /discord-enable @@ -319,7 +323,7 @@ These are the Discord routes currently served by Commonly: | `GET` | `/api/discord/callback` | Complete OAuth and redirect to the frontend. | | `POST` | `/api/webhooks/discord` | Receive Discord webhook deliveries and verify their signatures. | | `POST` | `/api/discord/interactions` | Receive signed slash-command interactions. | -| `POST` | `/api/discord/register-commands/:integrationId` | Register commands for one integration. | +| `POST` | `/api/discord/register-commands/:integrationId` | Clear the guild's per-guild command copies; every command is global (F161). | | `POST` | `/api/discord/register-all` | Register commands for all integrations; admin-only. | | `GET` | `/api/discord/health` | Check global command registration and health. | @@ -340,8 +344,13 @@ npm run discord:verify # verify the global command set ``` The deployer requires `DISCORD_CLIENT_ID` and `DISCORD_BOT_TOKEN`, retries a -failed Discord API request up to three times with a five-second delay, and -prints a registration report. It is not an automatic container-start hook. +failed Discord API request up to three times with a five-second delay and a +10-second timeout per request, prints a registration report, and exits 1 on +failure. `backend/start.sh` runs it at boot when the credentials are set and +starts the server whatever it returns, so a Discord outage delays boot by at +most about 45 seconds and never stops it. The set lives in +`backend/config/discordApplicationCommands.json`; the bridge commands carry +`contexts: [0]` (guild channels only). Nothing is registered per guild. For database-backed, integration-scoped registration and inspection: diff --git a/docs/discord/REGISTER_DISCORD_COMMANDS.md b/docs/discord/REGISTER_DISCORD_COMMANDS.md index d866380bf1..e8863c32d8 100644 --- a/docs/discord/REGISTER_DISCORD_COMMANDS.md +++ b/docs/discord/REGISTER_DISCORD_COMMANDS.md @@ -27,7 +27,31 @@ take up to an hour to show in every guild. need the caller's Discord id (`member.user.id`) to equal the row's `config.discordUserId`, the installer the OAuth callback proved. Everyone else gets an ephemeral refusal naming that person. Reads are open to anyone in the - bound channel. + bound channel. The legacy `/discord-enable` and `/discord-disable` take the + same rule (F161). +- **Connection state (F161).** `/tldr` and `/summary` act only on a row whose + `status` is `connected`, the relay's own rule. A settings write on a row that + isn't connected says so and writes nothing; `/status` still reads it. +- **Cost (F161).** `/summary` is an LLM run, so each row runs it at most once a + minute (per backend process); a second call inside that minute gets an + ephemeral "try again shortly". +- **Durations (F161).** `/mute` takes nothing (60 minutes), a bare number + (minutes), `30m` or `2h`. Anything else, such as `90s` or `1.5h`, gets usage + and mutes nothing, on Discord, Telegram and Slack alike. Telegram and Slack + read the whole argument, so `/mute 2 hours` is 120 minutes. +- **Per-guild registration (F161).** `POST /api/discord/register-commands/:id` + and `npm run discord:register` now register an empty per-guild set, which + clears the duplicate legacy copies older guild registrations left. They first + read the global set and keep the guild's commands if any global command is + missing (a boot deploy that failed). +- **Boot deploy (F161).** Each PUT attempt has a 15 s wall-clock deadline + (`AbortController`) over request and response; axios's own `timeout` stops + counting once headers arrive. `discord:verify` exits 1 when a command is + missing or registered with different `contexts` or options. +- **Reconnect (F161).** "Add the bot again" on a dropped row is a fresh install. + When it binds the same pod, guild and channel, the caller's rows there in + `error` or `disconnected` are retired as Remove would (grants revoked, webhook + deleted, platform record gone) and kept inactive. - **Binding.** A command acts only on rows whose `serverId` and `channelId` are the ones it was typed in. Unlike the legacy commands, it has no guild-wide fallback. - **Scope.** The install link asks for `applications.commands` already. A guild diff --git a/frontend/src/i18n/locales/en.json b/frontend/src/i18n/locales/en.json index d847b076b4..03917ee57d 100644 --- a/frontend/src/i18n/locales/en.json +++ b/frontend/src/i18n/locales/en.json @@ -2666,7 +2666,8 @@ "discordMirrorHint": "Every agent message is posted in the Discord channel, where everyone in the server can read it.", "discordAttentionHint": "Only escalations and the lead agent are posted in the Discord channel, where everyone in the server can read them.", "discordInboundNote": "Your own messages in the channel reach the pod as you.", - "discordNeedsProof": "Add the bot again from Commonly to turn on relay for this channel." + "discordNeedsProof": "Add the bot again from Commonly to turn on relay for this channel.", + "discordReinstall": "Add the bot again" }, "artifacts": { "title": "Artifacts", diff --git a/frontend/src/i18n/locales/zh-CN.json b/frontend/src/i18n/locales/zh-CN.json index 966716805b..5505ecd590 100644 --- a/frontend/src/i18n/locales/zh-CN.json +++ b/frontend/src/i18n/locales/zh-CN.json @@ -2667,7 +2667,8 @@ "discordMirrorHint": "每条智能体消息都会发到这个 Discord 频道,服务器里的每个人都能看到。", "discordAttentionHint": "只有需要关注的消息和主导智能体的消息会发到这个 Discord 频道,服务器里的每个人都能看到。", "discordInboundNote": "你在频道里发的消息,会以你的身份进入 Pod。", - "discordNeedsProof": "从 Commonly 重新添加一次机器人,就能为这个频道开启中继。" + "discordNeedsProof": "从 Commonly 重新添加一次机器人,就能为这个频道开启中继。", + "discordReinstall": "重新添加机器人" }, "artifacts": { "title": "产物", diff --git a/frontend/src/v2/__tests__/ConnectorsPage.channels.test.tsx b/frontend/src/v2/__tests__/ConnectorsPage.channels.test.tsx index 32b61bf586..9ea6d36d25 100644 --- a/frontend/src/v2/__tests__/ConnectorsPage.channels.test.tsx +++ b/frontend/src/v2/__tests__/ConnectorsPage.channels.test.tsx @@ -1809,6 +1809,39 @@ describe('Connectors page: Discord live relay (F132)', () => { expect(axios.patch).not.toHaveBeenCalled(); }); + // F161 item 10: a Discord row in error said "reconnect to resume" with nothing + // to press. Reconnecting a Discord channel is adding the bot again, through + // the same install link the Connect flow uses. + it('offers a Discord row in error the install link again, and the button leaves for it', async () => { + const realLocation = window.location; + const assign = jest.fn(); + Object.defineProperty(window, 'location', { configurable: true, value: { ...realLocation, assign } }); + try { + const INSTALL_URL = 'https://discord.com/api/oauth2/authorize?client_id=1&response_type=code'; + mockApi({ + connectors: [discordRow({}, { status: 'error' })], + route: (url: string) => (url === '/api/discord/install-link/p2' + ? Promise.resolve({ data: { installUrl: INSTALL_URL, podId: 'p2', status: 'ready' } }) + : undefined), + }); + renderPage(); + const drawer = await openDrawer('Discord'); + expect(within(drawer).getByText('The connection dropped.')).toBeInTheDocument(); + fireEvent.click(within(drawer).getByRole('button', { name: 'Add the bot again' })); + await waitFor(() => expect(assign).toHaveBeenCalledWith(INSTALL_URL)); + expect(axios.get).toHaveBeenCalledWith('/api/discord/install-link/p2', expect.anything()); + } finally { + Object.defineProperty(window, 'location', { configurable: true, value: realLocation }); + } + }); + + it('has the reconnect label in en and zh-CN', () => { + // eslint-disable-next-line global-require + const zh = require('../../i18n/locales/zh-CN.json'); + expect(en.connectors.discordReinstall).toBe('Add the bot again'); + expect(zh.connectors.discordReinstall).toBe('重新添加机器人'); + }); + // CodeRabbit (body, minor): attention mode also sends every lead-agent line. // The row mark has no renderer on the v2 page, so the copy is asserted where // it lives, in both locales. diff --git a/frontend/src/v2/connectors/channelModel.ts b/frontend/src/v2/connectors/channelModel.ts index e3130c529e..831048625b 100644 --- a/frontend/src/v2/connectors/channelModel.ts +++ b/frontend/src/v2/connectors/channelModel.ts @@ -127,7 +127,7 @@ export interface SlackAuthorizeResponse { export type ConnectorAction = | 'manage' | 'show-code' | 'new-code' | 'authorize' | 'confirm' - | 'connect' | 'cancel' | 'retry' | 'retry-remove' | 'pick-pod'; + | 'connect' | 'cancel' | 'retry' | 'retry-remove' | 'pick-pod' | 'reinstall'; export interface ConnectorRow { action: ConnectorAction | null; diff --git a/frontend/src/v2/connectors/useChannelConnectors.ts b/frontend/src/v2/connectors/useChannelConnectors.ts index 467b01d7df..6e83a88cad 100644 --- a/frontend/src/v2/connectors/useChannelConnectors.ts +++ b/frontend/src/v2/connectors/useChannelConnectors.ts @@ -705,9 +705,24 @@ export const useChannelConnectors = () => { const title = connector.config?.chatTitle || connector.config?.teamName || TYPE_LABELS[connector.type] || connector.type; if (connector.status === 'error') { + // F161: reconnecting a Discord channel is adding the bot again, through + // the install link the Connect flow uses. + const isDiscordRow = connector.type === 'discord'; + let errorAction: ConnectorAction | null = null; + let errorActionLabel = ''; + if (isTelegram) { + errorAction = 'new-code'; + errorActionLabel = t('connectors.newCode', { defaultValue: 'New code' }); + } else if (isSlack) { + errorAction = 'authorize'; + errorActionLabel = t('connectors.slackAuthorize', { defaultValue: 'Authorize in Slack' }); + } else if (isDiscordRow && connectorPodId(connector)) { + errorAction = 'reinstall'; + errorActionLabel = t('connectors.discordReinstall', { defaultValue: 'Add the bot again' }); + } return { - action: isTelegram ? 'new-code' : isSlack ? 'authorize' : null, - actionLabel: isTelegram ? t('connectors.newCode', { defaultValue: 'New code' }) : t('connectors.slackAuthorize', { defaultValue: 'Authorize in Slack' }), + action: errorAction, + actionLabel: errorActionLabel, detail: t('connectors.errorReconnect', { defaultValue: 'reconnect to resume' }), dot: 'empty', line: connector.errorMessageUserFacing && connector.errorMessage @@ -1073,6 +1088,10 @@ export const useChannelConnectors = () => { if (!connector) return undefined; if (action === 'new-code') return regenerateCode(connector); if (action === 'authorize') return authorizeSlack(connector, item.key); + if (action === 'reinstall') { + const podId = connectorPodId(connector); + return podId ? linkInstall(connector.type, String(podId)) : undefined; + } // The row opens the aside where the active pod's lead can be chosen. // Sending a confirm here would silently omit that choice. if (action === 'confirm') return undefined;