From 5b2c64e88206f5c1581d17550dc354748dc50f89 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Tue, 8 Sep 2026 00:49:40 +0000 Subject: [PATCH 1/3] fix(slack): scope invalid_blocks fix to pre-formatted mrkdwn replies Auth-pause notices (buildAuthPauseResponse) already emit Slack mrkdwn (<@user> mentions, links) rather than CommonMark. Sending that text through the shared markdown block (designed for CommonMark model output) makes Slack's markdown-to-rich_text converter mis-tag an inline element with a url property, which Slack rejects as invalid_blocks. Give buildSlackReplyBlocks/sendSlackReply an explicit bodyFormat ("commonmark" default, "mrkdwn" opt-in) instead of globally swapping the markdown block for a mrkdwn section block. postAuthPauseNotice now requests the mrkdwn format for its pre-formatted text, while normal agent replies keep the markdown block so CommonMark (bold, links, tables, headers) still renders as intended. Fixes JUNIOR-72. Closes #1797, which fixed the reported error but regressed CommonMark rendering for all other replies (5 failing contract/component tests) by switching every reply body to mrkdwn. Co-Authored-By: David Cramer --- .../junior/src/chat/providers/slack/turn.ts | 6 +++++ packages/junior/src/chat/slack/footer.ts | 23 +++++++++++++++---- packages/junior/src/chat/slack/reply.ts | 2 ++ .../junior/tests/unit/slack/footer.test.ts | 16 +++++++++++++ 4 files changed, 42 insertions(+), 5 deletions(-) diff --git a/packages/junior/src/chat/providers/slack/turn.ts b/packages/junior/src/chat/providers/slack/turn.ts index ea961dcccc..e4db148f66 100644 --- a/packages/junior/src/chat/providers/slack/turn.ts +++ b/packages/junior/src/chat/providers/slack/turn.ts @@ -444,7 +444,13 @@ export function createSlackTurn(deps: SlackTurnDeps) { try { await beforeFirstResponsePost(); if (channelId && threadTs) { + // `buildAuthPauseResponse` already produces Slack mrkdwn (e.g. + // `<@user>` mentions), not the CommonMark this pipeline + // otherwise expects for the `markdown` block. Deliver it as a + // `mrkdwn` section instead so Slack's markdown-to-rich_text + // conversion doesn't reject the payload (`invalid_blocks`). await sendSlackReply({ + bodyFormat: "mrkdwn", channelId, conversationId, replyAttribution: options.execution?.dispatch?.replyAttribution, diff --git a/packages/junior/src/chat/slack/footer.ts b/packages/junior/src/chat/slack/footer.ts index 53709d2d3c..de83ed7d0e 100644 --- a/packages/junior/src/chat/slack/footer.ts +++ b/packages/junior/src/chat/slack/footer.ts @@ -90,20 +90,33 @@ export function buildSlackReplyFooter(args: { : undefined; } -/** Build Slack blocks for a reply chunk using the Slack-flavored markdown block for the body. */ +/** + * Build Slack blocks for a reply chunk. + * + * `bodyFormat` selects how the main body block is rendered: + * - `"commonmark"` (default) uses the Slack-flavored `markdown` block, which + * Slack renders natively from standard Markdown. Use this for model-authored + * text normalized by `normalizeSlackReplyMarkdown`. + * - `"mrkdwn"` uses a `section` block with a `mrkdwn` text object. Use this + * only for text that is already Slack mrkdwn (Slack mention syntax like + * `<@U123>`, single-`*` emphasis, `` links) such as auth-pause + * notices built by `buildAuthPauseResponse`. Slack's markdown-to-rich_text + * converter mishandles raw mrkdwn tokens inside a `markdown` block and + * rejects the message with `invalid_blocks`. + */ export function buildSlackReplyBlocks( text: string, footer: SlackReplyFooter | undefined, + bodyFormat: "commonmark" | "mrkdwn" = "commonmark", ): SlackMessageBlock[] | undefined { if (!text.trim()) { return undefined; } const blocks: SlackMessageBlock[] = [ - { - type: "markdown", - text, - }, + bodyFormat === "mrkdwn" + ? { type: "section", text: { type: "mrkdwn", text } } + : { type: "markdown", text }, ]; if (footer && (footer.attribution || footer.items.length > 0)) { diff --git a/packages/junior/src/chat/slack/reply.ts b/packages/junior/src/chat/slack/reply.ts index eaad1b1767..4a8feb1fc8 100644 --- a/packages/junior/src/chat/slack/reply.ts +++ b/packages/junior/src/chat/slack/reply.ts @@ -20,6 +20,7 @@ import { splitSlackReplyText } from "@/chat/slack/output"; * context, and posts through the shared Slack outbound boundary. */ export async function sendSlackReply(args: { + bodyFormat?: "commonmark" | "mrkdwn"; channelId: string; conversationId: string; replyAttribution?: ReplyAttribution; @@ -41,6 +42,7 @@ export async function sendSlackReply(args: { const blocks = buildSlackReplyBlocks( text, isFinalChunk ? footer : undefined, + args.bodyFormat, ); const fallbackText = isFinalChunk && args.replyAttribution diff --git a/packages/junior/tests/unit/slack/footer.test.ts b/packages/junior/tests/unit/slack/footer.test.ts index 6657dd4137..108b2a2fb2 100644 --- a/packages/junior/tests/unit/slack/footer.test.ts +++ b/packages/junior/tests/unit/slack/footer.test.ts @@ -160,6 +160,22 @@ describe("buildSlackReplyBlocks", () => { expect(buildSlackReplyBlocks(" ", footer)).toBeUndefined(); }); + + it("renders a mrkdwn section block for pre-formatted Slack mrkdwn text", () => { + // Regression test: auth-pause notices (`buildAuthPauseResponse`) emit + // Slack mrkdwn directly (mentions, `` links) rather than CommonMark. + // Slack's markdown-to-rich_text converter rejects that content inside a + // `markdown` block with `invalid_blocks`, so callers with pre-formatted + // mrkdwn must request the `mrkdwn` body format instead. + const text = + "<@U123> I need access to GitHub to continue.\n\n*Why:* check out and proceed\n\nI sent you a link."; + expect(buildSlackReplyBlocks(text, undefined, "mrkdwn")).toEqual([ + { + type: "section", + text: { type: "mrkdwn", text }, + }, + ]); + }); }); describe("getDashboardTaskLink", () => { From e723f4a1f873b109ac48caed4d8b2e76cdab648e Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Tue, 8 Sep 2026 15:09:41 +0000 Subject: [PATCH 2/3] fix(slack): use CommonMark bold in auth-pause notices Per review feedback on #1798: drop the bodyFormat/mrkdwn-section branching and keep every reply, including auth-pause notices, on the single shared markdown block. buildAuthPauseResponse used mrkdwn single-asterisk bold (*Why:*) inside text delivered through the markdown block, which Slack renders as CommonMark. Single-* is CommonMark italics, not bold, so this was a malformed/mismatched marker riding along with the JUNIOR-72 invalid_blocks failure. Switch it to CommonMark double-asterisk bold (**Why:**), matching every other reply body and the normalizer's own tested contract (normalizeSlackReplyMarkdown leaves `**bold**` untouched). The user mention prefix (<@id>) and the auto-linked URL inside the request text are unchanged from ordinary agent replies, which already use the same tokens successfully through this block. Verified: pnpm typecheck clean; full tests/unit, tests/component, tests/integration suites pass (only the 2 pre-existing, unrelated nitro-plugin-module package-resolution failures remain, same as before this change). Co-Authored-By: David Cramer --- .../junior/src/chat/providers/slack/turn.ts | 6 ---- .../src/chat/services/auth-pause-response.ts | 7 ++++- packages/junior/src/chat/slack/footer.ts | 23 +++----------- packages/junior/src/chat/slack/reply.ts | 2 -- .../auth/mcp-auth-runtime-slack.test.ts | 8 ++--- .../unit/services/auth-pause-response.test.ts | 2 +- .../junior/tests/unit/slack/footer.test.ts | 31 ++++++++++++------- 7 files changed, 36 insertions(+), 43 deletions(-) diff --git a/packages/junior/src/chat/providers/slack/turn.ts b/packages/junior/src/chat/providers/slack/turn.ts index e4db148f66..ea961dcccc 100644 --- a/packages/junior/src/chat/providers/slack/turn.ts +++ b/packages/junior/src/chat/providers/slack/turn.ts @@ -444,13 +444,7 @@ export function createSlackTurn(deps: SlackTurnDeps) { try { await beforeFirstResponsePost(); if (channelId && threadTs) { - // `buildAuthPauseResponse` already produces Slack mrkdwn (e.g. - // `<@user>` mentions), not the CommonMark this pipeline - // otherwise expects for the `markdown` block. Deliver it as a - // `mrkdwn` section instead so Slack's markdown-to-rich_text - // conversion doesn't reject the payload (`invalid_blocks`). await sendSlackReply({ - bodyFormat: "mrkdwn", channelId, conversationId, replyAttribution: options.execution?.dispatch?.replyAttribution, diff --git a/packages/junior/src/chat/services/auth-pause-response.ts b/packages/junior/src/chat/services/auth-pause-response.ts index 5243177ed1..c3468fcdec 100644 --- a/packages/junior/src/chat/services/auth-pause-response.ts +++ b/packages/junior/src/chat/services/auth-pause-response.ts @@ -26,5 +26,10 @@ export function buildAuthPauseResponse( if (!request) { return `${mention}I'll need you to authorize ${providerDisplayName}. I sent you a link.`; } - return `${mention}I need access to ${providerDisplayName} to continue.\n\n*Why:* ${request}\n\nI sent you a link.`; + // CommonMark bold (`**...**`), not mrkdwn bold (`*...*`) — this text is + // delivered through the same `markdown` block as every other Slack reply, + // which Slack renders from standard Markdown. A single-`*` emphasis marker + // is CommonMark italics, not bold, so `*Why:*` rendered wrong even before + // it contributed to the invalid_blocks failure in JUNIOR-72. + return `${mention}I need access to ${providerDisplayName} to continue.\n\n**Why:** ${request}\n\nI sent you a link.`; } diff --git a/packages/junior/src/chat/slack/footer.ts b/packages/junior/src/chat/slack/footer.ts index de83ed7d0e..53709d2d3c 100644 --- a/packages/junior/src/chat/slack/footer.ts +++ b/packages/junior/src/chat/slack/footer.ts @@ -90,33 +90,20 @@ export function buildSlackReplyFooter(args: { : undefined; } -/** - * Build Slack blocks for a reply chunk. - * - * `bodyFormat` selects how the main body block is rendered: - * - `"commonmark"` (default) uses the Slack-flavored `markdown` block, which - * Slack renders natively from standard Markdown. Use this for model-authored - * text normalized by `normalizeSlackReplyMarkdown`. - * - `"mrkdwn"` uses a `section` block with a `mrkdwn` text object. Use this - * only for text that is already Slack mrkdwn (Slack mention syntax like - * `<@U123>`, single-`*` emphasis, `` links) such as auth-pause - * notices built by `buildAuthPauseResponse`. Slack's markdown-to-rich_text - * converter mishandles raw mrkdwn tokens inside a `markdown` block and - * rejects the message with `invalid_blocks`. - */ +/** Build Slack blocks for a reply chunk using the Slack-flavored markdown block for the body. */ export function buildSlackReplyBlocks( text: string, footer: SlackReplyFooter | undefined, - bodyFormat: "commonmark" | "mrkdwn" = "commonmark", ): SlackMessageBlock[] | undefined { if (!text.trim()) { return undefined; } const blocks: SlackMessageBlock[] = [ - bodyFormat === "mrkdwn" - ? { type: "section", text: { type: "mrkdwn", text } } - : { type: "markdown", text }, + { + type: "markdown", + text, + }, ]; if (footer && (footer.attribution || footer.items.length > 0)) { diff --git a/packages/junior/src/chat/slack/reply.ts b/packages/junior/src/chat/slack/reply.ts index 4a8feb1fc8..eaad1b1767 100644 --- a/packages/junior/src/chat/slack/reply.ts +++ b/packages/junior/src/chat/slack/reply.ts @@ -20,7 +20,6 @@ import { splitSlackReplyText } from "@/chat/slack/output"; * context, and posts through the shared Slack outbound boundary. */ export async function sendSlackReply(args: { - bodyFormat?: "commonmark" | "mrkdwn"; channelId: string; conversationId: string; replyAttribution?: ReplyAttribution; @@ -42,7 +41,6 @@ export async function sendSlackReply(args: { const blocks = buildSlackReplyBlocks( text, isFinalChunk ? footer : undefined, - args.bodyFormat, ); const fallbackText = isFinalChunk && args.replyAttribution diff --git a/packages/junior/tests/component/auth/mcp-auth-runtime-slack.test.ts b/packages/junior/tests/component/auth/mcp-auth-runtime-slack.test.ts index 6eb3425995..544346f185 100644 --- a/packages/junior/tests/component/auth/mcp-auth-runtime-slack.test.ts +++ b/packages/junior/tests/component/auth/mcp-auth-runtime-slack.test.ts @@ -506,7 +506,7 @@ describe("mcp auth runtime slack integration", () => { params: expect.objectContaining({ channel: "C123", thread_ts: "1700000000.001", - text: "<@U123> I need access to Eval Auth to continue.\n\n*Why:* what did i say about the budget?\n\nI sent you a link.", + text: "<@U123> I need access to Eval Auth to continue.\n\n**Why:** what did i say about the budget?\n\nI sent you a link.", }), }), ]); @@ -666,7 +666,7 @@ describe("mcp auth runtime slack integration", () => { params: expect.objectContaining({ channel: "C123", thread_ts: "1700000000.001", - text: "<@U123> I need access to Eval Auth to continue.\n\n*Why:* what did i say about the budget?\n\nI sent you a link.", + text: "<@U123> I need access to Eval Auth to continue.\n\n**Why:** what did i say about the budget?\n\nI sent you a link.", }), }), expect.objectContaining({ @@ -764,7 +764,7 @@ describe("mcp auth runtime slack integration", () => { params: expect.objectContaining({ channel: "C124", thread_ts: "1700000000.002", - text: "<@U123> I need access to Eval Auth to continue.\n\n*Why:* what did i say about the budget?\n\nI sent you a link.", + text: "<@U123> I need access to Eval Auth to continue.\n\n**Why:** what did i say about the budget?\n\nI sent you a link.", }), }), ]); @@ -1017,7 +1017,7 @@ describe("mcp auth runtime slack integration", () => { params: expect.objectContaining({ channel: "C125", thread_ts: "1700000000.003", - text: "<@U123> I need access to Eval Auth to continue.\n\n*Why:* use eval-auth directly for the budget answer\n\nI sent you a link.", + text: "<@U123> I need access to Eval Auth to continue.\n\n**Why:** use eval-auth directly for the budget answer\n\nI sent you a link.", }), }), expect.objectContaining({ diff --git a/packages/junior/tests/unit/services/auth-pause-response.test.ts b/packages/junior/tests/unit/services/auth-pause-response.test.ts index 4c4694c98f..3309082678 100644 --- a/packages/junior/tests/unit/services/auth-pause-response.test.ts +++ b/packages/junior/tests/unit/services/auth-pause-response.test.ts @@ -10,7 +10,7 @@ describe("buildAuthPauseResponse", () => { " Update & notify the team ", ), ).toBe( - "<@U123> I need access to GitHub to continue.\n\n*Why:* Update <roadmap> & notify the team\n\nI sent you a link.", + "<@U123> I need access to GitHub to continue.\n\n**Why:** Update <roadmap> & notify the team\n\nI sent you a link.", ); }); diff --git a/packages/junior/tests/unit/slack/footer.test.ts b/packages/junior/tests/unit/slack/footer.test.ts index 108b2a2fb2..e958f92491 100644 --- a/packages/junior/tests/unit/slack/footer.test.ts +++ b/packages/junior/tests/unit/slack/footer.test.ts @@ -161,18 +161,27 @@ describe("buildSlackReplyBlocks", () => { expect(buildSlackReplyBlocks(" ", footer)).toBeUndefined(); }); - it("renders a mrkdwn section block for pre-formatted Slack mrkdwn text", () => { - // Regression test: auth-pause notices (`buildAuthPauseResponse`) emit - // Slack mrkdwn directly (mentions, `` links) rather than CommonMark. - // Slack's markdown-to-rich_text converter rejects that content inside a - // `markdown` block with `invalid_blocks`, so callers with pre-formatted - // mrkdwn must request the `mrkdwn` body format instead. - const text = - "<@U123> I need access to GitHub to continue.\n\n*Why:* check out and proceed\n\nI sent you a link."; - expect(buildSlackReplyBlocks(text, undefined, "mrkdwn")).toEqual([ + it("renders auth-pause-style text (mention, CommonMark bold, embedded URL) as a single markdown block", async () => { + // Regression test for JUNIOR-72: `buildAuthPauseResponse` is delivered + // through this same `markdown` block as every other reply. It must stay + // valid CommonMark — the earlier `*Why:*` (CommonMark italics, not bold) + // contributed to Slack's invalid_blocks rejection. + const { buildAuthPauseResponse } = await import( + "@/chat/services/auth-pause-response" + ); + const text = buildAuthPauseResponse( + "U123", + "GitHub", + "check out https://github.com/foo/bar and proceed", + ); + + expect(text).toBe( + "<@U123> I need access to GitHub to continue.\n\n**Why:** check out https://github.com/foo/bar and proceed\n\nI sent you a link.", + ); + expect(buildSlackReplyBlocks(text, undefined)).toEqual([ { - type: "section", - text: { type: "mrkdwn", text }, + type: "markdown", + text, }, ]); }); From 31932d41e596324618c5d01cacdc10789cb3b5f6 Mon Sep 17 00:00:00 2001 From: "sentry-junior[bot]" <264270552+sentry-junior[bot]@users.noreply.github.com> Date: Tue, 8 Sep 2026 15:25:05 +0000 Subject: [PATCH 3/3] fix(slack): split a leading Slack mention out of the markdown block Per Cursor Bugbot on #1798: Slack's markdown block has no user-mention syntax (docs.slack.dev/reference/block-kit/blocks/markdown-block only lists bold/italic/links/lists/etc). A literal <@id> mention token left inside that block's text is exactly the JUNIOR-72 invalid_blocks combination the previous commit's CommonMark-bold fix did not cover. buildSlackReplyBlocks now detects a leading <@id> mention and splits it into its own leading mrkdwn context block (where mentions are documented and already used, e.g. the footer's ID/attribution context), leaving the remaining body as pure CommonMark in the markdown block. auth-pause-response.ts and turn.ts are unchanged; this generic split applies to any reply that opens with a mention, not only auth-pause notices. Verified: pnpm typecheck clean; full tests/unit, tests/component, and tests/integration/slack suites pass (only the 2 pre-existing, unrelated nitro-plugin-module package-resolution failures remain). Added regression tests for the leading-mention split, the mention-only edge case, and confirmed a mid-sentence mention is left untouched. --- packages/junior/src/chat/slack/footer.ts | 37 ++++++++++++++++--- .../junior/tests/unit/slack/footer.test.ts | 36 +++++++++++++++--- 2 files changed, 61 insertions(+), 12 deletions(-) diff --git a/packages/junior/src/chat/slack/footer.ts b/packages/junior/src/chat/slack/footer.ts index 53709d2d3c..ba41e1fdec 100644 --- a/packages/junior/src/chat/slack/footer.ts +++ b/packages/junior/src/chat/slack/footer.ts @@ -90,6 +90,17 @@ export function buildSlackReplyFooter(args: { : undefined; } +/** + * Slack's `markdown` block renders standard Markdown and has no user-mention + * syntax (see docs.slack.dev/reference/block-kit/blocks/markdown-block). A + * literal `<@id>` mention token — valid only in `mrkdwn` text — left inside a + * `markdown` block makes Slack's markdown-to-rich_text conversion mis-tag an + * inline element and reject the whole message with `invalid_blocks` + * (JUNIOR-72). Replies that open with a mention (for example auth-pause + * notices) split that mention into its own `mrkdwn` context block instead. + */ +const LEADING_SLACK_MENTION_RE = /^<@([UW][A-Z0-9]+)>[ \t]*/; + /** Build Slack blocks for a reply chunk using the Slack-flavored markdown block for the body. */ export function buildSlackReplyBlocks( text: string, @@ -99,12 +110,26 @@ export function buildSlackReplyBlocks( return undefined; } - const blocks: SlackMessageBlock[] = [ - { - type: "markdown", - text, - }, - ]; + const mentionMatch = LEADING_SLACK_MENTION_RE.exec(text); + const body = mentionMatch ? text.slice(mentionMatch[0].length) : text; + + const blocks: SlackMessageBlock[] = []; + if (mentionMatch) { + blocks.push({ + type: "context", + elements: [{ type: "mrkdwn", text: mentionMatch[0].trimEnd() }], + }); + } + blocks.push({ + type: "markdown", + text: body, + }); + + if (mentionMatch && !body.trim()) { + // A mention-only reply has nothing left for the markdown block. Drop it + // rather than post a Slack-rejected empty `markdown` block. + blocks.pop(); + } if (footer && (footer.attribution || footer.items.length > 0)) { const attributionElements: SlackPlainTextObject[] = footer.attribution diff --git a/packages/junior/tests/unit/slack/footer.test.ts b/packages/junior/tests/unit/slack/footer.test.ts index e958f92491..404bc4c02d 100644 --- a/packages/junior/tests/unit/slack/footer.test.ts +++ b/packages/junior/tests/unit/slack/footer.test.ts @@ -161,11 +161,13 @@ describe("buildSlackReplyBlocks", () => { expect(buildSlackReplyBlocks(" ", footer)).toBeUndefined(); }); - it("renders auth-pause-style text (mention, CommonMark bold, embedded URL) as a single markdown block", async () => { - // Regression test for JUNIOR-72: `buildAuthPauseResponse` is delivered - // through this same `markdown` block as every other reply. It must stay - // valid CommonMark — the earlier `*Why:*` (CommonMark italics, not bold) - // contributed to Slack's invalid_blocks rejection. + it("renders auth-pause-style text (leading mention, CommonMark bold, embedded URL) as a leading mention context block plus a markdown body", async () => { + // Regression test for JUNIOR-72: Slack's `markdown` block has no + // user-mention syntax (docs.slack.dev/reference/block-kit/blocks/markdown-block). + // A literal `<@id>` mention left inside that block made Slack's + // markdown-to-rich_text conversion reject the auth-pause notice as + // invalid_blocks. buildAuthPauseResponse's leading mention must be split + // into its own `mrkdwn` context block instead. const { buildAuthPauseResponse } = await import( "@/chat/services/auth-pause-response" ); @@ -179,9 +181,31 @@ describe("buildSlackReplyBlocks", () => { "<@U123> I need access to GitHub to continue.\n\n**Why:** check out https://github.com/foo/bar and proceed\n\nI sent you a link.", ); expect(buildSlackReplyBlocks(text, undefined)).toEqual([ + { + type: "context", + elements: [{ type: "mrkdwn", text: "<@U123>" }], + }, + { + type: "markdown", + text: "I need access to GitHub to continue.\n\n**Why:** check out https://github.com/foo/bar and proceed\n\nI sent you a link.", + }, + ]); + }); + + it("drops the markdown block for a mention-only reply", () => { + expect(buildSlackReplyBlocks("<@U123> ", undefined)).toEqual([ + { + type: "context", + elements: [{ type: "mrkdwn", text: "<@U123>" }], + }, + ]); + }); + + it("does not treat a mention elsewhere in the text as a leading mention", () => { + expect(buildSlackReplyBlocks("cc <@U123> please review", undefined)).toEqual([ { type: "markdown", - text, + text: "cc <@U123> please review", }, ]); });