Skip to content

fix(gateway): strip language tag from Slack fenced code blocks - #21085

Closed
z23 wants to merge 1 commit into
NousResearch:mainfrom
z23:fix/slack-code-block-language-tag
Closed

fix(gateway): strip language tag from Slack fenced code blocks#21085
z23 wants to merge 1 commit into
NousResearch:mainfrom
z23:fix/slack-code-block-language-tag

Conversation

@z23

@z23 z23 commented May 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Slack's mrkdwn does not strip the optional language tag from fenced code blocks the way GitHub-flavored markdown does — it renders the tag as a literal first line of the rendered code block. When the agent emitted ```text fences around raw command output (a common LLM habit), every such block in Slack arrived looking like:

text
<the actual command output>

format_message() "protects" fenced code blocks by stashing them verbatim behind a placeholder, so the language tag survived all the way to chat_postMessage. This PR drops the tag from the opening fence before stashing.

Update 2026-07-15: rebased onto current main and ported to plugins/platforms/slack/adapter.py following the adapter migration in 5600105 (the original patch targeted the now-removed gateway/platforms/slack.py). While porting, the strip was hardened: it now only fires for a genuine opening fence — a ``` at the start of a line, tagged with a single token (no spaces/backticks) — and preserves the original (CR)LF. Adversarial old-vs-new fuzzing showed the original \A```[^\n]*\n substitution could silently delete real content when the deliberately loose fence-protection regex groups a mid-line ``` (e.g. an inline ```span``` wrapping across a newline) as an "opening fence". With the line-start guard, the only behavioral delta vs main is the tag strip itself.

Why this is still relevant now that Block Kit rendering is on main

The Block Kit markdown rendering path (#8552 / #19108) is on main, but it does not cover this: render_blocks() intercepts fenced code itself (into rich_text_preformatted) and never routes fences through format_message(), while format_message() remains the producer of every surface that is still plain mrkdwn:

  • the plain-text fallback field sent alongside blocks on every message — Slack uses it for notifications, search indexing, screen readers, and quoted/forwarded message previews;
  • slash-command ephemeral replies (response_url path);
  • standalone cron delivery (chat.postMessage without the in-process adapter);
  • any message where the block renderer declines and the caller falls back to text.

Without this fix, all of those surfaces still leak the literal text line.

Changes

File Change
plugins/platforms/slack/adapter.py format_message() now strips the language tag from the opening fence inside the code-block protection step. Stripping requires a line-start fence and a single-token tag; the original line ending is preserved.
tests/gateway/test_slack.py Updated 3 existing assertions that expected lang-tag preservation; added 9 regression tests: the ```text case mirroring the bug, bare/single-line fences left untouched, tags with trailing spaces, CRLF fences (tagged and untagged), mid-line ``` spans (multi-word and single-token), and back-to-back fences.

Test plan

  • scripts/run_tests.sh tests/gateway/test_slack.py tests/gateway/test_slack_block_kit.py tests/gateway/test_slack_block_kit_adapter.py — 283 passed, 0 failed
  • Full tests/gateway/ run — only pre-existing failures unrelated to this change (WeCom optional XML dependency, a startup-restart race test), reproduced identically on pristine main
  • Adversarial differential fuzz (20,000 generated messages, main's format_message vs this branch): no deviation beyond the intended single-token tag strip on line-start fences
  • Manually verified on a live Slack workspace (pre-migration adapter; the fence-stripping behavior for the ```text case is identical): messages from the agent containing ```text\n…\n``` no longer render a literal text line at the top of the code block
  • No cross-platform impact — change is confined to the Slack adapter's format_message()
  • ruff check clean on both files

Tested on

  • Linux, Python 3.12.

Related: #8552, #19108

@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery platform/slack Slack app adapter P2 Medium — degraded but workaround exists labels May 7, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused regression coverage. The underlying issue remains on current main: plugins/platforms/slack/adapter.py:1926-1930 still stashes fenced blocks unchanged, and tests/gateway/test_slack.py:2207-2209 currently expects a python tag to survive.

