fix(slack): scope invalid_blocks fix to pre-formatted mrkdwn replies - #1798
fix(slack): scope invalid_blocks fix to pre-formatted mrkdwn replies#1798sentry-junior[bot] wants to merge 3 commits into
Conversation
Auth-pause notices (buildAuthPauseResponse) already emit Slack mrkdwn (<@user> mentions, <url> 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 <david@sentry.io>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![sentry-junior[bot]](https://avatars.githubusercontent.com/in/2960336?s=60&v=4){kind=small}
There was a problem hiding this comment.
Good question — worth being explicit about. This isn't an arbitrary split; buildAuthPauseResponse is deliberately Slack mrkdwn today, not CommonMark:
return bounded
.replaceAll("&", "&")
.replaceAll("<", "<")
.replaceAll(">", ">");That's the exact same escaping as escapeSlackMrkdwnText (used everywhere else we build mrkdwn, e.g. the footer context items) — Slack's mrkdwn convention for neutralizing its own special tokens (<@id>, <url>, <!here>). CommonMark doesn't use that escaping at all, so this text was never CommonMark to begin with, and it also needs a real <@${slackUserId}> mention token that has no CommonMark equivalent (the markdown block spec has no mention syntax — see docs.slack.dev/reference/block-kit/blocks/markdown-block). Both Seer RCAs on JUNIOR-72 point at this same text being "mrkdwn-formatted... in a markdown block type" as the cause of the malformed rich_text conversion.
Converting it to CommonMark would mean writing a new Markdown-escaping helper (today's escaping is mrkdwn-specific) and we'd still need the literal mention token, so we wouldn't actually eliminate the two-format reality — we'd just be escaping it differently while keeping the same underlying mismatch. Model-authored replies stay CommonMark-only because normalizeSlackReplyMarkdown is built and tested for that (bold, tables, headers, links) and converting that path to mrkdwn was #1797's mistake — it silently dropped that formatting and broke 5 tests.
So: two body formats, not by preference, but because we have two genuinely different kinds of text (model-authored CommonMark vs. our own pre-formatted Slack mrkdwn for one system notice). Happy to rename/restructure if there's a cleaner shape you'd prefer, but I don't think we can honestly collapse it to one format without reintroducing the bug or losing formatting fidelity somewhere.
|
seems like it'd be easy enough to resolve the problem and use universal commonmark everywhere. lets take a stab at that. |
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 <david@sentry.io>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e723f4a. Configure here.
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.
1 participant
Fixes
SlackActionError: An API error occurred: invalid_blocksfrompostAuthPauseNotice(JUNIOR-72). Replaces #1797.Root cause:
buildAuthPauseResponsealready emits Slack mrkdwn directly (<@user>mentions,<url>links, single-*emphasis) — it isn't CommonMark.sendSlackReply/buildSlackReplyBlocksputs every reply body in a{ type: "markdown" }block, which Slack renders from standard Markdown via its own markdown-to-rich_text conversion. Feeding raw mrkdwn tokens (especially<@user>mentions) into that converter makes Slack mis-tag an inline rich_text element with aurlproperty, and the API rejects the whole message asinvalid_blocks.Why not #1797's approach: that PR switched the shared body block from
markdowntosection/mrkdwnfor every reply, not just the auth-pause path. Our outbound normalizer (normalizeSlackReplyMarkdown) intentionally preserves standard Markdown (**bold**,[text](url), tables, headers) for delivery through themarkdownblock per this package's own architecture note ("Translate Junior Markdown to Slackmrkdwnonly at the outbound boundary"). Switching everything tomrkdwnsilently drops that formatting for normal agent replies, and broke 5 existing contract/component tests that assert themarkdownblock is used.Fix: give
buildSlackReplyBlocks/sendSlackReplyan explicitbodyFormat("commonmark"default,"mrkdwn"opt-in).postAuthPauseNoticenow requests"mrkdwn"for its pre-formatted text; everything else keeps the existingmarkdownblock/CommonMark behavior unchanged.Testing:
mrkdwnbody format produces a validsection/mrkdwnblock for auth-pause-style text (mentions +<url>links).pnpm typecheckclean.pnpm vitest run tests/unit tests/component tests/integration— 2833/2835 passing (2 unrelated pre-existingnitro-plugin-modulefailures caused by sandbox package resolution, not this change); all Slack/footer/reply/auth-pause suites green.Follow-up to autofix PR #1797 — closing that one in favor of this scoped fix.
Requested by David Cramer.
--
View Junior Session [Sentry]