From 2efa94575e06048728fd0d25b6866c186b412e74 Mon Sep 17 00:00:00 2001 From: Amit Vijapur Date: Wed, 19 Aug 2026 16:58:05 +0800 Subject: [PATCH 1/2] fix(slack): keep alert attachment content on normalized messages Slack integrations such as Sentry, PagerDuty and GitHub carry their real payload in an attachment's `title`, `text` and `fields`. The adapter only read attachments to build link-unfurl previews, so that content reached nowhere on the normalized Message: `msg.text` held just the one-line summary, and every consumer inherited the gap, including thread.messages, toAiMessages and the chat/ai fetchMessages tool. Fold non-unfurl attachment content into the text before the AST is assembled, so both `formatted` and the derived plain text carry it. Link unfurls stay excluded on the same grounds their blocks already are: the content is not the message author's. `fallback` is used only when the attachment has neither its own fields nor blocks, since it is otherwise a duplicate stand-in for content rendered elsewhere. Mentions inside attachment content are resolved on the async path, matching how table cells are already handled there. Fixes #608 Signed-off-by: Amit Vijapur --- .changeset/quiet-donkeys-listen.md | 5 ++ packages/adapter-slack/src/index.test.ts | 78 ++++++++++++++++++++++++ packages/adapter-slack/src/index.ts | 66 +++++++++++++++++++- 3 files changed, 147 insertions(+), 2 deletions(-) create mode 100644 .changeset/quiet-donkeys-listen.md diff --git a/.changeset/quiet-donkeys-listen.md b/.changeset/quiet-donkeys-listen.md new file mode 100644 index 00000000..38b10bb0 --- /dev/null +++ b/.changeset/quiet-donkeys-listen.md @@ -0,0 +1,5 @@ +--- +"@chat-adapter/slack": patch +--- + +Keep alert attachment content on normalized Slack messages. Attachments that aren't link unfurls now contribute their title, text, and fields instead of being dropped. diff --git a/packages/adapter-slack/src/index.test.ts b/packages/adapter-slack/src/index.test.ts index 1d3510bf..7c5f19c6 100644 --- a/packages/adapter-slack/src/index.test.ts +++ b/packages/adapter-slack/src/index.test.ts @@ -1601,6 +1601,84 @@ describe("parseMessage", () => { }); }); + it("preserves alert attachment content as message content", async () => { + const event: SlackEvent = { + type: "message", + user: "U123", + username: "sentry", + channel: "C456", + text: "New alert", + ts: "1786120899.208429", + attachments: [ + { + fallback: "[Sentry] TypeError in checkout", + title: "TypeError: cannot read property 'id' of undefined", + text: "Occurred 42 times in the last hour.", + fields: [ + { title: "Project", value: "storefront", short: true }, + { title: "Environment", value: "production", short: true }, + ], + }, + ], + }; + const expected = + "New alert\n\n" + + "TypeError: cannot read property 'id' of undefined\n" + + "Occurred 42 times in the last hour.\n" + + "Project: storefront\n" + + "Environment: production"; + + const sync = adapter.parseMessage(event); + const internals = adapter as unknown as { + parseSlackMessage( + value: SlackEvent, + threadId: string + ): Promise>; + }; + const async = await internals.parseSlackMessage( + event, + "slack:C456:1786120899.208429" + ); + + for (const message of [sync, async]) { + expect(message.text).toBe(expected); + } + }); + + it("falls back to attachment fallback only when nothing else carries content", () => { + const message = adapter.parseMessage({ + type: "message", + user: "U123", + channel: "C456", + text: "Deploy finished", + ts: "1786120899.208429", + attachments: [{ fallback: "build #421 succeeded" }], + }); + + expect(message.text).toBe("Deploy finished\n\nbuild #421 succeeded"); + }); + + it("ignores content in unfurl and app attachments", () => { + const message = adapter.parseMessage({ + type: "message", + user: "U123", + channel: "C456", + text: "Check this out", + ts: "1786120899.208429", + attachments: [ + { is_msg_unfurl: true, title: "Foreign title", text: "Foreign text" }, + { is_app_unfurl: true, fallback: "Foreign fallback" }, + { from_url: "https://example.com", title: "Preview" }, + { + original_url: "https://example.com/page", + fields: [{ title: "Key", value: "Value" }], + }, + ], + }); + + expect(message.text).toBe("Check this out"); + }); + it("ignores tables in unfurl and app attachments", () => { const tableBlock = { type: "table", diff --git a/packages/adapter-slack/src/index.ts b/packages/adapter-slack/src/index.ts index 4dade312..aac95c9c 100644 --- a/packages/adapter-slack/src/index.ts +++ b/packages/adapter-slack/src/index.ts @@ -506,12 +506,66 @@ function eventTables(event: SlackEvent): SlackEventTables { }; } +/** + * Legacy attachment content. Alerting integrations (Sentry, PagerDuty, GitHub) + * put the real payload in `title`/`text`/`fields` rather than in blocks, so + * without this the normalized message keeps only the one-line summary. + */ +function attachmentContent( + attachment: NonNullable[number] +): string[] { + const lines: string[] = []; + const push = (value: string | undefined) => { + const trimmed = value?.trim(); + if (trimmed) { + lines.push(trimmed); + } + }; + + push(attachment.title); + push(attachment.text); + for (const field of attachment.fields ?? []) { + const title = field.title?.trim(); + const value = field.value?.trim(); + push(title && value ? `${title}: ${value}` : title || value); + } + + // `fallback` is a plain-text stand-in for content rendered elsewhere, so it + // only adds anything when nothing else on the attachment carried it. + const hasBlocks = + Array.isArray(attachment.blocks) && attachment.blocks.length > 0; + if (lines.length === 0 && !hasBlocks) { + push(attachment.fallback); + } + return lines; +} + +/** + * Attachment content from the author, in attachment order. Unfurls are skipped + * for the same reason their blocks are: the content is not theirs. + */ +function eventAttachmentContent(event: SlackEvent): string[] { + return (event.attachments ?? []) + .filter((attachment) => !isForeignAttachment(attachment)) + .flatMap(attachmentContent); +} + +/** Append attachment content below the message text. */ +function withAttachmentContent(text: string, lines: string[]): string { + if (lines.length === 0) { + return text; + } + const joined = lines.join("\n"); + return text ? `${text}\n\n${joined}` : joined; +} + /** Slack event payload (raw message format) */ export interface SlackEvent { /** Legacy attachments (unfurl previews, app unfurls, etc.) */ attachments?: Array<{ blocks?: SlackMessageBlock[]; fallback?: string; + fields?: Array<{ title?: string; value?: string; short?: boolean }>; from_url?: string; image_url?: string; is_app_unfurl?: boolean; @@ -5661,7 +5715,10 @@ export class SlackAdapter implements Adapter { } protected content(event: SlackEvent, text: string): FormattedContent { - return this.assembleContent(text, eventTables(event)); + return this.assembleContent( + withAttachmentContent(text, eventAttachmentContent(event)), + eventTables(event) + ); } /** @@ -5687,7 +5744,12 @@ export class SlackAdapter implements Adapter { }); const { leading, trailing } = eventTables(event); - return this.assembleContent(text, { + const attachmentLines = await Promise.all( + eventAttachmentContent(event).map((line) => + this.resolveInlineMentions(line, skipSelfMention) + ) + ); + return this.assembleContent(withAttachmentContent(text, attachmentLines), { leading: await Promise.all(leading.map(resolve)), trailing: await Promise.all(trailing.map(resolve)), }); From 44eede3ea0f3a1dc55f22c878481dec50e147eea Mon Sep 17 00:00:00 2001 From: Ben Sabic Date: Fri, 28 Aug 2026 15:50:19 +1000 Subject: [PATCH 2/2] fix(slack): render attachment content faithfully on normalized messages Attachment fields now parse per Slack's rendering rules: title, text, and fields stay plain text unless listed in mrkdwn_in, title links to title_link (also surfaced in message.links), pretext is included, and each part parses in isolation so an unclosed code fence can't swallow the content that follows. Attachments with blocks contribute their tables adjacent to their own text and skip legacy fields like Slack does, with fallback filling in when blocks carry nothing renderable. Mention IDs across table cells and attachment content are resolved in a single parallel lookup wave, and message_changed now parses the pre-edit snapshot through the same async path as the new message so both sides of an edit render mentions identically. Signed-off-by: Ben Sabic --- .changeset/quiet-donkeys-listen.md | 4 +- packages/adapter-slack/sample-messages.md | 6 + packages/adapter-slack/src/index.test.ts | 229 +++++++++- packages/adapter-slack/src/index.ts | 525 ++++++++++++++++------ 4 files changed, 617 insertions(+), 147 deletions(-) diff --git a/.changeset/quiet-donkeys-listen.md b/.changeset/quiet-donkeys-listen.md index 38b10bb0..39b753b1 100644 --- a/.changeset/quiet-donkeys-listen.md +++ b/.changeset/quiet-donkeys-listen.md @@ -2,4 +2,6 @@ "@chat-adapter/slack": patch --- -Keep alert attachment content on normalized Slack messages. Attachments that aren't link unfurls now contribute their title, text, and fields instead of being dropped. +Keep alert attachment content on normalized Slack messages. Attachments that aren't link unfurls now contribute their pretext, title (linked to `title_link` when present, with the URL also surfaced in `message.links`), text, and fields instead of being dropped; `fallback` fills in when nothing else on the attachment carries content. Matching how Slack renders these fields, they are treated as plain text unless listed in the attachment's `mrkdwn_in` array, so literal `*`, `_`, and backticks in alert text survive normalization. Tables inside attachment blocks now stay adjacent to their attachment's text. + +Because attachment content is part of `message.text`, mention detection and `onMessage` pattern handlers see it too: an attachment that quotes the bot's mention routes to `onNewMention`, and patterns match alert text. Handlers that should ignore other integrations' alerts can check `message.author.isBot`. diff --git a/packages/adapter-slack/sample-messages.md b/packages/adapter-slack/sample-messages.md index cf12ba10..e04cf1d5 100644 --- a/packages/adapter-slack/sample-messages.md +++ b/packages/adapter-slack/sample-messages.md @@ -71,3 +71,9 @@ body: '{"token":"xAbCdEfGhIjKlMnOpQrStUvW","team_id":"T00FAKE00AA","context_team_id":"T00FAKE00AA","context_enterprise_id":null,"api_app_id":"A00FAKEAPP01","event":{"type":"message","user":"U00FAKEUSER1","ts":"1786120899.208429","client_msg_id":"8f1c2d3e-45a6-47b8-9c0d-1e2f3a4b5c6d","text":"Which devices support remote firmware upgrades?","team":"T00FAKE00AA","blocks":[{"type":"rich_text","block_id":"tblQ1","elements":[{"type":"rich_text_section","elements":[{"type":"text","text":"Which devices support remote firmware upgrades?"}]}]}],"attachments":[{"id":1,"fallback":"[no preview available]","blocks":[{"type":"table","block_id":"pasted1","rows":[[{"type":"rich_text","elements":[{"type":"rich_text_section","elements":[{"type":"text","text":"Manufacturer","style":{"bold":true}}]}]},{"type":"raw_text","text":"Identifier Listed"},{"type":"raw_text","text":"Units"}],[{"type":"raw_text","text":"Samsung"},{"type":"raw_text","text":"QB55C"},{"type":"raw_number","value":3}]]}]}],"channel":"C00FAKECHAN1","event_ts":"1786120899.208429","channel_type":"channel"},"type":"event_callback","event_id":"Ev0ATABLE001","event_time":1786120899,"authorizations":[{"enterprise_id":null,"team_id":"T00FAKE00AA","user_id":"U00FAKEBOT01","is_bot":true,"is_enterprise_install":false}],"is_ext_shared_channel":false}' } ``` + +```log +[chat-sdk:slack] Slack webhook raw body { + body: '{"token":"xAbCdEfGhIjKlMnOpQrStUvW","team_id":"T00FAKE00AA","context_team_id":"T00FAKE00AA","context_enterprise_id":null,"api_app_id":"A00FAKEAPP01","event":{"type":"message","subtype":"bot_message","ts":"1786292411.318219","text":"","bot_id":"B00FAKEALRT1","username":"Sentry","icons":{"image_48":"https:\\/\\/avatars.slack-edge.com\\/sentry_48.png"},"attachments":[{"id":1,"color":"e03e2f","fallback":"[storefront] TypeError: cannot read property \'id\' of undefined","title":"TypeError: cannot read property \'id\' of undefined","title_link":"https:\\/\\/fake-org.sentry.io\\/issues\\/4501234567\\/?referrer=slack","text":"celery.app.trace in _trace_task\\n\\ncheckout\\/views.py in line 42","fields":[{"title":"Project","value":"storefront","short":true},{"title":"Environment","value":"production","short":true}],"footer":"STOREFRONT-8Q3","ts":1786292410,"mrkdwn_in":["text"]}],"channel":"C00FAKECHAN1","event_ts":"1786292411.318219","channel_type":"channel"},"type":"event_callback","event_id":"Ev0AALERT001","event_time":1786292411,"authorizations":[{"enterprise_id":null,"team_id":"T00FAKE00AA","user_id":"U00FAKEBOT01","is_bot":true,"is_enterprise_install":false}],"is_ext_shared_channel":false}' +} +``` diff --git a/packages/adapter-slack/src/index.test.ts b/packages/adapter-slack/src/index.test.ts index 7c5f19c6..f4193e9e 100644 --- a/packages/adapter-slack/src/index.test.ts +++ b/packages/adapter-slack/src/index.test.ts @@ -1679,6 +1679,229 @@ describe("parseMessage", () => { expect(message.text).toBe("Check this out"); }); + it("keeps attachment formatting characters literal unless mrkdwn_in enables them", () => { + const attachment = { + title: "Cleanup failed in ", + text: "rm -rf /tmp/*cache* failed for _id_ values", + }; + const literal = adapter.parseMessage({ + type: "message", + user: "U123", + channel: "C456", + text: "Alert", + ts: "1786120899.208429", + attachments: [{ ...attachment }], + }); + + // Slack renders attachment text as plain text unless mrkdwn_in lists it + expect(literal.text).toBe( + "Alert\n\n" + + "Cleanup failed in \n" + + "rm -rf /tmp/*cache* failed for _id_ values" + ); + + const mrkdwn = adapter.parseMessage({ + type: "message", + user: "U123", + channel: "C456", + text: "Alert", + ts: "1786120899.208429", + attachments: [ + { ...attachment, mrkdwn_in: ["text"], text: "deploy *failed* badly" }, + ], + }); + + // With mrkdwn_in, *bold* is markup; the title stays plain text + expect(mrkdwn.text).toBe( + "Alert\n\nCleanup failed in \n\ndeploy failed badly" + ); + expect(mrkdwn.formatted.children).toContainEqual( + expect.objectContaining({ + type: "paragraph", + children: expect.arrayContaining([ + expect.objectContaining({ type: "strong" }), + ]), + }) + ); + }); + + it("keeps attachment content out of an unclosed code fence in the body", () => { + const message = adapter.parseMessage({ + type: "message", + user: "U123", + channel: "C456", + text: "Deploy failed:\n```\nTypeError: boom", + ts: "1786120899.208429", + attachments: [ + { + title: "Deploy status", + fields: [{ title: "Environment", value: "production" }], + }, + ], + }); + + // The unclosed fence swallows the rest of the body, but the attachment + // parses in isolation and stays a paragraph. + const types = message.formatted.children.map((child) => child.type); + expect(types).toEqual(["paragraph", "code", "paragraph"]); + expect(message.text).toBe( + "Deploy failed:\n\nTypeError: boom\n\nDeploy status\nEnvironment: production" + ); + }); + + it("uses the fallback when attachment blocks carry nothing renderable", () => { + const message = adapter.parseMessage({ + type: "message", + user: "U123", + channel: "C456", + text: "Heads up", + ts: "1786120899.208429", + attachments: [ + { + fallback: "Deploy failed on step 3", + blocks: [{ type: "section", text: "Deploy failed on step 3" }], + }, + ], + }); + + expect(message.text).toBe("Heads up\n\nDeploy failed on step 3"); + }); + + it("prefers attachment blocks over legacy fields, matching Slack rendering", () => { + const message = adapter.parseMessage({ + type: "message", + user: "U123", + channel: "C456", + text: "Report", + ts: "1786120899.208429", + attachments: [ + { + fallback: "table fallback", + title: "Legacy title Slack does not render", + blocks: [ + { + type: "table", + rows: [ + [ + { type: "raw_text", text: "Region" }, + { type: "raw_text", text: "Status" }, + ], + [ + { type: "raw_text", text: "us-east" }, + { type: "raw_text", text: "down" }, + ], + ], + }, + ], + }, + ], + }); + + expect(message.text).toBe("Report\n\nRegion\tStatus\nus-east\tdown"); + }); + + it("links the attachment title to title_link and surfaces the URL", () => { + const message = adapter.parseMessage({ + type: "message", + user: "U123", + channel: "C456", + text: "New issue", + ts: "1786120899.208429", + attachments: [ + { + title: "TypeError in checkout", + title_link: "https://sentry.example.com/issues/123", + }, + ], + }); + + expect(message.text).toBe("New issue\n\nTypeError in checkout"); + expect(message.formatted.children[1]).toMatchObject({ + type: "paragraph", + children: [ + { + type: "link", + url: "https://sentry.example.com/issues/123", + children: [{ type: "text", value: "TypeError in checkout" }], + }, + ], + }); + expect(message.links.map((link) => link.url)).toContain( + "https://sentry.example.com/issues/123" + ); + }); + + it("keeps each attachment's tables adjacent to its text", () => { + const table = (cell: string) => ({ + type: "table", + rows: [[{ type: "raw_text", text: cell }]], + }); + const message = adapter.parseMessage({ + type: "message", + user: "U123", + channel: "C456", + text: "Two alerts", + ts: "1786120899.208429", + attachments: [ + // Blocks win over legacy fields per attachment, so give each + // attachment either text or a table and check the interleaving. + { title: "Alert A" }, + { blocks: [table("table A")] }, + { title: "Alert B" }, + { blocks: [table("table B")] }, + ], + }); + + expect(message.text).toBe( + "Two alerts\n\nAlert A\n\ntable A\n\nAlert B\n\ntable B" + ); + }); + + it("resolves mentions in attachment content with a single lookup per user", async () => { + const state = createMockState(); + const localAdapter = createSlackAdapter({ + botToken: "xoxb-test-token", + signingSecret: "test-secret", + logger: mockLogger, + botUserId: "U_BOT", + }); + await localAdapter.initialize(createMockChatInstance({ state })); + const internals = localAdapter as unknown as { + _client: { users: { info: unknown } }; + parseSlackMessage( + value: SlackEvent, + threadId: string + ): Promise>; + }; + const usersInfo = vi.fn().mockResolvedValue({ + user: { name: "jane", profile: { display_name: "jane" } }, + }); + internals._client.users.info = usersInfo; + + const message = await internals.parseSlackMessage( + { + type: "message", + user: "U123", + username: "pager", + channel: "C456", + text: "Incident", + ts: "1786120899.208429", + attachments: [ + { + fields: [ + { title: "Primary", value: "<@U777>" }, + { title: "Secondary", value: "<@U777>" }, + ], + }, + ], + }, + "slack:C456:1786120899.208429" + ); + + expect(message.text).toBe("Incident\n\nPrimary: @jane\nSecondary: @jane"); + expect(usersInfo).toHaveBeenCalledTimes(1); + }); + it("ignores tables in unfurl and app attachments", () => { const tableBlock = { type: "table", @@ -4066,6 +4289,7 @@ describe("message subtype handling", () => { const before = { type: "message", user: "U_USER", + username: "user", channel: "C_CHAN", text: "before", ts: "1234567890.111111", @@ -4095,7 +4319,10 @@ describe("message subtype handling", () => { const event = vi.mocked(chatInstance.processMessageUpdated).mock .calls[0]?.[0]; expect(event?.previousMessage).toBeDefined(); - expect((event?.previousMessage as Message).text).toBe("before"); + // The pre-edit snapshot parses through the same async path as the new + // message so mention rendering matches on both sides of the diff. + const previous = await (event?.previousMessage as () => Promise)(); + expect(previous.text).toBe("before"); }); it("ignores a message_changed where nothing actually changed", async () => { diff --git a/packages/adapter-slack/src/index.ts b/packages/adapter-slack/src/index.ts index aac95c9c..1257a0f4 100644 --- a/packages/adapter-slack/src/index.ts +++ b/packages/adapter-slack/src/index.ts @@ -75,6 +75,7 @@ import { encryptToken, isEncryptedTokenData, } from "./crypto"; +import { escapeSlackText, unescapeSlackText } from "./format"; import { SlackFormatConverter } from "./markdown"; import { decodeModalMetadata, @@ -165,6 +166,84 @@ function findNextMention(text: string): number { return Math.min(atIdx, hashIdx); } +/** Resolved display names for mention tokens, keyed by user/channel ID. */ +interface SlackMentionNames { + channels: Map; + users: Map; +} + +/** + * Collect the user and channel IDs referenced by `<@U…>` / `<#C…>` tokens. + * Parses by splitting on angle brackets to avoid ReDoS. + */ +function collectMentionIds( + text: string, + userIds: Set, + channelIds: Set +): void { + for (const segment of text.split("<")) { + const end = segment.indexOf(">"); + if (end === -1) { + continue; + } + const inner = segment.slice(0, end); + if (inner.startsWith("@")) { + const rest = inner.slice(1); + const pipeIdx = rest.indexOf("|"); + const uid = pipeIdx >= 0 ? rest.slice(0, pipeIdx) : rest; + if (SLACK_USER_ID_PATTERN.test(uid)) { + userIds.add(uid); + } + } else if (inner.startsWith("#")) { + const rest = inner.slice(1); + const pipeIdx = rest.indexOf("|"); + // Only collect bare channel IDs (no label already present) + if (pipeIdx === -1 && SLACK_USER_ID_PATTERN.test(rest)) { + channelIds.add(rest); + } + } + } +} + +/** + * Replace `<@U123>`, `<@U123|old>`, and `<#C123>` with resolved names. + * Tokens without a resolved name are left untouched. Uses a split-based + * approach to avoid ReDoS on user-controlled input. + */ +function applyMentionNames(text: string, names: SlackMentionNames): string { + if (names.users.size === 0 && names.channels.size === 0) { + return text; + } + + let result = ""; + let remaining = text; + let startIdx = findNextMention(remaining); + while (startIdx !== -1) { + result += remaining.slice(0, startIdx); + remaining = remaining.slice(startIdx); + const endIdx = remaining.indexOf(">"); + if (endIdx === -1) { + break; + } + const prefix = remaining[1]; // '@' or '#' + const inner = remaining.slice(2, endIdx); // after "<@" or "<#" + const pipeIdx = inner.indexOf("|"); + const id = pipeIdx >= 0 ? inner.slice(0, pipeIdx) : inner; + if (prefix === "@" && SLACK_USER_ID_PATTERN.test(id)) { + const name = names.users.get(id); + result += name ? `<@${id}|${name}>` : remaining.slice(0, endIdx + 1); + } else if (prefix === "#" && pipeIdx === -1 && names.channels.has(id)) { + const name = names.channels.get(id); + result += `<#${id}|${name}>`; + } else { + result += remaining.slice(0, endIdx + 1); + } + remaining = remaining.slice(endIdx + 1); + startIdx = findNextMention(remaining); + } + return result + remaining; +} + /** * Pattern to match Slack message URLs. * Format: https://{workspace}.slack.com/archives/{channelId}/p{timestamp} @@ -185,6 +264,7 @@ const SLACK_MESSAGE_URL_PATTERN = /^https?:\/\/[^/]+\.slack\.com\/archives\/([A-Z0-9]+)\/p(\d+)(?:\?.*)?$/; // Bracketed URL in message text; length-bounded to keep the scan linear. const BRACKETED_URL_PATTERN = /<(https?:\/\/[^>]{1,2048})>/g; +const HTTP_URL_PREFIX_PATTERN = /^https?:\/\//; import type { SlackAdapterConfig, @@ -315,12 +395,11 @@ export interface SlackMessageBlock extends SlackBlock { url?: string; } -type SlackTable = Extract< - FormattedContent["children"][number], - { type: "table" } ->; +type SlackContentNode = FormattedContent["children"][number]; +type SlackTable = Extract; type SlackTableRow = SlackTable["children"][number]; type SlackTableCell = SlackTableRow["children"][number]; +type SlackPhrasing = SlackTableCell["children"][number]; /** Table extracted from a Slack table block, one mrkdwn string per cell. */ interface SlackTableData { @@ -477,9 +556,11 @@ function isForeignAttachment( } /** - * Collect table blocks from the event, split by position. Slack flattens all - * rich text into `event.text`, so exact interleaving can't be reconstructed; - * tables pasted above the text at least stay above it. + * Collect table blocks from the message's own blocks, split by position. + * Slack flattens all rich text into `event.text`, so exact interleaving can't + * be reconstructed; tables pasted above the text at least stay above it. + * Attachment tables are handled per-attachment (see `attachmentContent`) so + * they stay adjacent to the attachment text they belong to. */ function eventTables(event: SlackEvent): SlackEventTables { const blocks = event.blocks ?? []; @@ -494,69 +575,157 @@ function eventTables(event: SlackEvent): SlackEventTables { return parsed ? [parsed] : []; }); - const attachmentBlocks = (event.attachments ?? []) - .filter((attachment) => !isForeignAttachment(attachment)) - .flatMap((attachment) => - Array.isArray(attachment.blocks) ? attachment.blocks : [] - ); - return { leading: parse(blocks.slice(0, splitIdx)), - trailing: [...parse(blocks.slice(splitIdx)), ...parse(attachmentBlocks)], + trailing: parse(blocks.slice(splitIdx)), }; } /** - * Legacy attachment content. Alerting integrations (Sentry, PagerDuty, GitHub) - * put the real payload in `title`/`text`/`fields` rather than in blocks, so + * Attachments authored by the message sender, in attachment order. Unfurls + * are excluded everywhere author attachments are read — content, tables, and + * links alike — because their content is not the author's. + */ +function authorAttachments( + event: SlackEvent +): NonNullable { + return (event.attachments ?? []).filter( + (attachment) => !isForeignAttachment(attachment) + ); +} + +/** + * One piece of attachment text. `mrkdwn` parts go through the format + * converter (formatting characters are markup); `literal` parts render as + * plain text where only Slack control sequences (`<@U…>`, ``, + * entity escapes) are honored, so literal `*`/`_`/backticks survive. + */ +type SlackAttachmentPart = { literal: string } | { mrkdwn: string }; + +/** Renderable content of one attachment. */ +interface SlackAttachmentContent { + parts: SlackAttachmentPart[]; + tables: SlackTableData[]; +} + +/** + * Content of a legacy attachment. Alerting integrations (Sentry, PagerDuty, + * GitHub) put the real payload in `pretext`/`title`/`text`/`fields`, so * without this the normalized message keeps only the one-line summary. + * + * Slack renders these fields as plain text unless they are listed in the + * attachment's `mrkdwn_in` array; the parts mirror that so alert text + * containing shell commands or globs isn't reinterpreted as formatting. + * `title` is always plain text and links to `title_link` when present. + * + * When the attachment carries blocks, Slack renders only the blocks and + * ignores the legacy fields — mirrored here by preferring extracted tables. + * Block types the parser can't render fall back to the legacy fields, and + * `fallback` (the attachment's plain-text stand-in) fills in last so an + * attachment never silently contributes nothing. */ function attachmentContent( attachment: NonNullable[number] -): string[] { - const lines: string[] = []; - const push = (value: string | undefined) => { - const trimmed = value?.trim(); - if (trimmed) { - lines.push(trimmed); +): SlackAttachmentContent { + const tables = (attachment.blocks ?? []).flatMap((block) => { + const parsed = tableData(block); + return parsed ? [parsed] : []; + }); + + const parts: SlackAttachmentPart[] = []; + if (tables.length === 0) { + const mrkdwnIn = new Set(attachment.mrkdwn_in ?? []); + const push = (value: string | undefined, mrkdwn: boolean) => { + const trimmed = value?.trim(); + if (trimmed) { + parts.push(mrkdwn ? { mrkdwn: trimmed } : { literal: trimmed }); + } + }; + + push(attachment.pretext, mrkdwnIn.has("pretext")); + const title = attachment.title?.trim(); + if (attachment.title_link) { + // Fold the link in as a control sequence so the title renders as a + // link node and the URL survives into the normalized message. + push( + title + ? `<${attachment.title_link}|${escapeSlackText(title)}>` + : `<${attachment.title_link}>`, + false + ); + } else { + push(title, false); + } + push(attachment.text, mrkdwnIn.has("text")); + for (const field of attachment.fields ?? []) { + const fieldTitle = field.title?.trim(); + const value = field.value?.trim(); + push( + fieldTitle && value ? `${fieldTitle}: ${value}` : fieldTitle || value, + mrkdwnIn.has("fields") + ); } - }; - push(attachment.title); - push(attachment.text); - for (const field of attachment.fields ?? []) { - const title = field.title?.trim(); - const value = field.value?.trim(); - push(title && value ? `${title}: ${value}` : title || value); + if (parts.length === 0) { + push(attachment.fallback, false); + } } - // `fallback` is a plain-text stand-in for content rendered elsewhere, so it - // only adds anything when nothing else on the attachment carried it. - const hasBlocks = - Array.isArray(attachment.blocks) && attachment.blocks.length > 0; - if (lines.length === 0 && !hasBlocks) { - push(attachment.fallback); - } - return lines; + return { parts, tables }; } /** - * Attachment content from the author, in attachment order. Unfurls are skipped - * for the same reason their blocks are: the content is not theirs. + * Render one line of plain-text Slack content to phrasing nodes. Control + * sequences are honored the same way `slackMrkdwnToMarkdown` renders them + * (`<@U…|name>` → `@name`, `` → link), but nothing is parsed as + * markdown — formatting characters stay literal. */ -function eventAttachmentContent(event: SlackEvent): string[] { - return (event.attachments ?? []) - .filter((attachment) => !isForeignAttachment(attachment)) - .flatMap(attachmentContent); -} +function literalPhrasing(line: string): SlackPhrasing[] { + const children: SlackPhrasing[] = []; + let plain = ""; + const flushPlain = () => { + if (plain) { + children.push({ type: "text", value: unescapeSlackText(plain) }); + plain = ""; + } + }; -/** Append attachment content below the message text. */ -function withAttachmentContent(text: string, lines: string[]): string { - if (lines.length === 0) { - return text; + let remaining = line; + while (remaining.length > 0) { + const start = remaining.indexOf("<"); + const end = start === -1 ? -1 : remaining.indexOf(">", start + 1); + if (end === -1) { + plain += remaining; + break; + } + plain += remaining.slice(0, start); + const token = remaining.slice(start, end + 1); + const inner = remaining.slice(start + 1, end); + remaining = remaining.slice(end + 1); + + const pipeIdx = inner.indexOf("|"); + const target = pipeIdx === -1 ? inner : inner.slice(0, pipeIdx); + const label = pipeIdx === -1 ? undefined : inner.slice(pipeIdx + 1); + const id = target.slice(1); + if (target.startsWith("@") && SLACK_USER_ID_PATTERN.test(id)) { + plain += `@${label ?? id}`; + } else if (target.startsWith("#") && SLACK_USER_ID_PATTERN.test(id)) { + plain += label ? `#${label} (${id})` : `#${id}`; + } else if (HTTP_URL_PREFIX_PATTERN.test(target)) { + flushPlain(); + children.push({ + type: "link", + url: target, + children: [ + { type: "text", value: label ? unescapeSlackText(label) : target }, + ], + }); + } else { + plain += token; + } } - const joined = lines.join("\n"); - return text ? `${text}\n\n${joined}` : joined; + flushPlain(); + return children; } /** Slack event payload (raw message format) */ @@ -570,7 +739,10 @@ export interface SlackEvent { image_url?: string; is_app_unfurl?: boolean; is_msg_unfurl?: boolean; + /** Field names Slack renders as mrkdwn ("pretext", "text", "fields") */ + mrkdwn_in?: string[]; original_url?: string; + pretext?: string; service_icon?: string; service_name?: string; text?: string; @@ -3032,23 +3204,35 @@ export class SlackAdapter implements Adapter { const threadId = this.threadIdForMessageEvent(normalized); // Slack sends the pre-edit message alongside the new one. Forward it so - // handlers can diff the change instead of only seeing the result. + // handlers can diff the change instead of only seeing the result. Parse + // it with the same async path as the new message so mentions render + // identically on both sides and unchanged content doesn't diff. const before = event.previous_message; + const parsePreviousMessage = before + ? async (): Promise> => { + const snapshot = { + ...before, + channel: before.channel ?? normalized.channel, + channel_type: before.channel_type ?? normalized.channel_type, + type: before.type ?? "message", + }; + try { + return await this.parseSlackMessage(snapshot, threadId); + } catch (error) { + // Never let a lookup failure on the old snapshot drop the edit + this.logger.warn( + "Falling back to sync parse for pre-edit message", + { error, threadId } + ); + return this.parseSlackMessageSync(snapshot, threadId); + } + } + : undefined; this.chat.processMessageUpdated( { adapter: this, message: () => this.parseSlackMessage(normalized, threadId), - previousMessage: before - ? this.parseSlackMessageSync( - { - ...before, - channel: before.channel ?? normalized.channel, - channel_type: before.channel_type ?? normalized.channel_type, - type: before.type ?? "message", - }, - threadId - ) - : undefined, + previousMessage: parsePreviousMessage, threadId, }, options @@ -3554,44 +3738,34 @@ export class SlackAdapter implements Adapter { ): Promise { const userIds = new Set(); const channelIds = new Set(); - // Parse mentions by splitting on angle brackets to avoid ReDoS - for (const segment of text.split("<")) { - const end = segment.indexOf(">"); - if (end === -1) { - continue; - } - const inner = segment.slice(0, end); - if (inner.startsWith("@")) { - const rest = inner.slice(1); - const pipeIdx = rest.indexOf("|"); - const uid = pipeIdx >= 0 ? rest.slice(0, pipeIdx) : rest; - if (SLACK_USER_ID_PATTERN.test(uid)) { - userIds.add(uid); - } - } else if (inner.startsWith("#")) { - const rest = inner.slice(1); - const pipeIdx = rest.indexOf("|"); - // Only collect bare channel IDs (no label already present) - if (pipeIdx === -1 && SLACK_USER_ID_PATTERN.test(rest)) { - channelIds.add(rest); - } - } - } - if (userIds.size === 0 && channelIds.size === 0) { - return text; - } + collectMentionIds(text, userIds, channelIds); + const names = await this.lookupMentionNames( + userIds, + channelIds, + skipSelfMention + ); + return applyMentionNames(text, names); + } - // Don't resolve the bot's own mention when processing incoming webhooks — - // detectMention needs @botUserId in the text + /** + * Look up display names for collected mention IDs in one parallel wave. + * Mutates `userIds`: the bot's own ID is removed when `skipSelfMention` is + * set — detectMention needs @botUserId to stay in the text (see + * `resolveInlineMentions`). + */ + private async lookupMentionNames( + userIds: Set, + channelIds: Set, + skipSelfMention: boolean + ): Promise { const currentBotUserId = this.botUserId; if (skipSelfMention && currentBotUserId) { userIds.delete(currentBotUserId); } if (userIds.size === 0 && channelIds.size === 0) { - return text; + return { channels: new Map(), users: new Map() }; } - // Look up all mentioned users and channels in parallel const [userLookups, channelLookups] = await Promise.all([ Promise.all( [...userIds].map(async (uid) => { @@ -3606,38 +3780,7 @@ export class SlackAdapter implements Adapter { }) ), ]); - const userNameMap = new Map(userLookups); - const channelNameMap = new Map(channelLookups); - - // Replace <@U123>, <@U123|old>, and <#C123> with resolved names - // Use split-based approach to avoid ReDoS on user-controlled input - let result = ""; - let remaining = text; - let startIdx = findNextMention(remaining); - while (startIdx !== -1) { - result += remaining.slice(0, startIdx); - remaining = remaining.slice(startIdx); - const endIdx = remaining.indexOf(">"); - if (endIdx === -1) { - break; - } - const prefix = remaining[1]; // '@' or '#' - const inner = remaining.slice(2, endIdx); // after "<@" or "<#" - const pipeIdx = inner.indexOf("|"); - const id = pipeIdx >= 0 ? inner.slice(0, pipeIdx) : inner; - if (prefix === "@" && SLACK_USER_ID_PATTERN.test(id)) { - const name = userNameMap.get(id); - result += name ? `<@${id}|${name}>` : `<@${id}>`; - } else if (prefix === "#" && pipeIdx === -1 && channelNameMap.has(id)) { - const name = channelNameMap.get(id); - result += `<#${id}|${name}>`; - } else { - result += remaining.slice(0, endIdx + 1); - } - remaining = remaining.slice(endIdx + 1); - startIdx = findNextMention(remaining); - } - return result + remaining; + return { channels: new Map(channelLookups), users: new Map(userLookups) }; } /** @@ -3696,6 +3839,11 @@ export class SlackAdapter implements Adapter { }); urls.add(attUrl); } + // Alert attachments link their title (e.g. the Sentry issue URL); + // surface it so handlers can reach what the Slack UI links to. + if (att.title_link && !isForeignAttachment(att)) { + urls.add(att.title_link); + } } } @@ -5716,48 +5864,88 @@ export class SlackAdapter implements Adapter { protected content(event: SlackEvent, text: string): FormattedContent { return this.assembleContent( - withAttachmentContent(text, eventAttachmentContent(event)), - eventTables(event) + text, + eventTables(event), + authorAttachments(event).flatMap((attachment) => + this.attachmentNodes(attachmentContent(attachment)) + ) ); } /** * Like `content`, but resolves user and channel mentions inside table - * cells the same way `resolveInlineMentions` resolves them in body text. + * cells and attachment content the same way `resolveInlineMentions` + * resolves them in body text. All mention IDs are collected up front and + * looked up in a single parallel wave so no ID is fetched twice. */ protected async resolvedContent( event: SlackEvent, text: string, skipSelfMention: boolean ): Promise { - const resolve = async (data: SlackTableData): Promise => ({ + const { leading, trailing } = eventTables(event); + const attachments = authorAttachments(event).map(attachmentContent); + + const userIds = new Set(); + const channelIds = new Set(); + const collectTable = (data: SlackTableData) => { + for (const row of data.rows) { + for (const cell of row) { + collectMentionIds(cell, userIds, channelIds); + } + } + }; + for (const data of [...leading, ...trailing]) { + collectTable(data); + } + for (const attachment of attachments) { + for (const data of attachment.tables) { + collectTable(data); + } + for (const part of attachment.parts) { + collectMentionIds( + "mrkdwn" in part ? part.mrkdwn : part.literal, + userIds, + channelIds + ); + } + } + const names = await this.lookupMentionNames( + userIds, + channelIds, + skipSelfMention + ); + + const resolveTable = (data: SlackTableData): SlackTableData => ({ ...data, - rows: await Promise.all( - data.rows.map((row) => - Promise.all( - row.map((cell) => - cell ? this.resolveInlineMentions(cell, skipSelfMention) : cell - ) - ) - ) + rows: data.rows.map((row) => + row.map((cell) => (cell ? applyMentionNames(cell, names) : cell)) ), }); + const resolvePart = (part: SlackAttachmentPart): SlackAttachmentPart => + "mrkdwn" in part + ? { mrkdwn: applyMentionNames(part.mrkdwn, names) } + : { literal: applyMentionNames(part.literal, names) }; - const { leading, trailing } = eventTables(event); - const attachmentLines = await Promise.all( - eventAttachmentContent(event).map((line) => - this.resolveInlineMentions(line, skipSelfMention) + return this.assembleContent( + text, + { + leading: leading.map(resolveTable), + trailing: trailing.map(resolveTable), + }, + attachments.flatMap((attachment) => + this.attachmentNodes({ + parts: attachment.parts.map(resolvePart), + tables: attachment.tables.map(resolveTable), + }) ) ); - return this.assembleContent(withAttachmentContent(text, attachmentLines), { - leading: await Promise.all(leading.map(resolve)), - trailing: await Promise.all(trailing.map(resolve)), - }); } private assembleContent( text: string, - tables: SlackEventTables + tables: SlackEventTables, + attachments: SlackContentNode[] = [] ): FormattedContent { const formatted = this.formatConverter.toAst(text); formatted.children.unshift( @@ -5766,9 +5954,56 @@ export class SlackAdapter implements Adapter { formatted.children.push( ...tables.trailing.map((data) => this.tableNode(data)) ); + formatted.children.push(...attachments); return formatted; } + /** + * Render one attachment's content to block nodes. Literal lines share a + * paragraph (separated by hard breaks) so an attachment reads as one block; + * mrkdwn parts are parsed in isolation so an unclosed code fence in the + * message body or another attachment can't swallow this one's content. + * Tables from the attachment's blocks follow its text, keeping each + * attachment's content adjacent. + */ + private attachmentNodes(content: SlackAttachmentContent): SlackContentNode[] { + const nodes: SlackContentNode[] = []; + let lines: SlackPhrasing[][] = []; + const flush = () => { + if (lines.length === 0) { + return; + } + const children: SlackPhrasing[] = []; + for (const line of lines) { + if (children.length > 0) { + children.push({ type: "break" }); + } + children.push(...line); + } + nodes.push({ type: "paragraph", children }); + lines = []; + }; + + for (const part of content.parts) { + if ("mrkdwn" in part) { + flush(); + nodes.push(...this.formatConverter.toAst(part.mrkdwn).children); + } else { + for (const line of part.literal.split("\n")) { + if (line.trim()) { + lines.push(literalPhrasing(line)); + } else { + // A blank line inside a literal part starts a new paragraph + flush(); + } + } + } + } + flush(); + nodes.push(...content.tables.map((data) => this.tableNode(data))); + return nodes; + } + private tableNode(data: SlackTableData): SlackTable { const rows: SlackTableRow[] = data.rows.map((row) => ({ type: "tableRow",