Problems

  • The PR edits gateway/platforms/slack.py, but commit 5600105478ffde29d7566b45421b100eaa29c4ef migrated that adapter to plugins/platforms/slack/adapter.py; the old path is absent at current HEAD.

Suggested changes

  • Port the fence-opening normalization and its tests to plugins/platforms/slack/adapter.py:1926 and the current TestFormatMessage assertions in tests/gateway/test_slack.py.
  • The formatter remains the correct shared layer: the send and edit paths call it at plugins/platforms/slack/adapter.py:1377 and :1487.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
@z23
z23 force-pushed the fix/slack-code-block-language-tag branch from 0a7b71c to 0550ec0 Compare July 15, 2026 11:46
@z23

z23 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

Ported to current main as suggested — the branch has been rebased and the conflict is gone (PR is mergeable again).

  • The fix now lives in plugins/platforms/slack/adapter.py (format_message(), step 1 fence protection); the TestFormatMessage assertions in tests/gateway/test_slack.py that expected lang-tag preservation are updated, and 9 regression tests were added (including the ```text case mirroring the original bug).
  • While porting, the strip was hardened beyond the original patch. Differential fuzzing (40k generated messages, main's formatter vs this branch) showed the original \A```[^\n]*\n substitution could silently delete real content when the deliberately loose fence-protection regex groups a mid-line ``` (e.g. an inline ```span``` wrapping across a newline) as an "opening fence". Stripping now only fires for a ``` at the start of a line, tagged with a single space/backtick-free token, and preserves the original (CR)LF. With that guard the fuzz reports zero deviations beyond the intended tag strip.
  • The Block Kit path is unaffected: render_blocks() intercepts fences itself before mrkdwn_fn is applied. This change covers the mrkdwn surfaces that still go through format_message() — the plain-text fallback field (notifications, search, accessibility), slash-command ephemeral replies, and standalone cron delivery — which is why it remains complementary to the Block Kit rendering that landed via fix(slack): use Block Kit markdown block type instead of legacy mrkdwn #19108.
  • scripts/run_tests.sh tests/gateway/test_slack.py tests/gateway/test_slack_block_kit.py tests/gateway/test_slack_block_kit_adapter.py — 283 passed, 0 failed. A full tests/gateway/ run shows only pre-existing failures unrelated to this change (WeCom optional XML dependency, a startup-restart race test), reproduced identically on pristine main.

@z23
z23 force-pushed the fix/slack-code-block-language-tag branch from 0550ec0 to eebd3cd Compare July 15, 2026 12:55
Slack's mrkdwn does not strip the optional language tag from fenced
code blocks like GitHub-flavored markdown does — it renders
```text\nfoo\n``` as a code block whose literal first line is "text".
The agent emitted ```text fences around raw command output, which
surfaced "text" as the first line of every such block.

Drop the tag from the opening fence in format_message() before stashing
the block behind a placeholder. Stripping only fires for a genuine
opening fence — a ``` at the start of a line, tagged with a single
token (no spaces or backticks) — and the original line ending is
preserved. The fence-protection regex deliberately matches loosely, so
a mid-line ``` (e.g. an inline ```span``` wrapping across a newline)
can be grouped as an "opening fence" whose first line is real content;
differential fuzzing against the pre-change formatter (40k generated
messages) confirms the only behavioral delta is the tag strip itself.

The Block Kit renderer is unaffected: render_blocks() intercepts fences
itself before mrkdwn_fn is applied, so this only changes the mrkdwn
surfaces that still go through format_message() — the plain-text
fallback field (notifications, search indexing, accessibility),
slash-command ephemeral replies, and standalone cron delivery.

Originally written against gateway/platforms/slack.py; ported to
plugins/platforms/slack/adapter.py after the adapter migration in
5600105.

Manually verified against a live Slack workspace (pre-migration
adapter; the ```text case strips identically) — code blocks no longer
carry a literal "text" first line.
@teknium1

Copy link
Copy Markdown
Contributor

Merged via #70191 — your commit was cherry-picked/reapplied onto current main with your authorship preserved in git history: your language-tag stripping was cherry-picked with a line-start guard.

Thanks for the contribution!

@teknium1 teknium1 closed this Jul 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/slack Slack app adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants