fix: render Slack replies with native formatting - #1586
Open
dcbuild3r wants to merge 1 commit into
Open
Conversation
Author
|
I tested a clean Chat SDK family upgrade from 4.31.0 to 4.39.0 in a separate worktree. Findings:
So the upgrade direction is useful, but it is not a safe substitute for this focused fix without separately porting or retiring Centaur's adapter/state patches. Full test evidence: 239 passed, 1 skipped, 13 failed on clean 4.39.0; the focused PR remains 254 passed, 1 skipped. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The model emits CommonMark, but Slack renders
mrkdwn. The two overlap enough that most output looked fine, so the mismatches were easy to miss until they showed up in real replies:[Issue #276](https://…/276)<https://…/276|Issue #276>)**Title:** Example meeting**Title:** Example meetingTwo delivery paths were sending CommonMark straight to Slack:
@chat-adapter/slackadapter passed each delta tochat.appendStreamasmarkdown_textunchanged. Slack'smarkdown_textaccepts mrkdwn, not full CommonMark, so labeled links in particular came through raw.renderFallbackFinalAnswerre-posts (or edits in) the durable final answer viathread.post/editMessageas plain text. Root DMs hit this path constantly: a top-level DM has nothread_ts, sochat.startStreamisn't usable and every answer lands as achat.postMessage— with the CommonMark intact.Fixing this at the transport boundary (rather than with prompt rules asking the model for mrkdwn) means every Slack agent built on the adapter gets native formatting, and the model can keep producing one canonical Markdown output for all surfaces.
What changed
patches/@chat-adapter__slack@4.31.0.patch(+ mechanicalpnpm-lock.yamlpatch-hash bump)appendpath, run each syntactically committable delta through the adapter's existingformatConverter.toResponseUrlText()before handing it tochat.appendStream. The existing "commit only complete constructs" buffering is what makes this safe — a link split across deltas ([Issue/#276](https://…/…) was updated.) is held until it's whole, then converted once.services/slackbotv2/src/index.tsSlackFormatConverterand convert the fallback answer to mrkdwn before both the freshthread.post(...)and theeditMessage(...)replacement.{ raw: … }so the chat SDK sends the already-converted mrkdwn as-is instead of running its own Markdown pass over it (which would re-escape or double-convert).services/slackbotv2/test/chat-sdk-emulate.test.tsrenders streamed CommonMark links as Slack-native rich links— splits a link across threeitem/agentMessage/deltaevents and asserts the joinedmarkdown_textstream is exactlyCompleted: <https://…/276|Issue #276> was updated.renders root-DM fallback answers as Slack-native rich text— drives a root DM through the fallback path and asserts bold, a bulleted bold label (• *Title:*), and a labeled link all arrive as mrkdwn with no CommonMark residue.chat.postMessagecalls so the fallback path is observable in tests.Not changed
Validation
bun test test/chat-sdk-emulate.test.ts --test-name-pattern "renders (streamed CommonMark links|root-DM fallback answers)"— 2 passedbun test test— 254 passed, 1 skippedgit diff --check— cleanbun run check:types— new code is clean; still reports the three pre-existingfetch.preconnectmock errors intest/slack-user.test.ts(unrelated, present onmain)