refactor(slack): centralize target parsing

This commit is contained in:
Peter Steinberger
2026-01-18 00:15:02 +00:00
parent a5aa48beea
commit 4d590f9254
8 changed files with 190 additions and 83 deletions

View File

@@ -48,6 +48,20 @@ describe("handleSlackAction", () => {
expect(reactSlackMessage).toHaveBeenCalledWith("C1", "123.456", "✅"); expect(reactSlackMessage).toHaveBeenCalledWith("C1", "123.456", "✅");
}); });
it("strips channel: prefix for channelId params", async () => {
const cfg = { channels: { slack: { botToken: "tok" } } } as ClawdbotConfig;
await handleSlackAction(
{
action: "react",
channelId: "channel:C1",
messageId: "123.456",
emoji: "✅",
},
cfg,
);
expect(reactSlackMessage).toHaveBeenCalledWith("C1", "123.456", "✅");
});
it("removes reactions on empty emoji", async () => { it("removes reactions on empty emoji", async () => {
const cfg = { channels: { slack: { botToken: "tok" } } } as ClawdbotConfig; const cfg = { channels: { slack: { botToken: "tok" } } } as ClawdbotConfig;
await handleSlackAction( await handleSlackAction(

View File

@@ -17,6 +17,7 @@ import {
sendSlackMessage, sendSlackMessage,
unpinSlackMessage, unpinSlackMessage,
} from "../../slack/actions.js"; } from "../../slack/actions.js";
import { parseSlackTarget, resolveSlackChannelId } from "../../slack/targets.js";
import { withNormalizedTimestamp } from "../date-time.js"; import { withNormalizedTimestamp } from "../date-time.js";
import { createActionGate, jsonResult, readReactionParams, readStringParam } from "./common.js"; import { createActionGate, jsonResult, readReactionParams, readStringParam } from "./common.js";
@@ -52,10 +53,9 @@ function resolveThreadTsFromContext(
// No context or missing required fields // No context or missing required fields
if (!context?.currentThreadTs || !context?.currentChannelId) return undefined; if (!context?.currentThreadTs || !context?.currentChannelId) return undefined;
// Normalize target (strip "channel:" prefix if present) const parsedTarget = parseSlackTarget(targetChannel, { defaultKind: "channel" });
const normalizedTarget = targetChannel.startsWith("channel:") if (!parsedTarget || parsedTarget.kind !== "channel") return undefined;
? targetChannel.slice("channel:".length) const normalizedTarget = parsedTarget.id;
: targetChannel;
// Different channel - don't inject // Different channel - don't inject
if (normalizedTarget !== context.currentChannelId) return undefined; if (normalizedTarget !== context.currentChannelId) return undefined;
@@ -76,6 +76,12 @@ export async function handleSlackAction(
cfg: ClawdbotConfig, cfg: ClawdbotConfig,
context?: SlackActionContext, context?: SlackActionContext,
): Promise<AgentToolResult<unknown>> { ): Promise<AgentToolResult<unknown>> {
const resolveChannelId = () =>
resolveSlackChannelId(
readStringParam(params, "channelId", {
required: true,
}),
);
const action = readStringParam(params, "action", { required: true }); const action = readStringParam(params, "action", { required: true });
const accountId = readStringParam(params, "accountId"); const accountId = readStringParam(params, "accountId");
const account = resolveSlackAccount({ cfg, accountId }); const account = resolveSlackAccount({ cfg, accountId });
@@ -109,7 +115,7 @@ export async function handleSlackAction(
if (!isActionEnabled("reactions")) { if (!isActionEnabled("reactions")) {
throw new Error("Slack reactions are disabled."); throw new Error("Slack reactions are disabled.");
} }
const channelId = readStringParam(params, "channelId", { required: true }); const channelId = resolveChannelId();
const messageId = readStringParam(params, "messageId", { required: true }); const messageId = readStringParam(params, "messageId", { required: true });
if (action === "react") { if (action === "react") {
const { emoji, remove, isEmpty } = readReactionParams(params, { const { emoji, remove, isEmpty } = readReactionParams(params, {
@@ -166,8 +172,8 @@ export async function handleSlackAction(
// threadTs: once we send a message to the current channel, consider the // threadTs: once we send a message to the current channel, consider the
// first reply "used" so later tool calls don't auto-thread again. // first reply "used" so later tool calls don't auto-thread again.
if (context?.hasRepliedRef && context.currentChannelId) { if (context?.hasRepliedRef && context.currentChannelId) {
const normalizedTarget = to.startsWith("channel:") ? to.slice("channel:".length) : to; const parsedTarget = parseSlackTarget(to, { defaultKind: "channel" });
if (normalizedTarget === context.currentChannelId) { if (parsedTarget?.kind === "channel" && parsedTarget.id === context.currentChannelId) {
context.hasRepliedRef.value = true; context.hasRepliedRef.value = true;
} }
} }
@@ -175,9 +181,7 @@ export async function handleSlackAction(
return jsonResult({ ok: true, result }); return jsonResult({ ok: true, result });
} }
case "editMessage": { case "editMessage": {
const channelId = readStringParam(params, "channelId", { const channelId = resolveChannelId();
required: true,
});
const messageId = readStringParam(params, "messageId", { const messageId = readStringParam(params, "messageId", {
required: true, required: true,
}); });
@@ -192,9 +196,7 @@ export async function handleSlackAction(
return jsonResult({ ok: true }); return jsonResult({ ok: true });
} }
case "deleteMessage": { case "deleteMessage": {
const channelId = readStringParam(params, "channelId", { const channelId = resolveChannelId();
required: true,
});
const messageId = readStringParam(params, "messageId", { const messageId = readStringParam(params, "messageId", {
required: true, required: true,
}); });
@@ -206,9 +208,7 @@ export async function handleSlackAction(
return jsonResult({ ok: true }); return jsonResult({ ok: true });
} }
case "readMessages": { case "readMessages": {
const channelId = readStringParam(params, "channelId", { const channelId = resolveChannelId();
required: true,
});
const limitRaw = params.limit; const limitRaw = params.limit;
const limit = const limit =
typeof limitRaw === "number" && Number.isFinite(limitRaw) ? limitRaw : undefined; typeof limitRaw === "number" && Number.isFinite(limitRaw) ? limitRaw : undefined;
@@ -237,7 +237,7 @@ export async function handleSlackAction(
if (!isActionEnabled("pins")) { if (!isActionEnabled("pins")) {
throw new Error("Slack pins are disabled."); throw new Error("Slack pins are disabled.");
} }
const channelId = readStringParam(params, "channelId", { required: true }); const channelId = resolveChannelId();
if (action === "pinMessage") { if (action === "pinMessage") {
const messageId = readStringParam(params, "messageId", { const messageId = readStringParam(params, "messageId", {
required: true, required: true,

View File

@@ -1,32 +1,10 @@
import { normalizeWhatsAppTarget } from "../../whatsapp/normalize.js";
import { parseDiscordTarget } from "../../discord/targets.js"; import { parseDiscordTarget } from "../../discord/targets.js";
import { parseSlackTarget } from "../../slack/targets.js";
import { normalizeWhatsAppTarget } from "../../whatsapp/normalize.js";
export function normalizeSlackMessagingTarget(raw: string): string | undefined { export function normalizeSlackMessagingTarget(raw: string): string | undefined {
const trimmed = raw.trim(); const target = parseSlackTarget(raw, { defaultKind: "channel" });
if (!trimmed) return undefined; return target?.normalized;
const mentionMatch = trimmed.match(/^<@([A-Z0-9]+)>$/i);
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("slack:")) {
const id = trimmed.slice(6).trim();
return id ? `user:${id}`.toLowerCase() : undefined;
}
if (trimmed.startsWith("@")) {
const id = trimmed.slice(1).trim();
return id ? `user:${id}`.toLowerCase() : undefined;
}
if (trimmed.startsWith("#")) {
const id = trimmed.slice(1).trim();
return id ? `channel:${id}`.toLowerCase() : undefined;
}
return `channel:${trimmed}`.toLowerCase();
} }
export function looksLikeSlackTargetId(raw: string): boolean { export function looksLikeSlackTargetId(raw: string): boolean {
@@ -40,6 +18,7 @@ export function looksLikeSlackTargetId(raw: string): boolean {
} }
export function normalizeDiscordMessagingTarget(raw: string): string | undefined { export function normalizeDiscordMessagingTarget(raw: string): string | undefined {
// Default bare IDs to channels so routing is stable across tool actions.
const target = parseDiscordTarget(raw, { defaultKind: "channel" }); const target = parseDiscordTarget(raw, { defaultKind: "channel" });
return target?.normalized; return target?.normalized;
} }

View File

@@ -1,6 +1,7 @@
import { createActionGate, readNumberParam, readStringParam } from "../../agents/tools/common.js"; import { createActionGate, readNumberParam, readStringParam } from "../../agents/tools/common.js";
import { handleSlackAction, type SlackActionContext } from "../../agents/tools/slack-actions.js"; import { handleSlackAction, type SlackActionContext } from "../../agents/tools/slack-actions.js";
import { listEnabledSlackAccounts } from "../../slack/accounts.js"; import { listEnabledSlackAccounts } from "../../slack/accounts.js";
import { resolveSlackChannelId } from "../../slack/targets.js";
import type { import type {
ChannelMessageActionAdapter, ChannelMessageActionAdapter,
ChannelMessageActionContext, ChannelMessageActionContext,
@@ -60,7 +61,9 @@ export function createSlackActions(providerId: string): ChannelMessageActionAdap
const accountId = ctx.accountId ?? undefined; const accountId = ctx.accountId ?? undefined;
const toolContext = ctx.toolContext as SlackActionContext | undefined; const toolContext = ctx.toolContext as SlackActionContext | undefined;
const resolveChannelId = () => const resolveChannelId = () =>
readStringParam(params, "channelId") ?? readStringParam(params, "to", { required: true }); resolveSlackChannelId(
readStringParam(params, "channelId") ?? readStringParam(params, "to", { required: true }),
);
if (action === "send") { if (action === "send") {
const to = readStringParam(params, "to", { required: true }); const to = readStringParam(params, "to", { required: true });

View File

@@ -1,8 +1,8 @@
import { getChannelPlugin, listChannelPlugins } from "../../channels/plugins/index.js"; import { getChannelPlugin, listChannelPlugins } from "../../channels/plugins/index.js";
import { resolveChannelDefaultAccountId } from "../../channels/plugins/helpers.js"; import { resolveChannelDefaultAccountId } from "../../channels/plugins/helpers.js";
import { normalizeDiscordMessagingTarget } from "../../channels/plugins/normalize-target.js";
import type { ChannelCapabilities, ChannelPlugin } from "../../channels/plugins/types.js"; import type { ChannelCapabilities, ChannelPlugin } from "../../channels/plugins/types.js";
import { fetchChannelPermissionsDiscord } from "../../discord/send.js"; import { fetchChannelPermissionsDiscord } from "../../discord/send.js";
import { parseDiscordTarget } from "../../discord/targets.js";
import { danger } from "../../globals.js"; import { danger } from "../../globals.js";
import type { ClawdbotConfig } from "../../config/config.js"; import type { ClawdbotConfig } from "../../config/config.js";
import { defaultRuntime, type RuntimeEnv } from "../../runtime.js"; import { defaultRuntime, type RuntimeEnv } from "../../runtime.js";
@@ -88,24 +88,24 @@ function formatSupport(capabilities?: ChannelCapabilities) {
function summarizeDiscordTarget(raw?: string): DiscordTargetSummary | undefined { function summarizeDiscordTarget(raw?: string): DiscordTargetSummary | undefined {
if (!raw) return undefined; if (!raw) return undefined;
const normalized = normalizeDiscordMessagingTarget(raw); const target = parseDiscordTarget(raw, { defaultKind: "channel" });
if (!normalized) return { raw }; if (!target) return { raw };
if (normalized.startsWith("channel:")) { if (target.kind === "channel") {
return { return {
raw, raw,
normalized, normalized: target.normalized,
kind: "channel", kind: "channel",
channelId: normalized.slice("channel:".length), channelId: target.id,
}; };
} }
if (normalized.startsWith("user:")) { if (target.kind === "user") {
return { return {
raw, raw,
normalized, normalized: target.normalized,
kind: "user", kind: "user",
}; };
} }
return { raw, normalized }; return { raw, normalized: target.normalized };
} }
function formatDiscordIntents(intents?: { function formatDiscordIntents(intents?: {

View File

@@ -7,6 +7,7 @@ import { loadWebMedia } from "../web/media.js";
import type { SlackTokenSource } from "./accounts.js"; import type { SlackTokenSource } from "./accounts.js";
import { resolveSlackAccount } from "./accounts.js"; import { resolveSlackAccount } from "./accounts.js";
import { markdownToSlackMrkdwnChunks } from "./format.js"; import { markdownToSlackMrkdwnChunks } from "./format.js";
import { parseSlackTarget } from "./targets.js";
import { resolveSlackBotToken } from "./token.js"; import { resolveSlackBotToken } from "./token.js";
const SLACK_TEXT_LIMIT = 4000; const SLACK_TEXT_LIMIT = 4000;
@@ -57,38 +58,11 @@ function resolveToken(params: {
} }
function parseRecipient(raw: string): SlackRecipient { function parseRecipient(raw: string): SlackRecipient {
const trimmed = raw.trim(); const target = parseSlackTarget(raw);
if (!trimmed) { if (!target) {
throw new Error("Recipient is required for Slack sends"); throw new Error("Recipient is required for Slack sends");
} }
const mentionMatch = trimmed.match(/^<@([A-Z0-9]+)>$/i); return { kind: target.kind, id: target.id };
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("slack:")) {
return { kind: "user", id: trimmed.slice("slack:".length) };
}
if (trimmed.startsWith("@")) {
const candidate = trimmed.slice(1);
if (!/^[A-Z0-9]+$/i.test(candidate)) {
throw new Error("Slack DMs require a user id (use user:<id> or <@id>)");
}
return { kind: "user", id: candidate };
}
if (trimmed.startsWith("#")) {
const candidate = trimmed.slice(1);
if (!/^[A-Z0-9]+$/i.test(candidate)) {
throw new Error("Slack channels require a channel id (use channel:<id>)");
}
return { kind: "channel", id: candidate };
}
return { kind: "channel", id: trimmed };
} }
async function resolveChannelId( async function resolveChannelId(

59
src/slack/targets.test.ts Normal file
View File

@@ -0,0 +1,59 @@
import { describe, expect, it } from "vitest";
import { normalizeSlackMessagingTarget } from "../channels/plugins/normalize-target.js";
import { parseSlackTarget, resolveSlackChannelId } from "./targets.js";
describe("parseSlackTarget", () => {
it("parses user mentions and prefixes", () => {
expect(parseSlackTarget("<@U123>")).toMatchObject({
kind: "user",
id: "U123",
normalized: "user:u123",
});
expect(parseSlackTarget("user:U456")).toMatchObject({
kind: "user",
id: "U456",
normalized: "user:u456",
});
expect(parseSlackTarget("slack:U789")).toMatchObject({
kind: "user",
id: "U789",
normalized: "user:u789",
});
});
it("parses channel targets", () => {
expect(parseSlackTarget("channel:C123")).toMatchObject({
kind: "channel",
id: "C123",
normalized: "channel:c123",
});
expect(parseSlackTarget("#C999")).toMatchObject({
kind: "channel",
id: "C999",
normalized: "channel:c999",
});
});
it("rejects invalid @ and # targets", () => {
expect(() => parseSlackTarget("@bob-1")).toThrow(/Slack DMs require a user id/);
expect(() => parseSlackTarget("#general-1")).toThrow(/Slack channels require a channel id/);
});
});
describe("resolveSlackChannelId", () => {
it("strips channel: prefix and accepts raw ids", () => {
expect(resolveSlackChannelId("channel:C123")).toBe("C123");
expect(resolveSlackChannelId("C123")).toBe("C123");
});
it("rejects user targets", () => {
expect(() => resolveSlackChannelId("user:U123")).toThrow(/channel id is required/i);
});
});
describe("normalizeSlackMessagingTarget", () => {
it("defaults raw ids to channels", () => {
expect(normalizeSlackMessagingTarget("C123")).toBe("channel:c123");
});
});

78
src/slack/targets.ts Normal file
View File

@@ -0,0 +1,78 @@
export type SlackTargetKind = "user" | "channel";
export type SlackTarget = {
kind: SlackTargetKind;
id: string;
raw: string;
normalized: string;
};
type SlackTargetParseOptions = {
defaultKind?: SlackTargetKind;
};
function normalizeTargetId(kind: SlackTargetKind, id: string) {
return `${kind}:${id}`.toLowerCase();
}
function buildTarget(kind: SlackTargetKind, id: string, raw: string): SlackTarget {
return {
kind,
id,
raw,
normalized: normalizeTargetId(kind, id),
};
}
export function parseSlackTarget(
raw: string,
options: SlackTargetParseOptions = {},
): SlackTarget | undefined {
const trimmed = raw.trim();
if (!trimmed) return undefined;
const mentionMatch = trimmed.match(/^<@([A-Z0-9]+)>$/i);
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("slack:")) {
const id = trimmed.slice("slack:".length).trim();
return id ? buildTarget("user", id, trimmed) : undefined;
}
if (trimmed.startsWith("@")) {
const candidate = trimmed.slice(1).trim();
if (!/^[A-Z0-9]+$/i.test(candidate)) {
throw new Error("Slack DMs require a user id (use user:<id> or <@id>)");
}
return buildTarget("user", candidate, trimmed);
}
if (trimmed.startsWith("#")) {
const candidate = trimmed.slice(1).trim();
if (!/^[A-Z0-9]+$/i.test(candidate)) {
throw new Error("Slack channels require a channel id (use channel:<id>)");
}
return buildTarget("channel", candidate, trimmed);
}
if (options.defaultKind) {
return buildTarget(options.defaultKind, trimmed, trimmed);
}
return buildTarget("channel", trimmed, trimmed);
}
export function resolveSlackChannelId(raw: string): string {
const target = parseSlackTarget(raw, { defaultKind: "channel" });
if (!target) {
throw new Error("Slack channel id is required.");
}
if (target.kind !== "channel") {
throw new Error("Slack channel id is required (use channel:<id>).");
}
return target.id;
}