From a08438ae97f1e468c496282a146ab37bc3c44f17 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 18 Jan 2026 00:03:35 +0000 Subject: [PATCH] refactor(discord): centralize target parsing Co-authored-by: Jonathan Rhyne --- src/agents/tools/discord-actions-messaging.ts | 51 +++++------- .../plugins/actions/discord/handle-action.ts | 5 +- src/channels/plugins/normalize-target.ts | 24 +----- src/discord/send.shared.ts | 34 ++------ src/discord/targets.test.ts | 75 ++++++++++++++++++ src/discord/targets.ts | 78 +++++++++++++++++++ 6 files changed, 184 insertions(+), 83 deletions(-) create mode 100644 src/discord/targets.test.ts create mode 100644 src/discord/targets.ts diff --git a/src/agents/tools/discord-actions-messaging.ts b/src/agents/tools/discord-actions-messaging.ts index 309187bdf..f552f17fd 100644 --- a/src/agents/tools/discord-actions-messaging.ts +++ b/src/agents/tools/discord-actions-messaging.ts @@ -28,6 +28,7 @@ import { readStringParam, } from "./common.js"; import { withNormalizedTimestamp } from "../date-time.js"; +import { resolveDiscordChannelId } from "../../discord/targets.js"; function parseDiscordMessageLink(link: string) { const normalized = link.trim(); @@ -51,6 +52,12 @@ export async function handleDiscordMessagingAction( params: Record, isActionEnabled: ActionGate, ): Promise> { + const resolveChannelId = () => + resolveDiscordChannelId( + readStringParam(params, "channelId", { + required: true, + }), + ); const normalizeMessage = (message: unknown) => { if (!message || typeof message !== "object") return message; return withNormalizedTimestamp( @@ -63,9 +70,7 @@ export async function handleDiscordMessagingAction( if (!isActionEnabled("reactions")) { throw new Error("Discord reactions are disabled."); } - const channelId = readStringParam(params, "channelId", { - required: true, - }); + const channelId = resolveChannelId(); const messageId = readStringParam(params, "messageId", { required: true, }); @@ -87,9 +92,7 @@ export async function handleDiscordMessagingAction( if (!isActionEnabled("reactions")) { throw new Error("Discord reactions are disabled."); } - const channelId = readStringParam(params, "channelId", { - required: true, - }); + const channelId = resolveChannelId(); const messageId = readStringParam(params, "messageId", { required: true, }); @@ -145,9 +148,7 @@ export async function handleDiscordMessagingAction( if (!isActionEnabled("permissions")) { throw new Error("Discord permissions are disabled."); } - const channelId = readStringParam(params, "channelId", { - required: true, - }); + const channelId = resolveChannelId(); const permissions = await fetchChannelPermissionsDiscord(channelId); return jsonResult({ ok: true, permissions }); } @@ -183,9 +184,7 @@ export async function handleDiscordMessagingAction( if (!isActionEnabled("messages")) { throw new Error("Discord message reads are disabled."); } - const channelId = readStringParam(params, "channelId", { - required: true, - }); + const channelId = resolveChannelId(); const messages = await readMessagesDiscord(channelId, { limit: typeof params.limit === "number" && Number.isFinite(params.limit) @@ -223,9 +222,7 @@ export async function handleDiscordMessagingAction( if (!isActionEnabled("messages")) { throw new Error("Discord message edits are disabled."); } - const channelId = readStringParam(params, "channelId", { - required: true, - }); + const channelId = resolveChannelId(); const messageId = readStringParam(params, "messageId", { required: true, }); @@ -241,9 +238,7 @@ export async function handleDiscordMessagingAction( if (!isActionEnabled("messages")) { throw new Error("Discord message deletes are disabled."); } - const channelId = readStringParam(params, "channelId", { - required: true, - }); + const channelId = resolveChannelId(); const messageId = readStringParam(params, "messageId", { required: true, }); @@ -254,9 +249,7 @@ export async function handleDiscordMessagingAction( if (!isActionEnabled("threads")) { throw new Error("Discord threads are disabled."); } - const channelId = readStringParam(params, "channelId", { - required: true, - }); + const channelId = resolveChannelId(); const name = readStringParam(params, "name", { required: true }); const messageId = readStringParam(params, "messageId"); const autoArchiveMinutesRaw = params.autoArchiveMinutes; @@ -299,9 +292,7 @@ export async function handleDiscordMessagingAction( if (!isActionEnabled("threads")) { throw new Error("Discord threads are disabled."); } - const channelId = readStringParam(params, "channelId", { - required: true, - }); + const channelId = resolveChannelId(); const content = readStringParam(params, "content", { required: true, }); @@ -317,9 +308,7 @@ export async function handleDiscordMessagingAction( if (!isActionEnabled("pins")) { throw new Error("Discord pins are disabled."); } - const channelId = readStringParam(params, "channelId", { - required: true, - }); + const channelId = resolveChannelId(); const messageId = readStringParam(params, "messageId", { required: true, }); @@ -330,9 +319,7 @@ export async function handleDiscordMessagingAction( if (!isActionEnabled("pins")) { throw new Error("Discord pins are disabled."); } - const channelId = readStringParam(params, "channelId", { - required: true, - }); + const channelId = resolveChannelId(); const messageId = readStringParam(params, "messageId", { required: true, }); @@ -343,9 +330,7 @@ export async function handleDiscordMessagingAction( if (!isActionEnabled("pins")) { throw new Error("Discord pins are disabled."); } - const channelId = readStringParam(params, "channelId", { - required: true, - }); + const channelId = resolveChannelId(); const pins = await listPinsDiscord(channelId); return jsonResult({ ok: true, pins: pins.map((pin) => normalizeMessage(pin)) }); } diff --git a/src/channels/plugins/actions/discord/handle-action.ts b/src/channels/plugins/actions/discord/handle-action.ts index b052896ec..82f08e686 100644 --- a/src/channels/plugins/actions/discord/handle-action.ts +++ b/src/channels/plugins/actions/discord/handle-action.ts @@ -7,6 +7,7 @@ import { import { handleDiscordAction } from "../../../../agents/tools/discord-actions.js"; import type { ChannelMessageActionContext } from "../../types.js"; import { tryHandleDiscordMessageActionGuildAdmin } from "./handle-action.guild-admin.js"; +import { resolveDiscordChannelId } from "../../../../discord/targets.js"; const providerId = "discord"; @@ -22,7 +23,9 @@ export async function handleDiscordMessageAction( const { action, params, cfg } = ctx; const resolveChannelId = () => - readStringParam(params, "channelId") ?? readStringParam(params, "to", { required: true }); + resolveDiscordChannelId( + readStringParam(params, "channelId") ?? readStringParam(params, "to", { required: true }), + ); if (action === "send") { const to = readStringParam(params, "to", { required: true }); diff --git a/src/channels/plugins/normalize-target.ts b/src/channels/plugins/normalize-target.ts index 1c5691ad6..c19c0fe7a 100644 --- a/src/channels/plugins/normalize-target.ts +++ b/src/channels/plugins/normalize-target.ts @@ -1,4 +1,5 @@ import { normalizeWhatsAppTarget } from "../../whatsapp/normalize.js"; +import { parseDiscordTarget } from "../../discord/targets.js"; export function normalizeSlackMessagingTarget(raw: string): string | undefined { const trimmed = raw.trim(); @@ -39,27 +40,8 @@ export function looksLikeSlackTargetId(raw: string): boolean { } export function normalizeDiscordMessagingTarget(raw: string): string | undefined { - const trimmed = raw.trim(); - if (!trimmed) return undefined; - const mentionMatch = trimmed.match(/^<@!?(\d+)>$/); - if (mentionMatch) return `user:${mentionMatch[1]}`.toLowerCase(); - if (trimmed.startsWith("user:")) { - const id = trimmed.slice(5).trim(); - return id ? `user:${id}`.toLowerCase() : undefined; - } - if (trimmed.startsWith("channel:")) { - const id = trimmed.slice(8).trim(); - return id ? `channel:${id}`.toLowerCase() : undefined; - } - if (trimmed.startsWith("discord:")) { - const id = trimmed.slice(8).trim(); - return id ? `user:${id}`.toLowerCase() : undefined; - } - if (trimmed.startsWith("@")) { - const id = trimmed.slice(1).trim(); - return id ? `user:${id}`.toLowerCase() : undefined; - } - return `channel:${trimmed}`.toLowerCase(); + const target = parseDiscordTarget(raw, { defaultKind: "channel" }); + return target?.normalized; } export function looksLikeDiscordTargetId(raw: string): boolean { diff --git a/src/discord/send.shared.ts b/src/discord/send.shared.ts index 8d67e23c7..2961375ce 100644 --- a/src/discord/send.shared.ts +++ b/src/discord/send.shared.ts @@ -12,6 +12,7 @@ import { resolveDiscordAccount } from "./accounts.js"; import { chunkDiscordText } from "./chunk.js"; import { fetchChannelPermissionsDiscord, isThreadChannelType } from "./send.permissions.js"; import { DiscordSendError } from "./send.types.js"; +import { parseDiscordTarget } from "./targets.js"; import { normalizeDiscordToken } from "./token.js"; const DISCORD_TEXT_LIMIT = 2000; @@ -90,36 +91,13 @@ function normalizeReactionEmoji(raw: string) { } function parseRecipient(raw: string): DiscordRecipient { - const trimmed = raw.trim(); - if (!trimmed) { + const target = parseDiscordTarget(raw, { + ambiguousMessage: `Ambiguous Discord recipient "${raw.trim()}". Use "user:${raw.trim()}" for DMs or "channel:${raw.trim()}" for channel messages.`, + }); + if (!target) { throw new Error("Recipient is required for Discord sends"); } - const mentionMatch = trimmed.match(/^<@!?(\d+)>$/); - if (mentionMatch) { - return { kind: "user", id: mentionMatch[1] }; - } - if (trimmed.startsWith("user:")) { - return { kind: "user", id: trimmed.slice("user:".length) }; - } - if (trimmed.startsWith("channel:")) { - return { kind: "channel", id: trimmed.slice("channel:".length) }; - } - if (trimmed.startsWith("discord:")) { - return { kind: "user", id: trimmed.slice("discord:".length) }; - } - if (trimmed.startsWith("@")) { - const candidate = trimmed.slice(1); - if (!/^\d+$/.test(candidate)) { - throw new Error("Discord DMs require a user id (use user: or a <@id> mention)"); - } - return { kind: "user", id: candidate }; - } - if (/^\d+$/.test(trimmed)) { - throw new Error( - `Ambiguous Discord recipient "${trimmed}". Use "user:${trimmed}" for DMs or "channel:${trimmed}" for channel messages.`, - ); - } - return { kind: "channel", id: trimmed }; + return { kind: target.kind, id: target.id }; } function normalizeStickerIds(raw: string[]) { diff --git a/src/discord/targets.test.ts b/src/discord/targets.test.ts new file mode 100644 index 000000000..6e83984fd --- /dev/null +++ b/src/discord/targets.test.ts @@ -0,0 +1,75 @@ +import { describe, expect, it } from "vitest"; + +import { normalizeDiscordMessagingTarget } from "../channels/plugins/normalize-target.js"; +import { parseDiscordTarget, resolveDiscordChannelId } from "./targets.js"; + +describe("parseDiscordTarget", () => { + it("parses user mention and prefixes", () => { + expect(parseDiscordTarget("<@123>")).toMatchObject({ + kind: "user", + id: "123", + normalized: "user:123", + }); + expect(parseDiscordTarget("<@!456>")).toMatchObject({ + kind: "user", + id: "456", + normalized: "user:456", + }); + expect(parseDiscordTarget("user:789")).toMatchObject({ + kind: "user", + id: "789", + normalized: "user:789", + }); + expect(parseDiscordTarget("discord:987")).toMatchObject({ + kind: "user", + id: "987", + normalized: "user:987", + }); + }); + + it("parses channel targets", () => { + expect(parseDiscordTarget("channel:555")).toMatchObject({ + kind: "channel", + id: "555", + normalized: "channel:555", + }); + expect(parseDiscordTarget("general")).toMatchObject({ + kind: "channel", + id: "general", + normalized: "channel:general", + }); + }); + + it("rejects ambiguous numeric ids without a default kind", () => { + expect(() => parseDiscordTarget("123")).toThrow(/Ambiguous Discord recipient/); + }); + + it("accepts numeric ids when a default kind is provided", () => { + expect(parseDiscordTarget("123", { defaultKind: "channel" })).toMatchObject({ + kind: "channel", + id: "123", + normalized: "channel:123", + }); + }); + + it("rejects non-numeric @ mentions", () => { + expect(() => parseDiscordTarget("@bob")).toThrow(/Discord DMs require a user id/); + }); +}); + +describe("resolveDiscordChannelId", () => { + it("strips channel: prefix and accepts raw ids", () => { + expect(resolveDiscordChannelId("channel:123")).toBe("123"); + expect(resolveDiscordChannelId("123")).toBe("123"); + }); + + it("rejects user targets", () => { + expect(() => resolveDiscordChannelId("user:123")).toThrow(/channel id is required/i); + }); +}); + +describe("normalizeDiscordMessagingTarget", () => { + it("defaults raw numeric ids to channels", () => { + expect(normalizeDiscordMessagingTarget("123")).toBe("channel:123"); + }); +}); diff --git a/src/discord/targets.ts b/src/discord/targets.ts new file mode 100644 index 000000000..6c5145576 --- /dev/null +++ b/src/discord/targets.ts @@ -0,0 +1,78 @@ +export type DiscordTargetKind = "user" | "channel"; + +export type DiscordTarget = { + kind: DiscordTargetKind; + id: string; + raw: string; + normalized: string; +}; + +type DiscordTargetParseOptions = { + defaultKind?: DiscordTargetKind; + ambiguousMessage?: string; +}; + +function normalizeTargetId(kind: DiscordTargetKind, id: string) { + return `${kind}:${id}`.toLowerCase(); +} + +function buildTarget(kind: DiscordTargetKind, id: string, raw: string): DiscordTarget { + return { + kind, + id, + raw, + normalized: normalizeTargetId(kind, id), + }; +} + +export function parseDiscordTarget( + raw: string, + options: DiscordTargetParseOptions = {}, +): DiscordTarget | undefined { + const trimmed = raw.trim(); + if (!trimmed) return undefined; + const mentionMatch = trimmed.match(/^<@!?(\d+)>$/); + if (mentionMatch) { + return buildTarget("user", mentionMatch[1], trimmed); + } + if (trimmed.startsWith("user:")) { + const id = trimmed.slice("user:".length).trim(); + return id ? buildTarget("user", id, trimmed) : undefined; + } + if (trimmed.startsWith("channel:")) { + const id = trimmed.slice("channel:".length).trim(); + return id ? buildTarget("channel", id, trimmed) : undefined; + } + if (trimmed.startsWith("discord:")) { + const id = trimmed.slice("discord:".length).trim(); + return id ? buildTarget("user", id, trimmed) : undefined; + } + if (trimmed.startsWith("@")) { + const candidate = trimmed.slice(1).trim(); + if (!/^\d+$/.test(candidate)) { + throw new Error("Discord DMs require a user id (use user: or a <@id> mention)"); + } + return buildTarget("user", candidate, trimmed); + } + if (/^\d+$/.test(trimmed)) { + if (options.defaultKind) { + return buildTarget(options.defaultKind, trimmed, trimmed); + } + throw new Error( + options.ambiguousMessage ?? + `Ambiguous Discord recipient "${trimmed}". Use "user:${trimmed}" for DMs or "channel:${trimmed}" for channel messages.`, + ); + } + return buildTarget("channel", trimmed, trimmed); +} + +export function resolveDiscordChannelId(raw: string): string { + const target = parseDiscordTarget(raw, { defaultKind: "channel" }); + if (!target) { + throw new Error("Discord channel id is required."); + } + if (target.kind !== "channel") { + throw new Error("Discord channel id is required (use channel:)."); + } + return target.id; +}