diff --git a/src/discord-bridge.ts b/src/discord-bridge.ts index c20012f3..7118a698 100644 --- a/src/discord-bridge.ts +++ b/src/discord-bridge.ts @@ -286,6 +286,9 @@ function coerce(raw: unknown): DiscordConfig { }; } +/** A failure with a reason worth showing as it is (unlike a network error, whose text is not). */ +class BridgeProblem extends Error {} + export async function readDiscordConfig(configDir: string): Promise { try { return coerce(JSON.parse(await fs.readFile(discordConfigPath(configDir), 'utf-8'))); @@ -674,6 +677,8 @@ export class DiscordBridge { private threads: Record = {}; private loaded = false; private gateway: DiscordGateway | null = null; + /** The token the running gateway was started with ('' when none). */ + private gatewayToken = ''; private lastSent = new Map(); private lastReply = new Map(); /** The last name seen per session, so only a real rename touches the thread. */ @@ -762,10 +767,14 @@ export class DiscordBridge { this.threadsParent = parent; await this.persistThreads(); } + activeStartupHint = this.cfg.enabled && this.cfg.token && this.cfg.startupHint ? DISCORD_STARTUP_HINT : undefined; + // A save that changed neither the switch nor the token keeps the gateway session, so it can resume. + const want = this.cfg.enabled && this.cfg.token && !this.held ? this.cfg.token : ''; + if (this.gateway && want && want === this.gatewayToken) return; this.gateway?.stop(); this.gateway = null; - activeStartupHint = this.cfg.enabled && this.cfg.token && this.cfg.startupHint ? DISCORD_STARTUP_HINT : undefined; - if (this.cfg.enabled && this.cfg.token && !this.held) { + this.gatewayToken = want; + if (want) { this.gateway = new DiscordGateway( () => this.cfg.token, (m) => void this.onGatewayMessage(m).catch(() => undefined), @@ -779,6 +788,7 @@ export class DiscordBridge { stop(): void { this.gateway?.stop(); this.gateway = null; + this.gatewayToken = ''; activeStartupHint = undefined; this.lastSent.clear(); this.lastReply.clear(); @@ -799,9 +809,11 @@ export class DiscordBridge { ? 'Channel not found (check the IDs)' : status ? `HTTP ${status}` - : (err as { name?: string })?.name === 'TimeoutError' - ? 'Timed out' - : 'Network error'; + : err instanceof BridgeProblem + ? err.message + : (err as { name?: string })?.name === 'TimeoutError' + ? 'Timed out' + : 'Network error'; return { ok: false, status, error, at: this.now() }; } @@ -823,12 +835,12 @@ export class DiscordBridge { const p = (mine.p = (async () => { try { const t = await this.api.createThread(parent, threadNameFor(name, sessionId)); - if (!t?.id) return this.fail(new Error('Discord did not return a thread')); + if (!t?.id) return this.fail(new BridgeProblem('Discord did not return a thread')); // A save moved the bridge to another channel meanwhile: this thread belongs to the old one. - if (parent !== this.cfg.parentChannelId) return this.fail(new Error('The parent channel changed')); + if (parent !== this.cfg.parentChannelId) return this.fail(new BridgeProblem('The parent channel changed')); if (this.closedWhilePending.has(sessionId)) { await this.closeThread(t.id); - return this.fail(new Error('The tab was closed')); + return this.fail(new BridgeProblem('The tab was closed')); } this.threads[sessionId] = t.id; await this.persistThreads(); diff --git a/src/web/routes/discord-routes.ts b/src/web/routes/discord-routes.ts index 1eb7a90b..10669ebd 100644 --- a/src/web/routes/discord-routes.ts +++ b/src/web/routes/discord-routes.ts @@ -77,6 +77,12 @@ function consumeNotifyToken(sessionId: string, now: number = Date.now()): boolea return true; } +/** Give back a token spent on a request that was refused before anything was posted. */ +function refundNotifyToken(sessionId: string): void { + const b = notifyBuckets.get(sessionId); + if (b) b.tokens = Math.min(NOTIFY_PER_MINUTE, b.tokens + 1); +} + /** Test hook: forget every bucket between runs. */ export function _resetNotifyBuckets(): void { notifyBuckets.clear(); @@ -197,6 +203,7 @@ export function registerDiscordRoutes(app: FastifyInstance, deps: DiscordRouteDe if (file !== undefined) { const problem = await fileProblem(session.workingDir, !!session.remote, file); if (typeof problem === 'string') { + refundNotifyToken(session.id); reply.code(400); return createErrorResponse(ApiErrorCode.INVALID_INPUT, problem); } diff --git a/test/discord-bridge.test.ts b/test/discord-bridge.test.ts index 3dc483b2..222eb9b6 100644 --- a/test/discord-bridge.test.ts +++ b/test/discord-bridge.test.ts @@ -582,7 +582,20 @@ describe('DiscordBridge', () => { it('names a thread that Discord answers without an id, instead of reporting an old result', async () => { const b = bridge({}, async (url) => (String(url).endsWith('/threads') ? jsonRes({}) : jsonRes({ id: 'msg' }))); const r = await b.notify(MSG); - expect(r).toMatchObject({ ok: false }); + expect(r).toMatchObject({ ok: false, error: 'Discord did not return a thread' }); + b.stop(); + }); + + it('keeps the gateway session across a save that changes neither the switch nor the token', async () => { + const b = bridge(); + await b.reload(); + const first = (b as unknown as { gateway: unknown }).gateway; + expect(first).not.toBeNull(); + await b.reload(); + expect((b as unknown as { gateway: unknown }).gateway).toBe(first); + await writeDiscordConfig(dir, { ...CFG, token: 'another-token' }); + await b.reload(); + expect((b as unknown as { gateway: unknown }).gateway).not.toBe(first); b.stop(); }); @@ -692,8 +705,9 @@ describe('DiscordBridge', () => { await vi.waitFor(() => expect(posts.some((p) => p.path.endsWith('/threads'))).toBe(true)); await b.sessionDeleted('s1'); release(); - await first; + const out = await first; expect(posts).toContainEqual({ path: `/channels/${THREAD}`, method: 'PATCH', body: { archived: true } }); + expect(out).toMatchObject({ ok: false, error: 'The tab was closed' }); expect(await readThreadMap(dir)).toEqual({}); b.stop(); }); diff --git a/test/routes/discord-routes.test.ts b/test/routes/discord-routes.test.ts index f64bd4a0..cae33123 100644 --- a/test/routes/discord-routes.test.ts +++ b/test/routes/discord-routes.test.ts @@ -216,6 +216,17 @@ describe('POST /api/sessions/:id/discord-notify', () => { bridge.stop(); }); + it('does not spend rate-limit tokens on a request refused for its file', async () => { + const { app, bridge } = await harness(); + await app.inject({ method: 'PUT', url: '/api/discord', payload: FULL }); + for (let i = 0; i < 8; i++) { + const bad = await app.inject({ method: 'POST', url, payload: { message: 'x', file: 'no-such-file.txt' } }); + expect(bad.statusCode).toBe(400); + } + expect((await app.inject({ method: 'POST', url, payload: { message: 'still allowed' } })).statusCode).toBe(200); + bridge.stop(); + }); + it('rejects an empty or oversized message', async () => { const { app } = await harness(); const url = `/api/sessions/test-session-1/discord-notify`;