From 5611bbf27ee825e692bc7eb19a24cd9cb37d952f Mon Sep 17 00:00:00 2001 From: Darwin Wu Date: Mon, 21 Sep 2026 17:45:10 -0700 Subject: [PATCH] fix(message): keep link handling Slack-native --- README.md | 2 +- skills/agent-slack/SKILL.md | 2 +- src/cli/message-actions.ts | 4 ++- src/cli/message-draft-actions.ts | 4 ++- src/cli/message-url-warning.ts | 11 ++++++ src/slack/rich-text.ts | 41 +++++++++++++++++++-- test/message-send.test.ts | 61 +++++++++++++++++++++++++++++++- test/rich-text.test.ts | 31 ++++++++++++++-- 8 files changed, 147 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 7fb9c07..5ae325e 100644 --- a/README.md +++ b/README.md @@ -299,7 +299,7 @@ agent-slack message edit "#general" "Updated text" --workspace "myteam" --ts "17 agent-slack message delete "#general" --workspace "myteam" --ts "1770165109.628379" ``` -`message edit` and ordinary `message send` calls convert bullet/numbered lists to Slack native rich text. `message send --blocks` uses the supplied blocks instead, while `message send --attach` sends its initial comment as plain text without automatic list conversion. Inside auto-converted lists, inline mentions, broadcasts, emoji shortcodes, `<#C...>` channel references, and Slack manual links such as `` are preserved as Slack elements. CommonMark links such as `[PR #42](https://example.com/pull/42)` are not converted into labeled link elements. +`message edit` and ordinary `message send` calls convert bullet/numbered lists to Slack native rich text. `message send --blocks` uses the supplied blocks instead, while `message send --attach` sends its initial comment as plain text without automatic list conversion. Bare HTTP(S) URLs and Slack manual links such as `` remain clickable inside auto-converted lists. CommonMark links such as `[PR #42](https://example.com/pull/42)` are sent literally and produce a warning; use Slack syntax for labeled links. Inline mentions, broadcasts, emoji shortcodes, and `<#C...>` channel references are preserved as Slack elements. Send options for `message send`: diff --git a/skills/agent-slack/SKILL.md b/skills/agent-slack/SKILL.md index c1143e1..0a49b74 100644 --- a/skills/agent-slack/SKILL.md +++ b/skills/agent-slack/SKILL.md @@ -37,7 +37,7 @@ Named `later remind --in` values such as `tomorrow` or `monday` also use the exe Use `--no-unfurl` with `message send` or `message compose` when the user wants Slack link and media previews suppressed. It cannot be combined with `message send --attach`. -Ordinary `message send` and `message edit` calls auto-convert lists. `message send --blocks` and `message edit --blocks` use supplied Block Kit blocks, while `message send --attach` sends its initial comment without automatic list conversion. Inside auto-converted lists, use Slack's `` syntax because CommonMark `[label](URL)` links are not converted into labeled link elements. +Ordinary `message send` and `message edit` calls auto-convert lists. `message send --blocks` and `message edit --blocks` use supplied Block Kit blocks, while `message send --attach` sends its initial comment without automatic list conversion. Use bare HTTP(S) URLs or Slack's `` syntax. CommonMark `[label](URL)` links are sent literally and produce a warning. Slack-native drafts (`message draft list|create|update|delete`) manage drafts that appear in the user's Slack client; `create` posts nothing. `create` and `update` accept repeatable `--attach `; on `update` the files are added to the draft's existing attachments rather than replacing them. They use undocumented session endpoints and require browser-style auth (xoxc/xoxd). diff --git a/src/cli/message-actions.ts b/src/cli/message-actions.ts index 7de5045..71092d2 100644 --- a/src/cli/message-actions.ts +++ b/src/cli/message-actions.ts @@ -4,7 +4,7 @@ import { fetchMessage } from "../slack/messages.ts"; import { parseMsgTarget } from "./targets.ts"; import { resolveChannelId, openDmChannel } from "../slack/channels.ts"; import { normalizeSlackReactionName } from "../slack/emoji.ts"; -import { warnOnTruncatedSlackUrl } from "./message-url-warning.ts"; +import { warnOnMarkdownLinkSyntax, warnOnTruncatedSlackUrl } from "./message-url-warning.ts"; import { textToRichTextBlocks } from "../slack/rich-text.ts"; import { formatOutboundSlackText } from "../slack/format-outbound.ts"; import type { SlackApiClient } from "../slack/client.ts"; @@ -135,6 +135,7 @@ export async function sendMessage(input: { "--no-unfurl cannot be combined with --attach (Slack file uploads do not accept unfurl parameters).", ); } + warnOnMarkdownLinkSyntax(input.text); const formattedText = formatOutboundSlackText(input.text); const blocks = input.options.blocks ? loadBlocksFromPath(input.options.blocks) @@ -332,6 +333,7 @@ export async function editMessage(input: { ); } const workspaceUrl = input.ctx.effectiveWorkspaceUrl(input.options.workspace); + warnOnMarkdownLinkSyntax(input.text); const formattedText = formatOutboundSlackText(input.text); const blocks = input.options.blocks ? loadBlocksFromPath(input.options.blocks) diff --git a/src/cli/message-draft-actions.ts b/src/cli/message-draft-actions.ts index 8baa3f3..7b15cd6 100644 --- a/src/cli/message-draft-actions.ts +++ b/src/cli/message-draft-actions.ts @@ -1,6 +1,6 @@ import type { CliContext } from "./context.ts"; import { parseMsgTarget, type MsgTarget } from "./targets.ts"; -import { warnOnTruncatedSlackUrl } from "./message-url-warning.ts"; +import { warnOnMarkdownLinkSyntax, warnOnTruncatedSlackUrl } from "./message-url-warning.ts"; import { normalizeChannelInput, openDmChannel, resolveChannelId } from "../slack/channels.ts"; import type { SlackApiClient } from "../slack/client.ts"; import { @@ -42,6 +42,7 @@ export async function createDraftAction(input: { text: string; options: { workspace?: string; threadTs?: string; broadcast?: boolean; attach?: string[] }; }): Promise> { + warnOnMarkdownLinkSyntax(input.text); const target = parseMsgTarget(String(input.targetInput)); const workspaceUrl = target.kind === "url" @@ -115,6 +116,7 @@ export async function updateDraftAction(input: { attach?: string[]; }; }): Promise> { + warnOnMarkdownLinkSyntax(input.text); const channelTarget = input.options.channel ? parseMsgTarget(String(input.options.channel)) : undefined; diff --git a/src/cli/message-url-warning.ts b/src/cli/message-url-warning.ts index ac7f129..fdb1ce1 100644 --- a/src/cli/message-url-warning.ts +++ b/src/cli/message-url-warning.ts @@ -6,3 +6,14 @@ export function warnOnTruncatedSlackUrl(ref: { possiblyTruncated?: boolean }): v ); } } + +const MARKDOWN_LINK_RE = /(^|[^!\\])\[[^\]\n]+\]\((?:https?:\/\/|mailto:)[^\s)]+/i; + +/** Warn when message text appears to use unsupported Markdown link syntax. */ +export function warnOnMarkdownLinkSyntax(text: string): void { + if (MARKDOWN_LINK_RE.test(text)) { + process.stderr.write( + "Warning: Markdown-style links are not converted. Use Slack link syntax: .\n", + ); + } +} diff --git a/src/slack/rich-text.ts b/src/slack/rich-text.ts index 2572666..5c53dd2 100644 --- a/src/slack/rich-text.ts +++ b/src/slack/rich-text.ts @@ -42,12 +42,13 @@ const BLOCKQUOTE_RE = /^> (.*)$/; /** * Parse mrkdwn inline formatting into Slack rich_text inline elements. * - * Handles: *bold*, _italic_, ~strike~, `code`, :emoji:, , + * Handles: *bold*, _italic_, ~strike~, `code`, :emoji:, , , + * and bare HTTP(S) URLs. */ export function parseInlineElements(text: string): InlineElement[] { const elements: InlineElement[] = []; const re = - /`([^`]+)`|(?:^|(?<=[^A-Za-z0-9_])):([a-zA-Z0-9_+-]+):(?![A-Za-z0-9_+-])|\*([^*]+)\*|_([^_]+)_|~([^~]+)~|<@([UWB][A-Z0-9]+)(?:\|[^>]*)?>|<#([CG][A-Z0-9]+)(?:\|[^>]*)?>|]*)?>|]*)?>|<([^>|]+)\|([^>]+)>|<([^>|]+)>|(?:^|(?<=[^A-Za-z0-9_]))@([UWB][A-Z0-9]{6,})\b|(?:^|(?<=[^A-Za-z0-9_]))@(here|channel|everyone)\b/g; + /`([^`]+)`|(?:^|(?<=[^A-Za-z0-9_])):([a-zA-Z0-9_+-]+):(?![A-Za-z0-9_+-])|\*([^*]+)\*|_([^_]+)_|~([^~]+)~|<@([UWB][A-Z0-9]+)(?:\|[^>]*)?>|<#([CG][A-Z0-9]+)(?:\|[^>]*)?>|]*)?>|]*)?>|<([^>|]+)\|([^>]+)>|<([^>|]+)>|(?]+)|(?:^|(?<=[^A-Za-z0-9_]))@([UWB][A-Z0-9]{6,})\b|(?:^|(?<=[^A-Za-z0-9_]))@(here|channel|everyone)\b/g; let lastIndex = 0; let match: RegExpExecArray | null; @@ -76,6 +77,7 @@ export function parseInlineElements(text: string): InlineElement[] { linkUrl, linkText, bareUrl, + plainUrl, bareUserId, bareBroadcast, ] = match; @@ -108,6 +110,10 @@ export function parseInlineElements(text: string): InlineElement[] { elements.push({ type: "link", url: bareUrl }); } else if (bareUrl != null) { elements.push({ type: "text", text: `<${bareUrl}>` }); + } else if (plainUrl != null) { + const { url, trailingText } = splitBareUrlTrailingText(plainUrl); + elements.push({ type: "link", url }); + pushText(trailingText); } else if (bareUserId != null) { elements.push({ type: "user", user_id: bareUserId }); } else if (bareBroadcast != null) { @@ -131,6 +137,37 @@ function isSlackManualLinkUrl(value: string): boolean { return /^(?:https?:\/\/|mailto:)/i.test(value); } +function splitBareUrlTrailingText(value: string): { url: string; trailingText: string } { + let urlEnd = value.length; + + while (urlEnd > 0) { + const candidate = value.slice(0, urlEnd); + const lastCharacter = candidate.at(-1)!; + if (/[.,!?;:]/.test(lastCharacter)) { + urlEnd--; + continue; + } + + const openingCharacter = ({ ")": "(", "]": "[", "}": "{" } as const)[lastCharacter]; + if (openingCharacter != null) { + const openingCount = countCharacter(candidate, openingCharacter); + const closingCount = countCharacter(candidate, lastCharacter); + if (closingCount > openingCount) { + urlEnd--; + continue; + } + } + + break; + } + + return { url: value.slice(0, urlEnd), trailingText: value.slice(urlEnd) }; +} + +function countCharacter(value: string, character: string): number { + return value.split(character).length - 1; +} + /** * Convert mrkdwn text to Slack rich_text blocks when bullet or numbered * lists are detected. Returns `null` when the text contains no lists, diff --git a/test/message-send.test.ts b/test/message-send.test.ts index 12ff5e3..04892ab 100644 --- a/test/message-send.test.ts +++ b/test/message-send.test.ts @@ -1,4 +1,4 @@ -import { afterEach, describe, expect, mock, test } from "bun:test"; +import { afterEach, describe, expect, mock, spyOn, test } from "bun:test"; import { mkdtemp, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -600,6 +600,65 @@ describe("sendMessage", () => { ]); }); + test("warns about Markdown-style links without rewriting them", async () => { + const calls: { method: string; params: Record }[] = []; + const ctx = createContext(calls); + const stderr = spyOn(process.stderr, "write").mockImplementation(() => true); + + try { + await sendMessage({ + ctx, + targetInput: "C12345678", + text: "Review [PR #42](https://example.com/pull/42)", + options: {}, + }); + + expect(stderr).toHaveBeenCalledWith(expect.stringContaining("Slack link syntax")); + expect(calls[0]?.params.text).toBe("Review [PR #42](https://example.com/pull/42)"); + expect(calls[0]?.params.blocks).toBeUndefined(); + } finally { + stderr.mockRestore(); + } + }); + + test("sends bare URLs in lists as rich-text link blocks", async () => { + const calls: { method: string; params: Record }[] = []; + const ctx = createContext(calls); + + await sendMessage({ + ctx, + targetInput: "C12345678", + text: "I got another PR in: https://example.com/pull/42\n\n- Passenger one", + options: {}, + }); + + expect(calls[0]?.params.blocks).toEqual([ + { + type: "rich_text", + elements: [ + { + type: "rich_text_section", + elements: [ + { type: "text", text: "I got another PR in: " }, + { type: "link", url: "https://example.com/pull/42" }, + { type: "text", text: "\n" }, + ], + }, + { + type: "rich_text_list", + style: "bullet", + elements: [ + { + type: "rich_text_section", + elements: [{ type: "text", text: "Passenger one" }], + }, + ], + }, + ], + }, + ]); + }); + test("--blocks: errors when an array element is not an object", async () => { const calls: { method: string; params: Record }[] = []; const ctx = createContext(calls); diff --git a/test/rich-text.test.ts b/test/rich-text.test.ts index e619bc6..bf074b8 100644 --- a/test/rich-text.test.ts +++ b/test/rich-text.test.ts @@ -74,6 +74,29 @@ describe("parseInlineElements", () => { ]); }); + test("bare URLs are parsed as links", () => { + expect(parseInlineElements("Visit https://example.com/docs")).toEqual([ + { type: "text", text: "Visit " }, + { type: "link", url: "https://example.com/docs" }, + ]); + }); + + test("bare URL punctuation and unmatched closing delimiters remain text", () => { + expect(parseInlineElements("See (https://example.com/docs), then continue.")).toEqual([ + { type: "text", text: "See (" }, + { type: "link", url: "https://example.com/docs" }, + { type: "text", text: ")," }, + { type: "text", text: " then continue." }, + ]); + }); + + test("bare URLs inside code remain code", () => { + expect(parseInlineElements("Run `curl https://example.com/docs`")).toEqual([ + { type: "text", text: "Run " }, + { type: "text", text: "curl https://example.com/docs", style: { code: true } }, + ]); + }); + test("non-url angle bracket text is preserved as text", () => { expect(parseInlineElements("Use ")).toEqual([ { type: "text", text: "Use " }, @@ -269,9 +292,9 @@ describe("textToRichTextBlocks", () => { ]); }); - test("Slack manual links and CommonMark links remain distinct in list items", () => { + test("Slack manual, CommonMark, and bare links remain distinct in list items", () => { const result = textToRichTextBlocks( - "- Review \n- Review [PR #43](https://example.com/pull/43)", + "- Review \n- Review [PR #43](https://example.com/pull/43)\n- Review https://example.com/pull/44", )!; const list = result[0]!.elements.find((e) => e.type === "rich_text_list") as { elements: { elements: unknown[] }[]; @@ -283,6 +306,10 @@ describe("textToRichTextBlocks", () => { expect(list.elements[1]!.elements).toEqual([ { type: "text", text: "Review [PR #43](https://example.com/pull/43)" }, ]); + expect(list.elements[2]!.elements).toEqual([ + { type: "text", text: "Review " }, + { type: "link", url: "https://example.com/pull/44" }, + ]); }); test("code block is preserved", () => {