diff --git a/apps/dashboard/src/components/content/process-message.tsx b/apps/dashboard/src/components/content/process-message.tsx index 791a1f8e..23deb35e 100644 --- a/apps/dashboard/src/components/content/process-message.tsx +++ b/apps/dashboard/src/components/content/process-message.tsx @@ -2,6 +2,7 @@ import type { AnchorHTMLAttributes } from "react"; import { Fragment, createElement } from "react"; import { jsx, jsxs } from "react/jsx-runtime"; import rehypeReact from "rehype-react"; +import remarkGfm from "remark-gfm"; import remarkParse from "remark-parse"; import remarkRehype from "remark-rehype"; import { unified } from "unified"; @@ -9,6 +10,7 @@ import { unified } from "unified"; export function ProcessMessage({ value }: { value: string }) { const result = unified() .use(remarkParse) + .use(remarkGfm) .use(remarkRehype) .use(rehypeReact, { createElement, diff --git a/apps/server/src/libs/test/doubles/slack-test-state.ts b/apps/server/src/libs/test/doubles/slack-test-state.ts index 4698fddb..7519e052 100644 --- a/apps/server/src/libs/test/doubles/slack-test-state.ts +++ b/apps/server/src/libs/test/doubles/slack-test-state.ts @@ -26,6 +26,8 @@ export interface SlackTestState { usersInfoImpl: (args: Record) => Promise; /** `reactions.get` result; the default message has no reactions. */ reactionsGetImpl: (args: Record) => Promise; + /** `conversations.info` result; the default channel has no name. */ + conversationsInfoImpl: (args: Record) => Promise; } const g = globalThis as Record; @@ -52,6 +54,7 @@ if (!g.__slackTestState) { }), usersInfoImpl: () => Promise.resolve({ ok: true, user: { profile: {} } }), reactionsGetImpl: () => Promise.resolve({ ok: true, message: {} }), + conversationsInfoImpl: () => Promise.resolve({ ok: true, channel: {} }), } satisfies SlackTestState; } diff --git a/apps/server/src/libs/test/doubles/slack-web-api.mock.ts b/apps/server/src/libs/test/doubles/slack-web-api.mock.ts index 01a73fc1..dc301130 100644 --- a/apps/server/src/libs/test/doubles/slack-web-api.mock.ts +++ b/apps/server/src/libs/test/doubles/slack-web-api.mock.ts @@ -81,6 +81,10 @@ export class WebClient { s.calls.push({ method: "conversations.history", args }); return s.historyImpl(); }, + info: (args: Record) => { + s.calls.push({ method: "conversations.info", args }); + return s.conversationsInfoImpl(args); + }, }; agents = { sessions: { diff --git a/apps/server/src/routes/slack/handler.test.ts b/apps/server/src/routes/slack/handler.test.ts index 425aab02..8b8cd958 100644 --- a/apps/server/src/routes/slack/handler.test.ts +++ b/apps/server/src/routes/slack/handler.test.ts @@ -2043,6 +2043,8 @@ describe("incident channel events", () => { resetSlackTestState(); slackTestState.reactionsGetImpl = () => Promise.resolve({ ok: true, message: {} }); + slackTestState.conversationsInfoImpl = () => + Promise.resolve({ ok: true, channel: {} }); channelId = `C_PIN_${crypto.randomUUID()}`; const [row] = await db .insert(incident) @@ -2101,6 +2103,165 @@ describe("incident channel events", () => { expect(rows[0].message).toContain("https://slack.test/archives/"); }); + test("rich_text wins over mrkdwn: emoji, styles, mentions and channels", async () => { + // U1 is the pinner and must stay linked; U9 has no email, so the + // profile name is the only thing we know about them. + slackTestState.usersInfoImpl = (args) => + Promise.resolve( + args.user === "U9" + ? { ok: true, user: { real_name: "Jane Doe", profile: {} } } + : { ok: true, user: { profile: { email: "ping@openstatus.dev" } } }, + ); + slackTestState.conversationsInfoImpl = () => + Promise.resolve({ ok: true, channel: { name: "inc-db" } }); + slackTestState.historyImpl = () => + Promise.resolve({ + messages: [ + { + ts: "503.1", + user: "U2", + text: "*rolled back* :face_holding_back_tears: <@U9> see <#C7|inc-db>", + blocks: [ + { + type: "rich_text", + elements: [ + { + type: "rich_text_section", + elements: [ + { + type: "text", + text: "rolled back", + style: { bold: true }, + }, + { type: "text", text: " " }, + { + type: "emoji", + name: "face_holding_back_tears", + unicode: "1f979", + }, + { type: "text", text: " " }, + { type: "user", user_id: "U9" }, + { type: "text", text: " see " }, + { type: "channel", channel_id: "C7" }, + ], + }, + ], + }, + ], + }, + ], + }); + await pin("503.1"); + await waitForCall("reactions.add"); + const rows = await notes(); + expect(rows).toHaveLength(1); + expect(rows[0].message).toContain( + "**rolled back** 🥹 @Jane Doe see #inc-db", + ); + expect(rows[0].message).not.toContain(":face_holding_back_tears:"); + }); + + test("a mention whose lookup fails keeps the raw id", async () => { + slackTestState.usersInfoImpl = (args) => + args.user === "U9" + ? Promise.reject(new Error("user_not_found")) + : Promise.resolve({ + ok: true, + user: { profile: { email: "ping@openstatus.dev" } }, + }); + slackTestState.historyImpl = () => + Promise.resolve({ + messages: [ + { + ts: "504.1", + text: "<@U9> ack", + blocks: [ + { + type: "rich_text", + elements: [ + { + type: "rich_text_section", + elements: [ + { type: "user", user_id: "U9" }, + { type: "text", text: " ack" }, + ], + }, + ], + }, + ], + }, + ], + }); + await pin("504.1"); + await waitForCall("reactions.add"); + const rows = await notes(); + expect(rows).toHaveLength(1); + expect(rows[0].message).toContain("@U9 ack"); + }); + + test("the note keeps the time the message was said", async () => { + const saidAt = Math.floor(Date.now() / 1000) - 3600; + const ts = `${saidAt}.000100`; + slackTestState.historyImpl = () => + Promise.resolve({ messages: [{ ts, text: "an hour ago" }] }); + await pin(ts); + await waitForCall("reactions.add"); + const rows = await notes(); + expect(rows).toHaveLength(1); + expect(Math.abs(rows[0].createdAt.getTime() - saidAt * 1000)).toBeLessThan( + 1000, + ); + }); + + test("a message with only attachments is noted from their fallback", async () => { + slackTestState.historyImpl = () => + Promise.resolve({ + messages: [ + { + ts: "505.1", + text: "", + attachments: [{ fallback: "[FIRING] db latency > 2s" }], + }, + ], + }); + await pin("505.1"); + await waitForCall("reactions.add"); + const rows = await notes(); + expect(rows).toHaveLength(1); + expect(rows[0].message).toContain("[FIRING] db latency > 2s"); + }); + + test("an attachment with an empty fallback is noted from its text", async () => { + slackTestState.historyImpl = () => + Promise.resolve({ + messages: [ + { + ts: "507.1", + text: "", + attachments: [{ fallback: "", text: "Deploy 1234 failed" }], + }, + ], + }); + await pin("507.1"); + await waitForCall("reactions.add"); + const rows = await notes(); + expect(rows).toHaveLength(1); + expect(rows[0].message).toContain("Deploy 1234 failed"); + }); + + test("a message with nothing to copy tells the pinner", async () => { + slackTestState.historyImpl = () => + Promise.resolve({ messages: [{ ts: "506.1", text: "" }] }); + await pin("506.1"); + const ephemeral = await waitForCall("postEphemeral"); + expect(ephemeral?.args.text).toContain("Nothing to copy"); + await new Promise((r) => setTimeout(r, 50)); + expect(slackTestState.calls.some((c) => c.method === "reactions.add")).toBe( + false, + ); + expect(await notes()).toHaveLength(0); + }); + test("a message already confirmed is not noted twice", async () => { slackTestState.historyImpl = () => Promise.resolve({ messages: [{ ts: "501.1", text: "Twice" }] }); diff --git a/apps/server/src/routes/slack/incident-events.ts b/apps/server/src/routes/slack/incident-events.ts index bbf61250..a7467407 100644 --- a/apps/server/src/routes/slack/incident-events.ts +++ b/apps/server/src/routes/slack/incident-events.ts @@ -3,6 +3,7 @@ import type { ServiceContext } from "@openstatus/services"; import { addIncidentNote, getIncidentBySlackChannel, + isAllowedNoteCreatedAt, unbindIncidentSlackChannel, } from "@openstatus/services/incident"; import { WebClient } from "@slack/web-api"; @@ -17,6 +18,12 @@ import { requireSlackMember, slackAgentAllowed, } from "./require-slack-member"; +import { resolveSlackMentionNames } from "./resolve-slack-user"; +import { + collectMentions, + mentionLabelsFromText, + richTextToMarkdown, +} from "./rich-text"; import type { SlackWorkspace } from "./workspace-resolver"; const logger = getLogger(["api-server", "slack", "incident-events"]); @@ -24,7 +31,67 @@ const logger = getLogger(["api-server", "slack", "incident-events"]); const PIN = "pushpin"; const DONE = "white_check_mark"; -type SlackMessage = { ts?: string; text?: string; user?: string }; +type SlackMessage = { + ts?: string; + text?: string; + user?: string; + blocks?: unknown[]; + attachments?: { text?: string; fallback?: string }[]; +}; + +const NOTHING_TO_COPY = "Nothing to copy from that message."; +const VERB_LAG_MS = 60_000; + +/** + * A Slack `ts` ("1759300000.123456") as the time the message was said, or + * `undefined` (now) when the verb would reject it as outside its window. + */ +function messageDate(ts: string): Date | undefined { + const date = new Date(Number(ts) * 1000); + const now = Date.now(); + // The verb samples its own `now` a moment later; a boundary message must + // pass both, or the pin would fail instead of falling back to now. + return isAllowedNoteCreatedAt(date, now) && + isAllowedNoteCreatedAt(date, now + VERB_LAG_MS) + ? date + : undefined; +} + +/** Alert bots post `attachments` with an empty `text`. */ +function attachmentsText(message: SlackMessage): string { + return (message.attachments ?? []) + .map((a) => a.fallback?.trim() || a.text?.trim() || "") + .filter(Boolean) + .join("\n\n"); +} + +async function noteBody(args: { + resolved: SlackWorkspace; + teamId: string; + slack: WebClient; + message: SlackMessage; +}): Promise { + const { resolved, teamId, slack, message } = args; + const labels = mentionLabelsFromText(message.text); + const lookedUp = await resolveSlackMentionNames({ + workspace: resolved.workspace, + teamId, + slack, + ...collectMentions(message.blocks), + }); + // A looked-up name beats the label Slack put in `text`. + const names = { + users: new Map([...(labels.users ?? []), ...(lookedUp.users ?? [])]), + channels: new Map([ + ...(labels.channels ?? []), + ...(lookedUp.channels ?? []), + ]), + usergroups: labels.usergroups, + }; + const body = + richTextToMarkdown(message.blocks, names) ?? message.text?.trim() ?? ""; + return body || attachmentsText(message); +} function system(resolved: SlackWorkspace): ServiceContext { return { @@ -131,8 +198,21 @@ export async function handlePinReaction(args: { try { if (await alreadyNoted(slack, channel, ts, resolved.botUserId)) return; const message = await findMessage(slack, channel, ts); - if (!message?.text) { + const body = message + ? await noteBody({ resolved, teamId, slack, message }) + : ""; + if (!body) { await redis.del(claim); + await slack.chat + .postEphemeral({ + channel, + user: slackUserId, + thread_ts: ts, + text: NOTHING_TO_COPY, + }) + .catch((error) => + logger.warn("slack failed to report an empty pin", { error }), + ); return; } const permalink = await slack.chat @@ -145,9 +225,8 @@ export async function handlePinReaction(args: { ctx, input: { id: bound.id, - message: permalink - ? `${message.text}\n\n[From Slack](${permalink})` - : message.text, + message: permalink ? `${body}\n\n[From Slack](${permalink})` : body, + createdAt: messageDate(ts), }, }); trackSlackIncident(ctx, "note", { via: "reaction" }); diff --git a/apps/server/src/routes/slack/resolve-slack-user.ts b/apps/server/src/routes/slack/resolve-slack-user.ts index c65d58b7..ea3ada79 100644 --- a/apps/server/src/routes/slack/resolve-slack-user.ts +++ b/apps/server/src/routes/slack/resolve-slack-user.ts @@ -1,55 +1,144 @@ import { getLogger } from "@logtape/logtape"; import type { Workspace } from "@openstatus/db/src/schema/workspaces/validation"; import type { ServiceContext } from "@openstatus/services"; -import { findMemberIdByEmail } from "@openstatus/services/member"; +import { + findMemberIdByEmail, + getMemberDisplayName, +} from "@openstatus/services/member"; import { createSlackUserMapping, getSlackUserMapping, } from "@openstatus/services/slack-user"; import type { WebClient } from "@slack/web-api"; +import type { MentionNames } from "./rich-text"; + const logger = getLogger(["api-server", "slack", "resolve-user"]); /** - * The openstatus member linked to a Slack user, or `null`. A missing link is - * created on the spot when the Slack profile email matches exactly one member. + * One `users.info` serves both the email match and the fallback name. Links + * the Slack user to the member on an email match, so merely being mentioned + * in a pinned message can create the mapping; it is the same rule applied + * when they act themselves. */ -export async function resolveSlackMember(args: { +async function lookupSlackMember(args: { workspace: Workspace; teamId: string; slackUserId: string; slack: WebClient; -}): Promise { +}): Promise<{ userId: number | null; profileName: string | null }> { const { workspace, teamId, slackUserId, slack } = args; - if (!teamId || !slackUserId) return null; + if (!teamId || !slackUserId) return { userId: null, profileName: null }; const ctx: ServiceContext = { workspace, actor: { type: "system", job: "slack-user-automap" }, }; + let profileName: string | null = null; try { const linked = await getSlackUserMapping({ ctx, input: { teamId, slackUserId }, }); - if (linked !== null) return linked; + if (linked !== null) return { userId: linked, profileName }; const info = await slack.users.info({ user: slackUserId }); + profileName = + info.user?.profile?.display_name || + info.user?.real_name || + info.user?.name || + null; const email = info.user?.profile?.email; - if (!email) return null; + if (!email) return { userId: null, profileName }; const userId = await findMemberIdByEmail({ ctx, input: { email } }); - if (userId === null) return null; + if (userId === null) return { userId: null, profileName }; await createSlackUserMapping({ ctx, input: { teamId, slackUserId, userId }, }); - return userId; + return { userId, profileName }; } catch (err) { logger.warn("slack user resolution failed", { workspaceId: workspace.id, teamId, error: err instanceof Error ? err.message : String(err), }); - return null; + return { userId: null, profileName }; } } + +/** + * The openstatus member linked to a Slack user, or `null`. A missing link is + * created on the spot when the Slack profile email matches exactly one member. + */ +export async function resolveSlackMember(args: { + workspace: Workspace; + teamId: string; + slackUserId: string; + slack: WebClient; +}): Promise { + return (await lookupSlackMember(args)).userId; +} + +// A pinned message rarely mentions more than a handful of people; Slack +// rate-limits `users.info`/`conversations.info`, so look them up a few at a time. +const MAX_MENTIONS = 20; +const LOOKUP_CONCURRENCY = 5; + +async function forEachBounded( + items: T[], + fn: (item: T) => Promise, +): Promise { + for (let i = 0; i < items.length; i += LOOKUP_CONCURRENCY) { + await Promise.all(items.slice(i, i + LOOKUP_CONCURRENCY).map(fn)); + } +} + +/** + * Names for the users and channels a message mentions: a linked member's + * openstatus name, else the Slack profile name, else nothing (the caller + * keeps the raw id). Lookups that fail are dropped, never thrown. + */ +export async function resolveSlackMentionNames(args: { + workspace: Workspace; + teamId: string; + slack: WebClient; + users: string[]; + channels: string[]; +}): Promise { + const { workspace, teamId, slack } = args; + const ctx: ServiceContext = { + workspace, + actor: { type: "system", job: "slack-user-automap" }, + }; + const users = new Map(); + const channels = new Map(); + + await Promise.all([ + forEachBounded(args.users.slice(0, MAX_MENTIONS), async (slackUserId) => { + const { userId, profileName } = await lookupSlackMember({ + workspace, + teamId, + slackUserId, + slack, + }); + const memberName = + userId === null + ? null + : await getMemberDisplayName({ ctx, input: { userId } }).catch( + () => null, + ); + const name = memberName ?? profileName; + if (name) users.set(slackUserId, name); + }), + forEachBounded(args.channels.slice(0, MAX_MENTIONS), async (channelId) => { + const res = await slack.conversations + .info({ channel: channelId }) + .catch(() => undefined); + const name = res?.channel?.name; + if (name) channels.set(channelId, name); + }), + ]); + + return { users, channels }; +} diff --git a/apps/server/src/routes/slack/rich-text.test.ts b/apps/server/src/routes/slack/rich-text.test.ts new file mode 100644 index 00000000..6aa5d612 --- /dev/null +++ b/apps/server/src/routes/slack/rich-text.test.ts @@ -0,0 +1,404 @@ +import { describe, expect, test } from "@openstatus/test-utils"; + +import { + collectMentions, + mentionLabelsFromText, + richTextToMarkdown, +} from "./rich-text"; + +function section(...elements: unknown[]) { + return { type: "rich_text_section", elements }; +} + +function message(...elements: unknown[]) { + return [{ type: "rich_text", elements }]; +} + +describe("richTextToMarkdown", () => { + test("returns undefined without a rich_text block", () => { + expect(richTextToMarkdown(undefined)).toBeUndefined(); + expect(richTextToMarkdown([])).toBeUndefined(); + expect( + richTextToMarkdown([{ type: "section", text: { type: "mrkdwn" } }]), + ).toBeUndefined(); + }); + + test("renders a standard emoji from its codepoints", () => { + expect( + richTextToMarkdown( + message( + section( + { type: "text", text: "oh wow, it syncs omg " }, + { + type: "emoji", + name: "face_holding_back_tears", + unicode: "1f979", + }, + ), + ), + ), + ).toBe("oh wow, it syncs omg 🥹"); + }); + + test("renders a skin-tone sequence", () => { + expect( + richTextToMarkdown( + message( + section({ + type: "emoji", + name: "+1", + skin_tone: 3, + unicode: "1f44d-1f3fc", + }), + ), + ), + ).toBe("👍🏼"); + }); + + test("keeps a custom emoji as its shortcode", () => { + expect( + richTextToMarkdown( + message(section({ type: "emoji", name: "partyparrot" })), + ), + ).toBe(":partyparrot:"); + }); + + test("converts styles to markdown", () => { + expect( + richTextToMarkdown( + message( + section( + { type: "text", text: "bold", style: { bold: true } }, + { type: "text", text: " " }, + { type: "text", text: "italic", style: { italic: true } }, + { type: "text", text: " " }, + { type: "text", text: "gone", style: { strike: true } }, + { type: "text", text: " " }, + { type: "text", text: "a*b", style: { code: true } }, + ), + ), + ), + ).toBe("**bold** _italic_ ~~gone~~ `a*b`"); + }); + + test("keeps surrounding whitespace outside style markers", () => { + expect( + richTextToMarkdown( + message( + section( + { type: "text", text: "rolled back ", style: { bold: true } }, + { type: "text", text: "to v41" }, + ), + ), + ), + ).toBe("**rolled back** to v41"); + }); + + test("converts links with and without a label", () => { + expect( + richTextToMarkdown( + message( + section( + { type: "link", url: "https://example.com/run/1", text: "the run" }, + { type: "text", text: " and " }, + { type: "link", url: "https://example.com" }, + ), + ), + ), + ).toBe("[the run](https://example.com/run/1) and "); + }); + + test("names mentions from the map and falls back to the id", () => { + expect( + richTextToMarkdown( + message( + section( + { type: "user", user_id: "U1" }, + { type: "text", text: " ping " }, + { type: "user", user_id: "U2" }, + { type: "text", text: " in " }, + { type: "channel", channel_id: "C1" }, + { type: "text", text: " " }, + { type: "channel", channel_id: "C2" }, + ), + ), + { + users: new Map([["U1", "Maximilian Kaske"]]), + channels: new Map([["C1", "inc-db"]]), + }, + ), + ).toBe("@Maximilian Kaske ping @U2 in #inc-db #C2"); + }); + + test("escapes markdown specials in names and plain text", () => { + expect( + richTextToMarkdown( + message( + section( + { type: "user", user_id: "U1" }, + { type: "text", text: " said 2*3 is_fine [sic]" }, + ), + ), + { users: new Map([["U1", "jo_hn"]]) }, + ), + ).toBe("@jo\\_hn said 2\\*3 is\\_fine \\[sic\\]"); + }); + + test("names a user group from the map and falls back to the id", () => { + const blocks = message( + section( + { type: "usergroup", usergroup_id: "S1" }, + { type: "text", text: " " }, + { type: "usergroup", usergroup_id: "S2" }, + ), + ); + expect( + richTextToMarkdown(blocks, { + usergroups: new Map([["S1", "engineering"]]), + }), + ).toBe("@engineering @S2"); + }); + + test("renders broadcasts", () => { + expect( + richTextToMarkdown( + message( + section( + { type: "broadcast", range: "here" }, + { type: "text", text: " db is back" }, + ), + ), + ), + ).toBe("@here db is back"); + }); + + test("renders lists, quotes and code blocks", () => { + expect( + richTextToMarkdown( + message( + section({ type: "text", text: "Findings:" }), + { + type: "rich_text_list", + style: "bullet", + elements: [ + section({ type: "text", text: "one" }), + section({ type: "text", text: "two" }), + ], + }, + { + type: "rich_text_list", + style: "ordered", + indent: 1, + elements: [section({ type: "text", text: "nested" })], + }, + { + type: "rich_text_quote", + elements: [{ type: "text", text: "quoted\nlines" }], + }, + { + type: "rich_text_preformatted", + elements: [{ type: "text", text: "SELECT *\nFROM t;" }], + }, + ), + ), + ).toBe( + [ + "Findings:", + "", + "- one\n- two\n 1. nested", + "", + "> quoted\n> lines", + "", + "```\nSELECT *\nFROM t;\n```", + ].join("\n"), + ); + }); + + test("escapes a line-start dash so it does not become a list", () => { + expect( + richTextToMarkdown( + message(section({ type: "text", text: "- not a list" })), + ), + ).toBe("\\- not a list"); + }); +}); + +describe("richTextToMarkdown edge cases", () => { + test("honors the ordered-list offset", () => { + expect( + richTextToMarkdown( + message({ + type: "rich_text_list", + style: "ordered", + offset: 2, + elements: [ + section({ type: "text", text: "third" }), + section({ type: "text", text: "fourth" }), + ], + }), + ), + ).toBe("3. third\n4. fourth"); + }); + + test("escapes tildes so literal text is not struck through", () => { + expect( + richTextToMarkdown( + message(section({ type: "text", text: "~~not gone~~" })), + ), + ).toBe("\\~\\~not gone\\~\\~"); + }); + + test("widens code delimiters past the backticks inside", () => { + expect( + richTextToMarkdown( + message( + section({ type: "text", text: "a `b` c", style: { code: true } }), + { + type: "rich_text_preformatted", + elements: [{ type: "text", text: "```\nx\n```" }], + }, + ), + ), + ).toBe("``a `b` c``\n\n````\n```\nx\n```\n````"); + }); + + test("pads inline code that starts or ends with a backtick", () => { + expect( + richTextToMarkdown( + message(section({ type: "text", text: "`x", style: { code: true } })), + ), + ).toBe("`` `x ``"); + expect( + richTextToMarkdown( + message(section({ type: "text", text: "x`", style: { code: true } })), + ), + ).toBe("`` x` ``"); + }); + + test("keeps a multi-line list item inside the item", () => { + expect( + richTextToMarkdown( + message( + { + type: "rich_text_list", + style: "ordered", + offset: 9, + elements: [section({ type: "text", text: "first\nstill first" })], + }, + { + type: "rich_text_list", + style: "bullet", + indent: 1, + elements: [section({ type: "text", text: "a\nb" })], + }, + ), + ), + ).toBe("10. first\n still first\n - a\n b"); + }); + + test("escapes block syntax at line start, also inside quotes", () => { + expect( + richTextToMarkdown( + message( + section({ type: "text", text: "## not a heading\n---\n+ no\n1) no" }), + { + type: "rich_text_quote", + elements: [{ type: "text", text: "# q" }], + }, + ), + ), + ).toBe("\\## not a heading\n\\---\n\\+ no\n\\1) no\n\n> \\# q"); + }); + + test("renders a link with an unsafe scheme as plain text", () => { + expect( + richTextToMarkdown( + message( + section( + { type: "link", url: "javascript:alert(1)", text: "click" }, + { type: "text", text: " " }, + { type: "link", url: "javascript:alert(1)" }, + { type: "text", text: " " }, + { type: "link", url: "mailto:ops@example.com" }, + ), + ), + ), + ).toBe("click javascript:alert(1) "); + }); + + test("encodes spaces and parentheses in link destinations", () => { + expect( + richTextToMarkdown( + message( + section( + { type: "link", url: "https://w.org/wiki/Foo_(bar)", text: "wiki" }, + { type: "text", text: " " }, + { type: "link", url: "https://e.com/a b" }, + ), + ), + ), + ).toBe("[wiki](https://w.org/wiki/Foo_%28bar%29) "); + }); +}); + +describe("mentionLabelsFromText", () => { + test("harvests the labels Slack spelled out in mrkdwn", () => { + const names = mentionLabelsFromText( + "<@U1|max> ping in <#C1|inc-db> and <#C2>", + ); + expect(names.users).toEqual(new Map([["U1", "max"]])); + expect(names.usergroups).toEqual(new Map([["S1", "engineering"]])); + expect(names.channels).toEqual(new Map([["C1", "inc-db"]])); + }); + + test("is empty without text", () => { + expect(mentionLabelsFromText(undefined).users?.size).toBe(0); + }); +}); + +describe("collectMentions", () => { + test("dedupes user and channel ids across blocks", () => { + expect( + collectMentions( + message( + section( + { type: "user", user_id: "U1" }, + { type: "channel", channel_id: "C1" }, + ), + { + type: "rich_text_quote", + elements: [ + { type: "user", user_id: "U1" }, + { type: "user", user_id: "U2" }, + ], + }, + ), + ), + ).toEqual({ users: ["U1", "U2"], channels: ["C1"] }); + }); + + test("is empty without rich_text", () => { + expect(collectMentions(undefined)).toEqual({ users: [], channels: [] }); + }); + + test("finds mentions inside list items", () => { + const blocks = message({ + type: "rich_text_list", + style: "bullet", + elements: [ + section( + { type: "user", user_id: "U5" }, + { type: "text", text: " owns it" }, + ), + section({ type: "channel", channel_id: "C5" }), + ], + }); + expect(collectMentions(blocks)).toEqual({ + users: ["U5"], + channels: ["C5"], + }); + expect( + richTextToMarkdown(blocks, { users: new Map([["U5", "Sam"]]) }), + ).toBe("- @Sam owns it\n- #C5"); + }); +}); diff --git a/apps/server/src/routes/slack/rich-text.ts b/apps/server/src/routes/slack/rich-text.ts new file mode 100644 index 00000000..1830125d --- /dev/null +++ b/apps/server/src/routes/slack/rich-text.ts @@ -0,0 +1,316 @@ +/** + * Slack attaches `rich_text` blocks to every user-typed message. They carry + * what `message.text` (mrkdwn) loses: emoji codepoints, structured mentions + * and links, styles. Convert them to the markdown the timeline renders. + * + * Structural types instead of `@slack/types`: the package is only a + * transitive dep and the fields used here are stable. + */ + +type Style = { + bold?: boolean; + italic?: boolean; + strike?: boolean; + code?: boolean; +}; + +type InlineElement = + | { type: "text"; text: string; style?: Style } + | { type: "emoji"; name: string; unicode?: string; style?: Style } + | { type: "link"; url: string; text?: string; style?: Style } + | { type: "user"; user_id: string; style?: Style } + | { type: "channel"; channel_id: string; style?: Style } + | { type: "usergroup"; usergroup_id: string; style?: Style } + | { type: "broadcast"; range: string; style?: Style } + | { type: "date"; fallback?: string; style?: Style } + | { type: "color"; value: string; style?: Style }; + +type Section = { type: "rich_text_section"; elements: InlineElement[] }; +type List = { + type: "rich_text_list"; + style: "bullet" | "ordered"; + indent?: number; + offset?: number; + elements: Section[]; +}; +type Quote = { type: "rich_text_quote"; elements: InlineElement[] }; +type Preformatted = { + type: "rich_text_preformatted"; + elements: InlineElement[]; +}; +type RichTextElement = Section | List | Quote | Preformatted; +type RichTextBlock = { type: "rich_text"; elements: RichTextElement[] }; + +export type MentionNames = { + users?: Map; + channels?: Map; + usergroups?: Map; +}; + +// `<@U1|max>`, `<#C1|inc-db>`, `` in mrkdwn `text`. +const LABELLED_MENTION = /<([@#]|!subteam\^)([A-Z0-9]+)\|@?([^>|]+)>/g; + +/** + * Names Slack already spelled out in `message.text`. The only source for a + * user group's handle, since `rich_text` carries just its id and reading + * groups would need a scope we do not request. + */ +export function mentionLabelsFromText(text: string | undefined): MentionNames { + const names: Required = { + users: new Map(), + channels: new Map(), + usergroups: new Map(), + }; + for (const match of (text ?? "").matchAll(LABELLED_MENTION)) { + const [, kind, id, label] = match; + const target = + kind === "@" + ? names.users + : kind === "#" + ? names.channels + : names.usergroups; + target.set(id, label.trim()); + } + return names; +} + +function isRichTextBlock(block: unknown): block is RichTextBlock { + return ( + typeof block === "object" && + block !== null && + (block as { type?: unknown }).type === "rich_text" && + Array.isArray((block as { elements?: unknown }).elements) + ); +} + +function richTextBlocks(blocks: unknown[] | undefined): RichTextBlock[] { + return (blocks ?? []).filter(isRichTextBlock); +} + +/** A list's children are sections; everything else holds inlines directly. */ +function inlinesOf(element: RichTextElement): InlineElement[] { + if (element.type === "rich_text_list") { + return element.elements.flatMap((item) => item.elements ?? []); + } + return element.elements ?? []; +} + +/** Every user and channel id mentioned, deduplicated, for the caller to resolve. */ +export function collectMentions(blocks: unknown[] | undefined): { + users: string[]; + channels: string[]; +} { + const users = new Set(); + const channels = new Set(); + for (const block of richTextBlocks(blocks)) { + for (const element of block.elements) { + for (const inline of inlinesOf(element)) { + if (inline.type === "user") users.add(inline.user_id); + if (inline.type === "channel") channels.add(inline.channel_id); + } + } + } + return { users: [...users], channels: [...channels] }; +} + +// Only what remark would otherwise interpret; `#`, `>`, `-` matter at line start. +const INLINE_SPECIALS = /[\\`*_~[\]]/g; +const LINE_START_SPECIALS = + /^(\s*)(#{1,6}|>|[-+]|-{3,}|={3,}|\d+[.)])(?=\s|$)/gm; + +function escapeText(text: string): string { + return text + .replace(INLINE_SPECIALS, "\\$&") + .replace(LINE_START_SPECIALS, "$1\\$2"); +} + +function longestBacktickRun(text: string): number { + let max = 0; + for (const match of text.matchAll(/`+/g)) { + max = Math.max(max, match[0].length); + } + return max; +} + +/** A delimiter longer than any backtick run inside, so the span cannot close early. */ +function inlineCode(text: string): string { + const fence = "`".repeat(longestBacktickRun(text) + 1); + const pad = text.startsWith("`") || text.endsWith("`") ? " " : ""; + return `${fence}${pad}${text}${pad}${fence}`; +} + +function codeFence(code: string): string { + const fence = "`".repeat(Math.max(3, longestBacktickRun(code) + 1)); + return `${fence}\n${code}\n${fence}`; +} + +// Not every renderer sanitizes `javascript:`; anything else becomes plain text. +const SAFE_LINK = /^(https?:|mailto:)/i; + +/** Spaces and parentheses end a markdown destination early; `<>` break autolinks. */ +function linkDestination(url: string): string { + return url.replace( + /[ ()<>]/g, + (c) => `%${c.charCodeAt(0).toString(16).toUpperCase()}`, + ); +} + +/** `"1f44d-1f3fc"` → 👍🏼. Hyphen-separated hex codepoints. */ +function emojiFromUnicode(unicode: string): string | undefined { + const points = unicode.split("-").map((hex) => Number.parseInt(hex, 16)); + if (points.length === 0 || points.some((p) => !Number.isFinite(p))) { + return undefined; + } + try { + return String.fromCodePoint(...points); + } catch { + return undefined; + } +} + +/** Markers must hug the text, or remark reads `**bold **` literally. */ +function applyStyle(text: string, style: Style | undefined): string { + if (!style || !text.trim()) return text; + const leading = text.match(/^\s*/)?.[0] ?? ""; + const trailing = text.match(/\s*$/)?.[0] ?? ""; + let inner = text.slice(leading.length, text.length - trailing.length); + if (style.code) { + inner = inlineCode(inner); + } else { + if (style.strike) inner = `~~${inner}~~`; + if (style.italic) inner = `_${inner}_`; + if (style.bold) inner = `**${inner}**`; + } + return `${leading}${inner}${trailing}`; +} + +function renderInline(element: InlineElement, names: MentionNames): string { + switch (element.type) { + case "text": + return applyStyle( + element.style?.code ? element.text : escapeText(element.text), + element.style, + ); + case "emoji": { + const char = element.unicode + ? emojiFromUnicode(element.unicode) + : undefined; + return applyStyle(char ?? `:${element.name}:`, element.style); + } + case "link": { + const label = element.text?.trim(); + if (!SAFE_LINK.test(element.url)) { + return applyStyle(escapeText(label || element.url), element.style); + } + const url = linkDestination(element.url); + const md = + label && label !== element.url + ? `[${escapeText(label)}](${url})` + : `<${url}>`; + return applyStyle(md, element.style); + } + case "user": { + const name = names.users?.get(element.user_id); + return applyStyle( + `@${escapeText(name ?? element.user_id)}`, + element.style, + ); + } + case "channel": { + const name = names.channels?.get(element.channel_id); + return applyStyle( + `#${escapeText(name ?? element.channel_id)}`, + element.style, + ); + } + case "usergroup": { + const name = names.usergroups?.get(element.usergroup_id); + return applyStyle( + `@${escapeText(name ?? element.usergroup_id)}`, + element.style, + ); + } + case "broadcast": + return applyStyle(`@${element.range}`, element.style); + case "date": + return applyStyle(escapeText(element.fallback ?? ""), element.style); + case "color": + return applyStyle(escapeText(element.value), element.style); + default: + return ""; + } +} + +function renderInlines(elements: InlineElement[], names: MentionNames): string { + return elements.map((element) => renderInline(element, names)).join(""); +} + +// Wide enough to nest under `1. ` (3 columns) as well as `- `. +const LIST_INDENT = " "; + +function renderElement(element: RichTextElement, names: MentionNames): string { + switch (element.type) { + case "rich_text_section": + return renderInlines(element.elements, names); + case "rich_text_list": { + const pad = LIST_INDENT.repeat(element.indent ?? 0); + const start = (element.offset ?? 0) + 1; + return element.elements + .map((item, index) => { + const marker = + element.style === "ordered" ? `${start + index}.` : "-"; + // Continuation lines stay inside the item when indented to its text. + const hang = `\n${pad}${" ".repeat(marker.length + 1)}`; + const text = renderInlines(item.elements, names).replace(/\n/g, hang); + return `${pad}${marker} ${text}`; + }) + .join("\n"); + } + case "rich_text_quote": + return renderInlines(element.elements, names) + .split("\n") + .map((line) => `> ${line}`) + .join("\n"); + case "rich_text_preformatted": + return codeFence( + element.elements + .map((inline) => + inline.type === "text" + ? inline.text + : inline.type === "link" + ? (inline.text ?? inline.url) + : "", + ) + .join(""), + ); + default: + return ""; + } +} + +/** + * Markdown for a message's `rich_text` blocks, or `undefined` when there are + * none so the caller falls back to `message.text`. + */ +export function richTextToMarkdown( + blocks: unknown[] | undefined, + names: MentionNames = {}, +): string | undefined { + const rich = richTextBlocks(blocks); + if (rich.length === 0) return undefined; + let out = ""; + let previous: RichTextElement["type"] | undefined; + for (const block of rich) { + for (const element of block.elements) { + const rendered = renderElement(element, names); + if (!rendered.trim()) continue; + // Slack emits one list per nesting level; a blank line would split them. + const adjacentLists = + previous === "rich_text_list" && element.type === "rich_text_list"; + if (out) out += adjacentLists ? "\n" : "\n\n"; + out += rendered; + previous = element.type; + } + } + return out.trim() || undefined; +} diff --git a/packages/services/src/incident/__tests__/incident.test.ts b/packages/services/src/incident/__tests__/incident.test.ts index 640c411b..7dc78c15 100644 --- a/packages/services/src/incident/__tests__/incident.test.ts +++ b/packages/services/src/incident/__tests__/incident.test.ts @@ -459,6 +459,43 @@ describe("addIncidentNote", () => { }); }); + test("keeps a createdAt inside the window and treats null as now", async () => { + await withTestTransaction(async (tx) => { + const row = await declare(tx); + const saidAt = new Date(Date.now() - 60 * 60 * 1000); + const backdated = await addIncidentNote({ + ctx: as(memberId, tx), + input: { id: row.id, message: "an hour ago", createdAt: saidAt }, + }); + // `created_at` is stored in whole seconds. + expect( + Math.abs(backdated.createdAt.getTime() - saidAt.getTime()), + ).toBeLessThan(1000); + const now = await addIncidentNote({ + ctx: as(memberId, tx), + input: { id: row.id, message: "now", createdAt: null }, + }); + expect(Math.abs(now.createdAt.getTime() - Date.now())).toBeLessThan(5000); + }); + }); + + test("rejects a createdAt in the future or older than the window", async () => { + await withTestTransaction(async (tx) => { + const row = await declare(tx); + for (const createdAt of [ + new Date(Date.now() + 10 * 60 * 1000), + new Date(Date.now() - 31 * 24 * 60 * 60 * 1000), + ]) { + await expect( + addIncidentNote({ + ctx: as(memberId, tx), + input: { id: row.id, message: "off the timeline", createdAt }, + }), + ).rejects.toThrow(/createdAt must be within the last 30 days/); + } + }); + }); + test("a closed incident takes no notes", async () => { await withTestTransaction(async (tx) => { const row = await createIncident( diff --git a/packages/services/src/incident/add-note.ts b/packages/services/src/incident/add-note.ts index d518408c..106f0842 100644 --- a/packages/services/src/incident/add-note.ts +++ b/packages/services/src/incident/add-note.ts @@ -32,6 +32,7 @@ export async function addIncidentNote(args: { incidentId: existing.id, type: "note", message: input.message, + createdAt: input.createdAt ?? undefined, }); }); } diff --git a/packages/services/src/incident/index.ts b/packages/services/src/incident/index.ts index 7130a983..b4f6ddfb 100644 --- a/packages/services/src/incident/index.ts +++ b/packages/services/src/incident/index.ts @@ -37,6 +37,9 @@ export { } from "./postmortem-draft"; export { AddIncidentNoteInput, + isAllowedNoteCreatedAt, + NOTE_BACKDATE_MAX_MS, + NOTE_FUTURE_SKEW_MS, ApprovePostmortemInput, CloseIncidentInput, DraftPostmortemInput, diff --git a/packages/services/src/incident/schemas.ts b/packages/services/src/incident/schemas.ts index 125857b3..4c659935 100644 --- a/packages/services/src/incident/schemas.ts +++ b/packages/services/src/incident/schemas.ts @@ -42,7 +42,31 @@ export const SetIncidentStatusInput = z.object({ }); export type SetIncidentStatusInput = z.infer; -export const AddIncidentNoteInput = z.object({ id, message: note }); +// A note copied from elsewhere (Slack) may keep the time it was said, within +// a window that keeps the timeline honest for every caller of the verb. +export const NOTE_BACKDATE_MAX_MS = 30 * 24 * 60 * 60 * 1000; +export const NOTE_FUTURE_SKEW_MS = 60_000; + +export function isAllowedNoteCreatedAt(date: Date, now = Date.now()): boolean { + const ms = date.getTime(); + return ( + Number.isFinite(ms) && + ms <= now + NOTE_FUTURE_SKEW_MS && + ms >= now - NOTE_BACKDATE_MAX_MS + ); +} + +export const AddIncidentNoteInput = z.object({ + id, + message: note, + createdAt: z.coerce + .date() + .refine((d) => isAllowedNoteCreatedAt(d), { + message: + "createdAt must be within the last 30 days and at most a minute ahead", + }) + .nullish(), +}); export type AddIncidentNoteInput = z.infer; export const LinkIncidentStatusReportInput = z.object({ diff --git a/packages/services/src/member/__tests__/display-name.test.ts b/packages/services/src/member/__tests__/display-name.test.ts new file mode 100644 index 00000000..8cc7adc8 --- /dev/null +++ b/packages/services/src/member/__tests__/display-name.test.ts @@ -0,0 +1,44 @@ +import { + addUserToWorkspace, + createUser, +} from "@openstatus/db/src/test/factories"; +import { expect } from "@std/expect"; +import { beforeAll, describe, test } from "@std/testing/bdd"; + +import { createWorkspaceFixture, makeSystemCtx } from "../../../test/helpers"; +import type { ServiceContext } from "../../context"; +import { getMemberDisplayName } from "../index.ts"; + +let ctx: ServiceContext; +let workspaceId: number; + +beforeAll(async () => { + const fixture = await createWorkspaceFixture("team"); + workspaceId = fixture.workspace.id; + ctx = makeSystemCtx(fixture.workspace, { job: "slack-user-automap" }); +}); + +describe("getMemberDisplayName", () => { + test("prefers the explicit name over first and last", async () => { + const member = await createUser({ name: "Max K." }); + await addUserToWorkspace(member.id, workspaceId, "member"); + expect( + await getMemberDisplayName({ ctx, input: { userId: member.id } }), + ).toBe("Max K."); + }); + + test("falls back to first and last name", async () => { + const member = await createUser({ name: null }); + await addUserToWorkspace(member.id, workspaceId, "member"); + expect( + await getMemberDisplayName({ ctx, input: { userId: member.id } }), + ).toBe("Test User"); + }); + + test("returns null for a user who is not a member here", async () => { + const outsider = await createUser(); + expect( + await getMemberDisplayName({ ctx, input: { userId: outsider.id } }), + ).toBeNull(); + }); +}); diff --git a/packages/services/src/member/display-name.ts b/packages/services/src/member/display-name.ts new file mode 100644 index 00000000..6a35fdaf --- /dev/null +++ b/packages/services/src/member/display-name.ts @@ -0,0 +1,32 @@ +import { and, eq, isNull } from "@openstatus/db"; +import { user, usersToWorkspaces } from "@openstatus/db/src/schema"; + +import { displayName } from "../attribution"; +import { type ServiceContext, getReadDb } from "../context"; +import { GetMemberDisplayNameInput } from "./schemas"; + +/** The display name of a live member of this workspace, or `null`. */ +export async function getMemberDisplayName(args: { + ctx: ServiceContext; + input: GetMemberDisplayNameInput; +}): Promise { + const input = GetMemberDisplayNameInput.parse(args.input); + const row = await getReadDb(args.ctx) + .select({ + name: user.name, + firstName: user.firstName, + lastName: user.lastName, + email: user.email, + }) + .from(usersToWorkspaces) + .innerJoin(user, eq(user.id, usersToWorkspaces.userId)) + .where( + and( + eq(usersToWorkspaces.workspaceId, args.ctx.workspace.id), + eq(usersToWorkspaces.userId, input.userId), + isNull(user.deletedAt), + ), + ) + .get(); + return row ? displayName(row) : null; +} diff --git a/packages/services/src/member/index.ts b/packages/services/src/member/index.ts index 10cc2823..6582ab10 100644 --- a/packages/services/src/member/index.ts +++ b/packages/services/src/member/index.ts @@ -1,8 +1,10 @@ export { deleteMember } from "./delete"; +export { getMemberDisplayName } from "./display-name"; export { findMemberIdByEmail } from "./find-by-email"; export { listMembers, type Member } from "./list"; export { DeleteMemberInput, FindMemberByEmailInput, + GetMemberDisplayNameInput, ListMembersInput, } from "./schemas"; diff --git a/packages/services/src/member/schemas.ts b/packages/services/src/member/schemas.ts index 0e24564a..db5605c4 100644 --- a/packages/services/src/member/schemas.ts +++ b/packages/services/src/member/schemas.ts @@ -8,3 +8,8 @@ export type DeleteMemberInput = z.infer; export const FindMemberByEmailInput = z.object({ email: z.string() }); export type FindMemberByEmailInput = z.infer; + +export const GetMemberDisplayNameInput = z.object({ userId: z.number().int() }); +export type GetMemberDisplayNameInput = z.infer< + typeof GetMemberDisplayNameInput +>;