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 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/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 6657dd4137..404bc4c02d 100644 --- a/packages/junior/tests/unit/slack/footer.test.ts +++ b/packages/junior/tests/unit/slack/footer.test.ts @@ -160,6 +160,55 @@ describe("buildSlackReplyBlocks", () => { expect(buildSlackReplyBlocks(" ", footer)).toBeUndefined(); }); + + 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" + ); + 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: "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: "cc <@U123> please review", + }, + ]); + }); }); describe("getDashboardTaskLink", () => {