From 409a02691f26c67b696d5b56e390651aaa7fe33f Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 22 Feb 2026 17:54:30 +0000 Subject: [PATCH] refactor(discord): dedupe directory and media send paths --- src/discord/directory-live.test.ts | 118 ++++++++++++++++++ src/discord/directory-live.ts | 35 ++++-- src/discord/monitor/reply-delivery.ts | 56 ++++++--- src/discord/send.components.ts | 20 +-- src/discord/send.outbound.ts | 10 +- src/discord/send.permissions.ts | 43 +++++-- .../send.sends-basic-channel-messages.test.ts | 30 +++-- src/discord/send.shared.ts | 32 +++-- 8 files changed, 253 insertions(+), 91 deletions(-) create mode 100644 src/discord/directory-live.test.ts diff --git a/src/discord/directory-live.test.ts b/src/discord/directory-live.test.ts new file mode 100644 index 000000000..e6f19d448 --- /dev/null +++ b/src/discord/directory-live.test.ts @@ -0,0 +1,118 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { DirectoryConfigParams } from "../channels/plugins/directory-config.js"; + +const mocks = vi.hoisted(() => ({ + fetchDiscord: vi.fn(), + normalizeDiscordToken: vi.fn((token: string) => token.trim()), + resolveDiscordAccount: vi.fn(), +})); + +vi.mock("./accounts.js", () => ({ + resolveDiscordAccount: mocks.resolveDiscordAccount, +})); + +vi.mock("./api.js", () => ({ + fetchDiscord: mocks.fetchDiscord, +})); + +vi.mock("./token.js", () => ({ + normalizeDiscordToken: mocks.normalizeDiscordToken, +})); + +import { listDiscordDirectoryGroupsLive, listDiscordDirectoryPeersLive } from "./directory-live.js"; + +function makeParams(overrides: Partial = {}): DirectoryConfigParams { + return { + cfg: {} as DirectoryConfigParams["cfg"], + ...overrides, + }; +} + +describe("discord directory live lookups", () => { + beforeEach(() => { + vi.clearAllMocks(); + mocks.resolveDiscordAccount.mockReturnValue({ token: "test-token" }); + mocks.normalizeDiscordToken.mockImplementation((token: string) => token.trim()); + }); + + it("returns empty group directory when token is missing", async () => { + mocks.normalizeDiscordToken.mockReturnValue(""); + + const rows = await listDiscordDirectoryGroupsLive(makeParams({ query: "general" })); + + expect(rows).toEqual([]); + expect(mocks.fetchDiscord).not.toHaveBeenCalled(); + }); + + it("returns empty peer directory without query and skips guild listing", async () => { + const rows = await listDiscordDirectoryPeersLive(makeParams({ query: " " })); + + expect(rows).toEqual([]); + expect(mocks.fetchDiscord).not.toHaveBeenCalled(); + }); + + it("filters group channels by query and respects limit", async () => { + mocks.fetchDiscord.mockImplementation(async (path: string) => { + if (path === "/users/@me/guilds") { + return [ + { id: "g1", name: "Guild 1" }, + { id: "g2", name: "Guild 2" }, + ]; + } + if (path === "/guilds/g1/channels") { + return [ + { id: "c1", name: "general" }, + { id: "c2", name: "random" }, + ]; + } + if (path === "/guilds/g2/channels") { + return [{ id: "c3", name: "announcements" }]; + } + return []; + }); + + const rows = await listDiscordDirectoryGroupsLive(makeParams({ query: "an", limit: 2 })); + + expect(rows).toEqual([ + expect.objectContaining({ kind: "group", id: "channel:c2", name: "random" }), + expect.objectContaining({ kind: "group", id: "channel:c3", name: "announcements" }), + ]); + }); + + it("returns ranked peer results and caps member search by limit", async () => { + mocks.fetchDiscord.mockImplementation(async (path: string) => { + if (path === "/users/@me/guilds") { + return [{ id: "g1", name: "Guild 1" }]; + } + if (path.startsWith("/guilds/g1/members/search?")) { + const params = new URLSearchParams(path.split("?")[1] ?? ""); + expect(params.get("query")).toBe("alice"); + expect(params.get("limit")).toBe("2"); + return [ + { user: { id: "u1", username: "alice", bot: false }, nick: "Ali" }, + { user: { id: "u2", username: "alice-bot", bot: true }, nick: null }, + { user: { id: "u3", username: "ignored", bot: false }, nick: null }, + ]; + } + return []; + }); + + const rows = await listDiscordDirectoryPeersLive(makeParams({ query: "alice", limit: 2 })); + + expect(rows).toEqual([ + expect.objectContaining({ + kind: "user", + id: "user:u1", + name: "Ali", + handle: "@alice", + rank: 1, + }), + expect.objectContaining({ + kind: "user", + id: "user:u2", + handle: "@alice-bot", + rank: 0, + }), + ]); + }); +}); diff --git a/src/discord/directory-live.ts b/src/discord/directory-live.ts index e17c9ae61..a75f1bf8b 100644 --- a/src/discord/directory-live.ts +++ b/src/discord/directory-live.ts @@ -9,6 +9,7 @@ type DiscordGuild = { id: string; name: string }; type DiscordUser = { id: string; username: string; global_name?: string; bot?: boolean }; type DiscordMember = { user: DiscordUser; nick?: string | null }; type DiscordChannel = { id: string; name?: string | null }; +type DiscordDirectoryAccess = { token: string; query: string }; function normalizeQuery(value?: string | null): string { return value?.trim().toLowerCase() ?? ""; @@ -18,17 +19,31 @@ function buildUserRank(user: DiscordUser): number { return user.bot ? 0 : 1; } -export async function listDiscordDirectoryGroupsLive( +function resolveDiscordDirectoryAccess( params: DirectoryConfigParams, -): Promise { +): DiscordDirectoryAccess | null { const account = resolveDiscordAccount({ cfg: params.cfg, accountId: params.accountId }); const token = normalizeDiscordToken(account.token); if (!token) { + return null; + } + return { token, query: normalizeQuery(params.query) }; +} + +async function listDiscordGuilds(token: string): Promise { + const rawGuilds = await fetchDiscord("/users/@me/guilds", token); + return rawGuilds.filter((guild) => guild.id && guild.name); +} + +export async function listDiscordDirectoryGroupsLive( + params: DirectoryConfigParams, +): Promise { + const access = resolveDiscordDirectoryAccess(params); + if (!access) { return []; } - const query = normalizeQuery(params.query); - const rawGuilds = await fetchDiscord("/users/@me/guilds", token); - const guilds = rawGuilds.filter((g) => g.id && g.name); + const { token, query } = access; + const guilds = await listDiscordGuilds(token); const rows: ChannelDirectoryEntry[] = []; for (const guild of guilds) { @@ -60,18 +75,16 @@ export async function listDiscordDirectoryGroupsLive( export async function listDiscordDirectoryPeersLive( params: DirectoryConfigParams, ): Promise { - const account = resolveDiscordAccount({ cfg: params.cfg, accountId: params.accountId }); - const token = normalizeDiscordToken(account.token); - if (!token) { + const access = resolveDiscordDirectoryAccess(params); + if (!access) { return []; } - const query = normalizeQuery(params.query); + const { token, query } = access; if (!query) { return []; } - const rawGuilds = await fetchDiscord("/users/@me/guilds", token); - const guilds = rawGuilds.filter((g) => g.id && g.name); + const guilds = await listDiscordGuilds(token); const rows: ChannelDirectoryEntry[] = []; const limit = typeof params.limit === "number" && params.limit > 0 ? params.limit : 25; diff --git a/src/discord/monitor/reply-delivery.ts b/src/discord/monitor/reply-delivery.ts index 398e8177a..0ee36b576 100644 --- a/src/discord/monitor/reply-delivery.ts +++ b/src/discord/monitor/reply-delivery.ts @@ -100,6 +100,26 @@ async function sendDiscordChunkWithFallback(params: { }); } +async function sendAdditionalDiscordMedia(params: { + target: string; + token: string; + rest?: RequestClient; + accountId?: string; + mediaUrls: string[]; + resolveReplyTo: () => string | undefined; +}) { + for (const mediaUrl of params.mediaUrls) { + const replyTo = params.resolveReplyTo(); + await sendMessageDiscord(params.target, "", { + token: params.token, + rest: params.rest, + mediaUrl, + accountId: params.accountId, + replyTo, + }); + } +} + export async function deliverDiscordReply(params: { replies: ReplyPayload[]; target: string; @@ -206,16 +226,14 @@ export async function deliverDiscordReply(params: { avatarUrl: persona.avatarUrl, }); // Additional media items are sent as regular attachments (voice is single-file only). - for (const extra of mediaList.slice(1)) { - const replyTo = resolveReplyTo(); - await sendMessageDiscord(params.target, "", { - token: params.token, - rest: params.rest, - mediaUrl: extra, - accountId: params.accountId, - replyTo, - }); - } + await sendAdditionalDiscordMedia({ + target: params.target, + token: params.token, + rest: params.rest, + accountId: params.accountId, + mediaUrls: mediaList.slice(1), + resolveReplyTo, + }); continue; } @@ -227,15 +245,13 @@ export async function deliverDiscordReply(params: { accountId: params.accountId, replyTo, }); - for (const extra of mediaList.slice(1)) { - const replyTo = resolveReplyTo(); - await sendMessageDiscord(params.target, "", { - token: params.token, - rest: params.rest, - mediaUrl: extra, - accountId: params.accountId, - replyTo, - }); - } + await sendAdditionalDiscordMedia({ + target: params.target, + token: params.token, + rest: params.rest, + accountId: params.accountId, + mediaUrls: mediaList.slice(1), + resolveReplyTo, + }); } } diff --git a/src/discord/send.components.ts b/src/discord/send.components.ts index 0afd1f833..e2c87fd5f 100644 --- a/src/discord/send.components.ts +++ b/src/discord/send.components.ts @@ -4,7 +4,6 @@ import { type MessagePayloadObject, type RequestClient, } from "@buape/carbon"; -import type { APIChannel } from "discord-api-types/v10"; import { ChannelType, Routes } from "discord-api-types/v10"; import { loadConfig } from "../config/config.js"; import { recordChannelActivity } from "../infra/channel-activity.js"; @@ -22,6 +21,8 @@ import { createDiscordClient, parseAndResolveRecipient, resolveChannelId, + resolveDiscordChannelType, + toDiscordFileBlob, stripUndefinedFields, SUPPRESS_NOTIFICATIONS_FLAG, } from "./send.shared.js"; @@ -63,13 +64,7 @@ export async function sendDiscordComponentMessage( const recipient = await parseAndResolveRecipient(to, opts.accountId); const { channelId } = await resolveChannelId(rest, recipient, request); - let channelType: number | undefined; - try { - const channel = (await rest.get(Routes.channel(channelId))) as APIChannel | undefined; - channelType = channel?.type; - } catch { - channelType = undefined; - } + const channelType = await resolveDiscordChannelType(rest, channelId); if (channelType && DISCORD_FORUM_LIKE_TYPES.has(channelType)) { throw new Error("Discord components are not supported in forum-style channels"); @@ -107,14 +102,7 @@ export async function sendDiscordComponentMessage( `Component file block expects attachment "${expectedAttachmentName}", but the uploaded file is "${fileName}". Update components.blocks[].file or provide a matching filename.`, ); } - let fileData: Blob; - if (media.buffer instanceof Blob) { - fileData = media.buffer; - } else { - const arrayBuffer = new ArrayBuffer(media.buffer.byteLength); - new Uint8Array(arrayBuffer).set(media.buffer); - fileData = new Blob([arrayBuffer]); - } + const fileData = toDiscordFileBlob(media.buffer); files = [{ data: fileData, name: fileName }]; } else if (expectedAttachmentName) { throw new Error( diff --git a/src/discord/send.outbound.ts b/src/discord/send.outbound.ts index 979054b43..70d5088d4 100644 --- a/src/discord/send.outbound.ts +++ b/src/discord/send.outbound.ts @@ -2,7 +2,6 @@ import crypto from "node:crypto"; import fs from "node:fs/promises"; import path from "node:path"; import { serializePayload, type MessagePayloadObject, type RequestClient } from "@buape/carbon"; -import type { APIChannel } from "discord-api-types/v10"; import { ChannelType, Routes } from "discord-api-types/v10"; import { resolveChunkMode } from "../auto-reply/chunk.js"; import { loadConfig } from "../config/config.js"; @@ -25,6 +24,7 @@ import { normalizeStickerIds, parseAndResolveRecipient, resolveChannelId, + resolveDiscordChannelType, resolveDiscordSendComponents, resolveDiscordSendEmbeds, sendDiscordMedia, @@ -148,13 +148,7 @@ export async function sendMessageDiscord( const { channelId } = await resolveChannelId(rest, recipient, request); // Forum/Media channels reject POST /messages; auto-create a thread post instead. - let channelType: number | undefined; - try { - const channel = (await rest.get(Routes.channel(channelId))) as APIChannel | undefined; - channelType = channel?.type; - } catch { - // If we can't fetch the channel, fall through to the normal send path. - } + const channelType = await resolveDiscordChannelType(rest, channelId); if (isForumLikeType(channelType)) { const threadName = deriveForumThreadName(textWithTables); diff --git a/src/discord/send.permissions.ts b/src/discord/send.permissions.ts index 2dd743bcb..dfe4d91c8 100644 --- a/src/discord/send.permissions.ts +++ b/src/discord/send.permissions.ts @@ -88,12 +88,14 @@ export async function fetchMemberGuildPermissionsDiscord( } /** - * Returns true when the user has ADMINISTRATOR or any required permission bit. + * Returns true when the user has ADMINISTRATOR or required permission bits + * matching the provided predicate. */ -export async function hasAnyGuildPermissionDiscord( +async function hasGuildPermissionsDiscord( guildId: string, userId: string, requiredPermissions: bigint[], + check: (permissions: bigint, requiredPermissions: bigint[]) => boolean, opts: DiscordReactOpts = {}, ): Promise { const permissions = await fetchMemberGuildPermissionsDiscord(guildId, userId, opts); @@ -103,7 +105,26 @@ export async function hasAnyGuildPermissionDiscord( if (hasAdministrator(permissions)) { return true; } - return requiredPermissions.some((permission) => hasPermissionBit(permissions, permission)); + return check(permissions, requiredPermissions); +} + +/** + * Returns true when the user has ADMINISTRATOR or any required permission bit. + */ +export async function hasAnyGuildPermissionDiscord( + guildId: string, + userId: string, + requiredPermissions: bigint[], + opts: DiscordReactOpts = {}, +): Promise { + return await hasGuildPermissionsDiscord( + guildId, + userId, + requiredPermissions, + (permissions, required) => + required.some((permission) => hasPermissionBit(permissions, permission)), + opts, + ); } /** @@ -115,14 +136,14 @@ export async function hasAllGuildPermissionsDiscord( requiredPermissions: bigint[], opts: DiscordReactOpts = {}, ): Promise { - const permissions = await fetchMemberGuildPermissionsDiscord(guildId, userId, opts); - if (permissions === null) { - return false; - } - if (hasAdministrator(permissions)) { - return true; - } - return requiredPermissions.every((permission) => hasPermissionBit(permissions, permission)); + return await hasGuildPermissionsDiscord( + guildId, + userId, + requiredPermissions, + (permissions, required) => + required.every((permission) => hasPermissionBit(permissions, permission)), + opts, + ); } /** diff --git a/src/discord/send.sends-basic-channel-messages.test.ts b/src/discord/send.sends-basic-channel-messages.test.ts index 7720876f5..bd56e7973 100644 --- a/src/discord/send.sends-basic-channel-messages.test.ts +++ b/src/discord/send.sends-basic-channel-messages.test.ts @@ -48,6 +48,18 @@ describe("sendMessageDiscord", () => { }; } + function setupForumSend(secondResponse: { id: string; channel_id: string }) { + const { rest, postMock, getMock } = makeDiscordRest(); + getMock.mockResolvedValueOnce({ type: ChannelType.GuildForum }); + postMock + .mockResolvedValueOnce({ + id: "thread1", + message: { id: "starter1", channel_id: "thread1" }, + }) + .mockResolvedValueOnce(secondResponse); + return { rest, postMock }; + } + beforeEach(() => { vi.clearAllMocks(); }); @@ -97,14 +109,7 @@ describe("sendMessageDiscord", () => { }); it("posts media as a follow-up message in forum channels", async () => { - const { rest, postMock, getMock } = makeDiscordRest(); - getMock.mockResolvedValueOnce({ type: ChannelType.GuildForum }); - postMock - .mockResolvedValueOnce({ - id: "thread1", - message: { id: "starter1", channel_id: "thread1" }, - }) - .mockResolvedValueOnce({ id: "media1", channel_id: "thread1" }); + const { rest, postMock } = setupForumSend({ id: "media1", channel_id: "thread1" }); const res = await sendMessageDiscord("channel:forum1", "Topic", { rest, token: "t", @@ -133,14 +138,7 @@ describe("sendMessageDiscord", () => { }); it("chunks long forum posts into follow-up messages", async () => { - const { rest, postMock, getMock } = makeDiscordRest(); - getMock.mockResolvedValueOnce({ type: ChannelType.GuildForum }); - postMock - .mockResolvedValueOnce({ - id: "thread1", - message: { id: "starter1", channel_id: "thread1" }, - }) - .mockResolvedValueOnce({ id: "msg2", channel_id: "thread1" }); + const { rest, postMock } = setupForumSend({ id: "msg2", channel_id: "thread1" }); const longText = "a".repeat(2001); await sendMessageDiscord("channel:forum1", longText, { rest, diff --git a/src/discord/send.shared.ts b/src/discord/send.shared.ts index 2d463f618..94508c813 100644 --- a/src/discord/send.shared.ts +++ b/src/discord/send.shared.ts @@ -8,7 +8,7 @@ import { } from "@buape/carbon"; import { PollLayoutType } from "discord-api-types/payloads/v10"; import type { RESTAPIPoll } from "discord-api-types/rest/v10"; -import { Routes, type APIEmbed } from "discord-api-types/v10"; +import { Routes, type APIChannel, type APIEmbed } from "discord-api-types/v10"; import type { ChunkMode } from "../auto-reply/chunk.js"; import { loadConfig } from "../config/config.js"; import type { RetryRunner } from "../infra/retry-policy.js"; @@ -242,6 +242,18 @@ async function resolveChannelId( return { channelId: dmChannel.id, dm: true }; } +export async function resolveDiscordChannelType( + rest: RequestClient, + channelId: string, +): Promise { + try { + const channel = (await rest.get(Routes.channel(channelId))) as APIChannel | undefined; + return channel?.type; + } catch { + return undefined; + } +} + // Discord message flag for silent/suppress notifications export const SUPPRESS_NOTIFICATIONS_FLAG = 1 << 12; @@ -329,6 +341,15 @@ export function stripUndefinedFields(value: T): T { return Object.fromEntries(Object.entries(value).filter(([, entry]) => entry !== undefined)) as T; } +export function toDiscordFileBlob(data: Blob | Uint8Array): Blob { + if (data instanceof Blob) { + return data; + } + const arrayBuffer = new ArrayBuffer(data.byteLength); + new Uint8Array(arrayBuffer).set(data); + return new Blob([arrayBuffer]); +} + async function sendDiscordText( rest: RequestClient, channelId: string, @@ -404,14 +425,7 @@ async function sendDiscordMedia( const caption = chunks[0] ?? ""; const messageReference = replyTo ? { message_id: replyTo, fail_if_not_exists: false } : undefined; const flags = silent ? SUPPRESS_NOTIFICATIONS_FLAG : undefined; - let fileData: Blob; - if (media.buffer instanceof Blob) { - fileData = media.buffer; - } else { - const arrayBuffer = new ArrayBuffer(media.buffer.byteLength); - new Uint8Array(arrayBuffer).set(media.buffer); - fileData = new Blob([arrayBuffer]); - } + const fileData = toDiscordFileBlob(media.buffer); const captionComponents = resolveDiscordSendComponents({ components, text: caption,