Skip to content

fix(slack): render markdown in post-and-edit stream fallback - #901

Merged
bensabic merged 1 commit into
vercel:mainfrom
CamdenA21:fix/slack-fallback-stream-markdown
Sep 5, 2026
Merged

fix(slack): render markdown in post-and-edit stream fallback#901
bensabic merged 1 commit into
vercel:mainfrom
CamdenA21:fix/slack-fallback-stream-markdown

Conversation

@CamdenA21

@CamdenA21 CamdenA21 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Summary

SlackAdapter.stream()'s post-and-edit fallback passes its accumulated text to postMessage/editMessage as a bare string, which SlackFormatConverter.toSlackPayload resolves to Slack's text field:

if (fallback.message) {
  await this.editMessage(threadId, fallback.message.id, committable);
} else {
  fallback.message = await this.postMessage(threadId, committable);
}

text renders classic mrkdwn (*bold*) only, not the GFM the renderer emits — so every intermediate edit during a fallback stream shows the user raw **/#/backtick syntax. Only a caller-side final replacement recovers the formatting, and only for the last frame; every frame before it was wrong while it was on screen.

It's also a materially smaller ceiling: text on chat.update caps at 4,000 characters vs. markdown_text's 12,000, so fallback mode fails on long answers sooner than it needs to — msg_too_long mid-stream, independent of the formatting bug.

StreamingMarkdownRenderer.getCommittableText() is already documented as text "safe for append-only streaming" — it holds back unclosed inline markers (**, *, ~~, `, [) and unconfirmed table headers. And the renderer's own class doc is explicit about which side owns conversion: "Outputs markdown (not platform text). Format conversion still happens in the adapter's editMessage → renderPostable → fromAst pipeline." Native mode honors that; fallback mode ships the same output through the wrong field. Wrapping it as { markdown: committable } routes it into the same markdown_text field native mode streams into, via toSlackPayload's existing markdown branch — nothing new to build.

Per Slack's reference for chat.postMessage and chat.update, markdown_text needs no scope beyond the chat:write the adapter already holds and carries no app-feature gate. Its one constraint is mutual exclusivity with text/blocks (markdown_text_conflict), and toSlackPayload emits exactly one field per branch, so that conflict can't arise here.

One behavior change worth naming: Slack documents that mobile notifications use message.text for block-based messages, and doesn't document how push previews are derived for markdown_text. In practice we see no notification regression — our app already posts markdown_text in native mode and for final message replacements — but flagging it rather than leaving it to be discovered.

How we hit this

A Slack Workflow Builder–authored message (posted as a bot, no real event.user) leaves recipient_user_id invalid for native streaming, so the adapter drops into fallback from the first send and stays there for the whole answer. That root cause is separate and app-side — not part of this PR — but it's what made this reproducible for us. Note it isn't the only route in: once switchToFallback() latches nativeStreamingBroken on a feature_not_enabled / method_deprecated / unknown_method error, every subsequent stream on that adapter instance takes this path.

Test plan

  • pnpm validate — 43/43 tasks pass (knip, lint, typecheck, test, build).
  • pnpm test:workspace — 3,714 passed / 6 skipped, 102 files.
  • New test in packages/adapter-slack/src/index.test.ts: streams fallback updates through markdown, not plain text — asserts every postMessage/editMessage call in fallback mode receives { markdown: <string> } rather than a bare string, and that markdown syntax survives to the last frame.
  • Three existing native streaming fallback tests read the posted payload to assert content; updated to read .markdown via a small markdownOf helper. These are the only places in the suite that assumed the bare-string shape — worth knowing for anyone auditing the blast radius.

Checklist

  • All commits are signed and verified
  • All commits are signed off for the DCO (git commit -s)
  • pnpm validate passes
  • Changeset added (patch, @chat-adapter/slack)
  • Documentation updated (N/A — no public API change)

The stream fallback passed its accumulated text to postMessage/editMessage
as a bare string, which toSlackPayload resolves to Slack's `text` field.
`text` renders legacy mrkdwn only, so every intermediate edit showed raw
`**`/`#` markdown to the user, and it caps at 4,000 characters instead of
`markdown_text`'s 12,000.

`getCommittableText()` already produces append-safe markdown, so wrapping it
as `{ markdown }` routes it through the same `markdown_text` field native
mode streams into. `markdown_text` needs no scope beyond `chat:write` and
carries no app-feature gate; it is only mutually exclusive with `text`/
`blocks`, and toSlackPayload emits exactly one field, so the conflict cannot
arise.

Signed-off-by: CamdenA21 <camden@sandstone.com>
@CamdenA21
CamdenA21 requested a review from a team as a code owner September 4, 2026 23:16
@vercel

vercel Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

@CamdenA21 is attempting to deploy a commit to the Vercel Team on Vercel.

A member of the Team first needs to authorize it.

@bensabic
bensabic merged commit 8fdaf4a into vercel:main Sep 5, 2026
13 of 15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants