From 3c66fee82fe5a4f703754b19209286e7129b1520 Mon Sep 17 00:00:00 2001 From: Artem Obukhov Date: Mon, 7 Sep 2026 12:45:43 +0400 Subject: [PATCH 1/2] fix(bridge): render agent markdown natively on Slack and Telegram Slack received agent prose through chat.postMessage's top-level text field, which Slack parses as mrkdwn -- a different dialect from the GFM the LLM emits. Headings, **bold**, - bullets, [label](url) links and pipe tables rendered as literal characters; only backtick code survived. Mattermost was unaffected because it parses Post.Message as real GFM. Slack prose is now emitted as Block Kit markdown blocks (slack.NewMarkdownBlock, already available in the vendored slack-go v0.25.0 -- no new dependency), which Slack renders as standard Markdown server-side. Chunked with a shared fence-aware splitter to Slack's documented 12,000-character cumulative budget across all markdown blocks per payload, capped at 50 blocks, with a visible truncation marker. The top-level text field is retained as the notification/accessibility fallback. Legacy mrkdwn link syntax that agents sometimes emit is normalized to [label](url) first, leaving <@U123> mentions and non-URL angle brackets alone. Telegram had the mirror-image bug: Send set no ParseMode at all, so every markdown character rendered verbatim. Prose is now converted to Telegram HTML and sent with ParseModeHTML; source markdown is chunked at 3,500 characters so both the raw chunk and its entity-expanded conversion stay inside Telegram's 4,096 cap. Neither platform can lose a message to a formatting failure: Slack retries once as plain text on a block-related API error and latches only on unambiguous capability rejections; Telegram retries a chunk with no ParseMode on an entity-parse error, without latching. New stdlib-only shared package internal/bridge/markdown. The Slack/Telegram RichRenderer tool-card paths, Mattermost, the queued- ack helpers and the dispatcher are unchanged. No config or schema surface. --- docs/bridge.md | 10 + internal/bridge/markdown/markdown.go | 348 ++++++++++++++ internal/bridge/markdown/markdown_test.go | 361 +++++++++++++++ internal/bridge/markdown/telegram.go | 425 ++++++++++++++++++ internal/bridge/markdown/telegram_test.go | 366 +++++++++++++++ internal/bridge/slack/adapter.go | 191 +++++++- .../bridge/slack/adapter_markdown_test.go | 343 ++++++++++++++ internal/bridge/slack/adapter_test.go | 21 + internal/bridge/telegram/adapter.go | 138 +++--- .../bridge/telegram/adapter_markdown_test.go | 276 ++++++++++++ internal/bridge/telegram/adapter_test.go | 35 ++ .../.openspec.yaml | 2 + .../bridge-slack-native-markdown/design.md | 314 +++++++++++++ .../bridge-slack-native-markdown/proposal.md | 117 +++++ .../specs/chat-bridge-adapters/spec.md | 228 ++++++++++ .../bridge-slack-native-markdown/tasks.md | 213 +++++++++ 16 files changed, 3324 insertions(+), 64 deletions(-) create mode 100644 internal/bridge/markdown/markdown.go create mode 100644 internal/bridge/markdown/markdown_test.go create mode 100644 internal/bridge/markdown/telegram.go create mode 100644 internal/bridge/markdown/telegram_test.go create mode 100644 internal/bridge/slack/adapter_markdown_test.go create mode 100644 internal/bridge/telegram/adapter_markdown_test.go create mode 100644 openspec/changes/bridge-slack-native-markdown/.openspec.yaml create mode 100644 openspec/changes/bridge-slack-native-markdown/design.md create mode 100644 openspec/changes/bridge-slack-native-markdown/proposal.md create mode 100644 openspec/changes/bridge-slack-native-markdown/specs/chat-bridge-adapters/spec.md create mode 100644 openspec/changes/bridge-slack-native-markdown/tasks.md diff --git a/docs/bridge.md b/docs/bridge.md index d821f72fb4..448338f3fe 100644 --- a/docs/bridge.md +++ b/docs/bridge.md @@ -190,6 +190,16 @@ A relay channel with **no chat platform of its own**. Outbound messages and ques - Relay frames are authenticated with HTTP Basic (the credential as password) and `202 Accepted` is the only success status. Attachments relay as **metadata only** (`fileName`, `mimeType`, `size`) — never content. - Groups / `@mention` gating don't apply; `POST /router/config/groups` rejects this channel explicitly. +## Outbound prose rendering + +Agent replies are authored as GFM (GitHub-flavored Markdown) — headings, bold/italic, links, lists, tables, fenced code. Each adapter's `Send` renders that same `Outbound.Text` into whatever markup dialect its platform actually understands, with automatic degradation to plain text if rendering is rejected. The shared, stdlib-only chunking and conversion helpers live in `internal/bridge/markdown`. + +- **Slack**: rendered as Block Kit `markdown` blocks (real GFM parsing, unlike the legacy mrkdwn `text` field, which cannot represent headings, tables, or fenced code with syntax highlighting). Text is chunked to Slack's 12,000-character cumulative budget across all blocks in one payload (`internal/bridge/markdown.BuildBlockChunks`, 3,000-char per-block target). The top-level `text` field is still sent alongside the blocks as the notification/accessibility fallback. If Slack rejects the blocks with an unambiguous block-capability error (`invalid_blocks`, `invalid_block`, `blocks_too_long`, `msg_blocks_too_long`, `invalid_block_id`), the adapter retries as plain text and sets a sticky per-identity latch so subsequent sends skip the blocks attempt entirely. A more ambiguous error (`invalid_arguments`, which Slack also returns for unrelated reasons) still retries as plain text for that one send but does **not** latch — the next send tries blocks again. +- **Telegram**: rendered as Telegram HTML (`internal/bridge/markdown.ToTelegramHTML`), the restricted tag set Telegram's `ParseMode: HTML` supports (``, ``, ``, ``/`
`, `
`, etc.) — chosen over MarkdownV2 because it requires escaping only three characters instead of ~18 reserved ones. Source markdown is chunked at 3,500 characters (`MarkdownChunkLimit`) before conversion, conservative headroom under Telegram's 4,096-character post-parse cap to absorb HTML tag overhead and `&`-style escaping. If a chunk's HTML is rejected with a parse/entity error, that one chunk is retried unformatted (no `ParseMode`) — there is no sticky latch, since a parse failure is specific to that chunk's content, not a platform capability. +- **Mattermost**: unchanged — `Post.Message` is sent as native GFM and Mattermost's own server-side parser already renders it correctly. + +The `RichRenderer` tool-card paths (`internal/bridge/slack/render.go`, `internal/bridge/telegram/render.go`) that hand-author Block Kit / legacy Markdown for tool calls, lists, tables, and status previews are separate code paths, untouched by the above — they compose their own markup directly rather than converting agent-authored GFM. + ## HTTP API (`/router/*`) All endpoints live on the existing opencode API port. Bare paths (`/send`, `/identities/*`, `/config/groups`) return 404 — everything is under `/router/*`. diff --git a/internal/bridge/markdown/markdown.go b/internal/bridge/markdown/markdown.go new file mode 100644 index 0000000000..4bb406c13b --- /dev/null +++ b/internal/bridge/markdown/markdown.go @@ -0,0 +1,348 @@ +// Package markdown provides shared, stdlib-only markdown utilities used by +// the chat-bridge adapters (Slack today; Telegram in a follow-up change). +// It has no dependency on internal/bridge or any platform SDK — mirroring +// the dependency-free discipline of internal/bridge itself — so it can be +// imported by every platform adapter package without an import cycle. +// +// Two responsibilities live here: +// +// - NormalizeSlackLinks rewrites Slack mrkdwn's own link syntax +// ( / ) into standard Markdown +// ([label](https://x) / https://x) without disturbing <@U123>-style +// mentions or incidental angle-bracket text. +// - Split is a fence-aware chunker that splits long markdown text into +// platform-sized pieces without ever cutting inside an open ``` fence, +// never splitting a multi-byte UTF-8 rune, and packing the final +// permitted chunk to the limit (appending a visible truncation marker) +// when a hard chunk-count cap is reached with content still remaining. +package markdown + +import ( + "math" + "regexp" + "strings" + "unicode/utf8" +) + +// TruncationMarker is appended to the last emitted chunk when content is +// dropped because a caller-supplied chunk-count limit was reached with +// text still remaining. Italic prose visually distinguishes it from +// agent-authored content. Uses an explicit \u2026 escape (not a literal +// … glyph) so the source file stays plain ASCII per repo convention. +const TruncationMarker = "\n\n_\u2026truncated\u2026_" + +// DefaultPayloadBudget and DefaultBlockTarget are the fallback values +// BuildBlockChunks clamps to when the caller passes a non-positive or +// otherwise unusable budget/target. They mirror Slack's documented +// markdown-block limits (see the slack package's own constants, which are +// defined independently to avoid a markdown -> slack import). +const ( + DefaultPayloadBudget = 12_000 + DefaultBlockTarget = 3_000 +) + +// slackLinkPattern matches Slack mrkdwn's own link syntax: +// or bare . Anchored on a URL scheme so `<@U123>` mentions, +// `<#C123|chan>` channel references, ``, and incidental angle-bracket +// text (e.g. "a < b && b > c") are never matched — only content that starts +// with http:// or https:// immediately after the opening `<` qualifies. +var slackLinkPattern = regexp.MustCompile(`<(https?://[^<>|]+)(?:\|([^<>]*))?>`) + +// NormalizeSlackLinks rewrites Slack mrkdwn's own link syntax into standard +// Markdown so it renders correctly through a parser that only understands +// GFM-style links (e.g. Slack's `markdown` block type). `` +// becomes `[label](https://x)`; `` (or `` — an empty +// label) becomes the bare URL `https://x`. Non-URL angle-bracket content +// (user/channel mentions, ``, or incidental `<`/`>` characters) is +// left untouched. +func NormalizeSlackLinks(s string) string { + if !strings.Contains(s, "<") { + return s + } + return slackLinkPattern.ReplaceAllStringFunc(s, func(m string) string { + sub := slackLinkPattern.FindStringSubmatch(m) + url := sub[1] + label := strings.TrimSpace(sub[2]) + if label == "" { + return url + } + return "[" + label + "](" + url + ")" + }) +} + +// fenceState tracks whether we are currently inside an open ``` fence, +// the length of the backtick run that opened it (a fence is only closed by +// a run of at least that many backticks — CommonMark's own rule, which +// also means a SHORTER backtick run inside an open fence is just content, +// not a toggle), and the fence's info string (e.g. "python"). +type fenceState struct { + open bool + markerLen int + info string +} + +// scanFence walks s line by line, updating fs for every fence-toggling line +// it finds (a line whose left-trimmed content starts with 3+ backticks). +// It is meant to be called incrementally, once per emitted chunk, so state +// carries forward correctly across chunk boundaries. +func scanFence(fs *fenceState, s string) { + for _, line := range strings.Split(s, "\n") { + trimmed := strings.TrimLeft(line, " \t") + n := countLeadingBackticks(trimmed) + if n < 3 { + continue + } + if !fs.open { + fs.open = true + fs.markerLen = n + fs.info = strings.TrimSpace(trimmed[n:]) + continue + } + // Already inside a fence: only a run at least as long as the + // opening one closes it. A shorter run is literal content. + if n >= fs.markerLen { + fs.open = false + fs.markerLen = 0 + fs.info = "" + } + } +} + +func countLeadingBackticks(s string) int { + n := 0 + for n < len(s) && s[n] == '`' { + n++ + } + return n +} + +// fenceCloseText returns the text to append to head to close the fence +// described by fs. It adds a leading newline only if head doesn't already +// end with one, so the closing delimiter always starts its own line. +func fenceCloseText(head string, fs fenceState) string { + if head != "" && !strings.HasSuffix(head, "\n") { + return "\n" + strings.Repeat("`", fs.markerLen) + } + return strings.Repeat("`", fs.markerLen) +} + +// fenceOpenText returns the text to prepend to the next chunk to reopen +// the fence described by fs, preserving its original info string. +func fenceOpenText(fs fenceState) string { + return strings.Repeat("`", fs.markerLen) + fs.info + "\n" +} + +var headingLineRe = regexp.MustCompile(`(?m)^#{1,6} `) + +// findBoundary picks the best cut point for remaining, at or before limit +// runes, in priority order: blank line, heading line, any newline, falling +// through to a hard-wrap at exactly limit runes when no candidate retains +// at least half of limit. The returned value is a rune index into +// remaining (NOT a byte offset). +func findBoundary(remaining string, limit int) int { + runeLen := utf8.RuneCountInString(remaining) + if limit >= runeLen { + return runeLen + } + prefixByteLen := runeIndexToByte(remaining, limit) + prefix := remaining[:prefixByteLen] + half := limit / 2 + + if idx := strings.LastIndex(prefix, "\n\n"); idx >= 0 { + cut := idx + 2 + if rc := utf8.RuneCountInString(remaining[:cut]); rc >= half { + return rc + } + } + + if locs := headingLineRe.FindAllStringIndex(prefix, -1); len(locs) > 0 { + last := locs[len(locs)-1] + cut := last[0] + if cut > 0 { + if rc := utf8.RuneCountInString(remaining[:cut]); rc >= half { + return rc + } + } + } + + if idx := strings.LastIndex(prefix, "\n"); idx >= 0 { + cut := idx + 1 + if rc := utf8.RuneCountInString(remaining[:cut]); rc >= half { + return rc + } + } + + return limit +} + +// runeIndexToByte returns the byte offset of the n-th rune in s (or +// len(s) if s has fewer than n runes). +func runeIndexToByte(s string, n int) int { + if n <= 0 { + return 0 + } + count := 0 + for i := range s { + if count == n { + return i + } + count++ + } + return len(s) +} + +// buildChunk cuts remRunes at rune index cut, closing any fence the cut +// broke (per fs, the fence state carried in from prior chunks). If the +// closing overhead (plus the caller's reserve, e.g. room for a truncation +// marker) would push the chunk's rune length past limit, cut is shrunk and +// the fence re-scanned until it fits — this is what guarantees every +// chunk, including ones with a broken-fence close appended, stays within +// limit runes. +// +// Returns the finalized chunk text (head), the fence state to carry into +// the next chunk (already "closed" if a close/reopen fixup happened — +// scanning the next chunk's reopening delimiter line naturally reopens +// it), the text to prepend to the next chunk (empty unless a fence was +// reopened), and the rune index actually consumed from remRunes (<= cut; +// only differs from the input cut when shrinking occurred). +func buildChunk(remRunes []rune, cut, limit, reserve int, fs fenceState) (head string, newFS fenceState, reopen string, usedCut int) { + if cut > len(remRunes) { + cut = len(remRunes) + } + if cut < 0 { + cut = 0 + } + for { + h := string(remRunes[:cut]) + fsCopy := fs + scanFence(&fsCopy, h) + + closing := "" + reopenText := "" + finalFS := fsCopy + if fsCopy.open { + closing = fenceCloseText(h, fsCopy) + reopenText = fenceOpenText(fsCopy) + finalFS = fenceState{} + } + + total := utf8.RuneCountInString(h) + utf8.RuneCountInString(closing) + reserve + if total <= limit || cut == 0 { + return h + closing, finalFS, reopenText, cut + } + overflow := total - limit + next := cut - overflow + if next >= cut { + next = cut - 1 + } + cut = next + if cut < 0 { + cut = 0 + } + } +} + +// Split divides text into chunks of at most limit runes each, never +// cutting inside an open ``` fence (closing and reopening it across the +// boundary instead) and never splitting a multi-byte UTF-8 rune. It +// prefers blank-line, then heading-line, then any-newline cut points, +// falling back to a hard rune-wrap when no such boundary retains at least +// half of limit. +// +// maxChunks caps the number of chunks emitted; <= 0 means unlimited. When +// the cap is reached with text still remaining, the final chunk is packed +// to the limit (rather than stopping at a tidy boundary) and marker is +// appended within budget — the marker's own rune length, and any fence +// the final cut broke, are reserved for BEFORE the cut is made, so the +// marker is never pushed over limit and never renders inside a code +// fence. truncated is true exactly when this happened. +// +// Whitespace-only input returns (nil, false). +func Split(text string, limit, maxChunks int, marker string) ([]string, bool) { + if strings.TrimSpace(text) == "" { + return nil, false + } + if limit <= 0 { + limit = 1 + } + if utf8.RuneCountInString(text) <= limit { + return []string{text}, false + } + + markerLen := utf8.RuneCountInString(marker) + var chunks []string + remaining := text + fs := fenceState{} + truncated := false + + for utf8.RuneCountInString(remaining) > limit { + remRunes := []rune(remaining) + isLast := maxChunks > 0 && len(chunks)+1 >= maxChunks + + if isLast { + cut := limit - markerLen + head, _, _, _ := buildChunk(remRunes, cut, limit, markerLen, fs) + chunks = append(chunks, head+marker) + truncated = true + remaining = "" + break + } + + cut := findBoundary(remaining, limit) + head, newFS, reopen, usedCut := buildChunk(remRunes, cut, limit, 0, fs) + chunks = append(chunks, head) + fs = newFS + remaining = reopen + string(remRunes[usedCut:]) + } + + if remaining != "" { + chunks = append(chunks, remaining) + } + + return chunks, truncated +} + +// BuildBlockChunks is the Slack-facing helper: it normalizes text via +// NormalizeSlackLinks, derives a per-chunk rune limit from the payload +// budget and per-block target (blockCount = ceil(payloadBudget / +// perBlockTarget); perChunkLimit = floor(payloadBudget / blockCount), so +// blockCount * perChunkLimit never exceeds payloadBudget), and calls +// Split with that limit and blockCount as the chunk cap. The result +// always satisfies sum(runeLen(chunk)) <= payloadBudget. +// +// Non-positive payloadBudget clamps to DefaultPayloadBudget; non-positive +// or oversized perBlockTarget (bigger than the budget) clamps to +// DefaultBlockTarget (itself capped to the budget). +func BuildBlockChunks(text string, payloadBudget, perBlockTarget int, marker string) ([]string, bool) { + if payloadBudget <= 0 { + payloadBudget = DefaultPayloadBudget + } + if perBlockTarget <= 0 || perBlockTarget > payloadBudget { + perBlockTarget = DefaultBlockTarget + if perBlockTarget > payloadBudget { + perBlockTarget = payloadBudget + } + } + + normalized := NormalizeSlackLinks(text) + if strings.TrimSpace(normalized) == "" { + return nil, false + } + + blockCount := int(math.Ceil(float64(payloadBudget) / float64(perBlockTarget))) + if blockCount < 1 { + blockCount = 1 + } + perChunkLimit := payloadBudget / blockCount // floor division + if perChunkLimit < 1 { + // Budget smaller than blockCount: shrink blockCount so the + // sum(chunkLengths) <= payloadBudget guarantee still holds. + perChunkLimit = 1 + blockCount = payloadBudget + if blockCount < 1 { + blockCount = 1 + } + } + + return Split(normalized, perChunkLimit, blockCount, marker) +} diff --git a/internal/bridge/markdown/markdown_test.go b/internal/bridge/markdown/markdown_test.go new file mode 100644 index 0000000000..6792322abc --- /dev/null +++ b/internal/bridge/markdown/markdown_test.go @@ -0,0 +1,361 @@ +package markdown + +import ( + "fmt" + "reflect" + "strings" + "testing" + "unicode/utf8" +) + +// --- NormalizeSlackLinks ----------------------------------------------- + +func TestNormalizeSlackLinks(t *testing.T) { + tests := []struct { + name string + in string + want string + }{ + { + name: "labelled link", + in: "Triggered on `c2`.", + want: "Triggered [MySQL #12](https://tc/build) on `c2`.", + }, + { + name: "bare link", + in: "See for details.", + want: "See https://gitlab.com/piano/x/-/merge_requests/1 for details.", + }, + { + name: "empty label", + in: "", + want: "https://example.com", + }, + { + name: "two labelled links", + in: " and ", + want: "[A](https://a.test) and [B](https://b.test)", + }, + { + name: "already-GFM content untouched", + in: "# Title\n\n- [MR !1](https://gitlab.com/x/-/merge_requests/1)\n- **bold**", + want: "# Title\n\n- [MR !1](https://gitlab.com/x/-/merge_requests/1)\n- **bold**", + }, + { + name: "incidental angle brackets untouched", + in: "if a < b && b > c then ", + want: "if a < b && b > c then ", + }, + { + name: "user mention untouched", + in: "cc <@U123456> please review", + want: "cc <@U123456> please review", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := NormalizeSlackLinks(tt.in) + if got != tt.want { + t.Errorf("NormalizeSlackLinks(%q) = %q, want %q", tt.in, got, tt.want) + } + }) + } +} + +// --- Split --------------------------------------------------------------- + +func TestSplit_FitsWithinLimitReturnsVerbatim(t *testing.T) { + text := "hello world, this fits easily." + chunks, truncated := Split(text, 1000, 0, TruncationMarker) + if truncated { + t.Fatalf("truncated = true, want false") + } + if len(chunks) != 1 || chunks[0] != text { + t.Fatalf("chunks = %#v, want single verbatim chunk", chunks) + } +} + +func TestSplit_WhitespaceOnlyInputReturnsNil(t *testing.T) { + chunks, truncated := Split(" \n\t ", 10, 0, TruncationMarker) + if chunks != nil || truncated { + t.Fatalf("chunks=%#v truncated=%v, want (nil, false)", chunks, truncated) + } + chunks, truncated = Split("", 10, 0, TruncationMarker) + if chunks != nil || truncated { + t.Fatalf("chunks=%#v truncated=%v, want (nil, false) for empty input", chunks, truncated) + } +} + +func TestSplit_FencesBalancedAcrossChunks(t *testing.T) { + text := buildParagraphsWithFence(40, 60) + chunks, _ := Split(text, 120, 0, TruncationMarker) + if len(chunks) < 2 { + t.Fatalf("expected multiple chunks, got %d", len(chunks)) + } + for i, c := range chunks { + if n := countBacktickToggles(c); n%2 != 0 { + t.Errorf("chunk %d has odd fence-toggle count %d (not independently valid): %q", i, n, c) + } + } +} + +func TestSplit_NoContentLossWhenNotTruncated(t *testing.T) { + text := buildParagraphsWithFence(40, 60) + chunks, truncated := Split(text, 120, 0, TruncationMarker) + if truncated { + t.Fatalf("truncated = true, want false (maxChunks unlimited)") + } + var got []string + for _, c := range chunks { + got = append(got, wordsSkippingFenceLines(c)...) + } + want := wordsSkippingFenceLines(text) + if !reflect.DeepEqual(got, want) { + t.Fatalf("word list mismatch after chunking:\ngot: %v\nwant: %v", got, want) + } +} + +func TestSplit_FenceSpanningBoundaryReopensWithSameInfo(t *testing.T) { + intro := "Intro paragraph with some words here to fill the buffer nicely." + var codeLines strings.Builder + for i := 0; i < 60; i++ { + codeLines.WriteString(fmt.Sprintf("line %02d of code content here\n", i)) + } + text := intro + "\n\n```python\n" + codeLines.String() + "```\n\nOutro paragraph text." + + chunks, truncated := Split(text, 200, 0, TruncationMarker) + if truncated { + t.Fatalf("truncated = true, want false") + } + if len(chunks) < 2 { + t.Fatalf("expected the fence to force a split, got %d chunk(s)", len(chunks)) + } + + foundReopen := false + for i := 0; i < len(chunks)-1; i++ { + lines := strings.Split(chunks[i], "\n") + last := strings.TrimSpace(lines[len(lines)-1]) + if last != "```" { + continue + } + nextFirst := strings.SplitN(chunks[i+1], "\n", 2)[0] + if nextFirst != "```python" { + t.Errorf("chunk %d closes a fence but chunk %d reopens as %q, want %q", i, i+1, nextFirst, "```python") + } + foundReopen = true + } + if !foundReopen { + t.Fatalf("no chunk boundary closed+reopened the fence; test fixture/limit needs adjustment") + } + + // Every chunk must independently be fence-balanced. + for i, c := range chunks { + if n := countBacktickToggles(c); n%2 != 0 { + t.Errorf("chunk %d not fence-balanced (toggles=%d): %q", i, n, c) + } + } +} + +func TestSplit_MaxChunksHardCap(t *testing.T) { + var sb strings.Builder + for i := 0; i < 30; i++ { + sb.WriteString(strings.Repeat("word ", 20)) + sb.WriteString("\n\n") + } + text := sb.String() + limit := 100 + + chunks, truncated := Split(text, limit, 3, TruncationMarker) + if len(chunks) != 3 { + t.Fatalf("len(chunks) = %d, want 3 (hard cap)", len(chunks)) + } + if !truncated { + t.Fatalf("truncated = false, want true") + } + last := chunks[len(chunks)-1] + if !strings.HasSuffix(last, TruncationMarker) { + t.Errorf("last chunk does not end with marker: %q", last) + } + lastRunes := utf8.RuneCountInString(last) + if lastRunes > limit { + t.Errorf("last chunk rune length %d exceeds limit %d", lastRunes, limit) + } + if lastRunes <= limit/2 { + t.Errorf("last chunk rune length %d did not pack near the limit (limit/2=%d)", lastRunes, limit/2) + } + for i, c := range chunks { + if n := utf8.RuneCountInString(c); n > limit { + t.Errorf("chunk %d rune length %d exceeds limit %d", i, n, limit) + } + } +} + +func TestSplit_RuneSafetyMultiByte(t *testing.T) { + text := strings.Repeat("h\u00e9llo \U0001F600 w\u00f6rld ", 900) + limit := 200 + + chunks, truncated := Split(text, limit, 0, TruncationMarker) + if truncated { + t.Fatalf("truncated = true, want false (maxChunks unlimited)") + } + if len(chunks) < 2 { + t.Fatalf("expected multiple chunks for a long repeated string") + } + for i, c := range chunks { + if !utf8.ValidString(c) { + t.Errorf("chunk %d is not valid UTF-8: %q", i, c) + } + if n := utf8.RuneCountInString(c); n > limit { + t.Errorf("chunk %d rune length %d exceeds limit %d", i, n, limit) + } + } +} + +func TestSplit_TruncationMarkerNeverInsideFence(t *testing.T) { + var body strings.Builder + for i := 0; i < 500; i++ { + body.WriteString("x\n") + } + text := "```go\n" + body.String() + "```\n" + total := utf8.RuneCountInString(text) + + for limit := 5; limit <= total; limit++ { + chunks, truncated := Split(text, limit, 1, TruncationMarker) + if len(chunks) == 0 { + continue + } + last := chunks[len(chunks)-1] + if truncated && !strings.HasSuffix(last, TruncationMarker) { + t.Fatalf("limit=%d: truncated chunk missing marker: %q", limit, tail(last, 40)) + } + if n := countBacktickToggles(last); n%2 != 0 { + t.Fatalf("limit=%d: unbalanced fence around marker (toggles=%d): %q", limit, n, tail(last, 60)) + } + } +} + +// --- BuildBlockChunks ------------------------------------------------------ + +func TestBuildBlockChunks_TruncatesWithinBudget(t *testing.T) { + text := strings.Repeat("word ", 3000) // ~15,000 runes, over the 12,000 budget + chunks, truncated := BuildBlockChunks(text, 12000, 3000, TruncationMarker) + if !truncated { + t.Fatalf("truncated = false, want true") + } + if len(chunks) > 4 { + t.Fatalf("len(chunks) = %d, want <= 4", len(chunks)) + } + sum := 0 + for _, c := range chunks { + sum += utf8.RuneCountInString(c) + } + if sum > 12000 { + t.Errorf("sum(runeLen) = %d, want <= 12000", sum) + } + if !strings.HasSuffix(chunks[len(chunks)-1], TruncationMarker) { + t.Errorf("last chunk does not end with truncation marker") + } +} + +func TestBuildBlockChunks_ShortInputSingleChunkNoTruncation(t *testing.T) { + chunks, truncated := BuildBlockChunks("hello **world**", 12000, 3000, TruncationMarker) + if truncated { + t.Fatalf("truncated = true, want false") + } + if len(chunks) != 1 || chunks[0] != "hello **world**" { + t.Fatalf("chunks = %#v, want single verbatim chunk", chunks) + } +} + +func TestBuildBlockChunks_NormalizesLinks(t *testing.T) { + chunks, _ := BuildBlockChunks("", 12000, 3000, TruncationMarker) + if len(chunks) != 1 || chunks[0] != "[y](https://x.test)" { + t.Fatalf("chunks = %#v, want normalized link", chunks) + } +} + +func TestBuildBlockChunks_ClampsNonPositiveBudget(t *testing.T) { + chunks, truncated := BuildBlockChunks("hello", 0, 0, TruncationMarker) + if truncated || len(chunks) != 1 || chunks[0] != "hello" { + t.Fatalf("zero budget: chunks=%#v truncated=%v, want single verbatim chunk", chunks, truncated) + } + chunks, truncated = BuildBlockChunks("hello", -5, -5, TruncationMarker) + if truncated || len(chunks) != 1 || chunks[0] != "hello" { + t.Fatalf("negative budget: chunks=%#v truncated=%v, want single verbatim chunk", chunks, truncated) + } +} + +// --- test helpers ---------------------------------------------------------- + +// buildParagraphsWithFence constructs a multi-paragraph markdown document +// (blank-line separated) containing one fenced code block, long enough +// that a small limit forces multiple chunks. +func buildParagraphsWithFence(paragraphs, wordsPerParagraph int) string { + var sb strings.Builder + for i := 0; i < paragraphs; i++ { + for j := 0; j < wordsPerParagraph; j++ { + fmt.Fprintf(&sb, "p%dw%d ", i, j) + // Wrap every few words so no single line is long enough to + // force a hard-wrap (which is explicitly allowed to split a + // line's content and would legitimately break the + // word-preservation invariant this fixture is meant to + // exercise via the newline/blank-line boundary tiers only). + if (j+1)%8 == 0 { + sb.WriteString("\n") + } + } + sb.WriteString("\n\n") + if i == paragraphs/2 { + sb.WriteString("```text\n") + for k := 0; k < 10; k++ { + fmt.Fprintf(&sb, "code line %d\n", k) + } + sb.WriteString("```\n\n") + } + } + return sb.String() +} + +// isFenceDelimiterLine reports whether line is nothing but a fence +// delimiter (3+ backticks, optionally followed by an info string) — used +// to skip both original and chunker-synthesized delimiter lines when +// comparing word content across chunk boundaries. +func isFenceDelimiterLine(line string) bool { + trimmed := strings.TrimSpace(line) + return len(trimmed) >= 3 && strings.HasPrefix(trimmed, "```") +} + +// wordsSkippingFenceLines returns the whitespace-separated words of text, +// skipping any line that is purely a fence delimiter. +func wordsSkippingFenceLines(text string) []string { + var words []string + for _, line := range strings.Split(text, "\n") { + if isFenceDelimiterLine(line) { + continue + } + words = append(words, strings.Fields(line)...) + } + return words +} + +// countBacktickToggles counts lines that open or close a ``` fence. This +// is an independent (deliberately simpler) check from the package's own +// fenceState/scanFence machinery, used to assert chunks are individually +// fence-balanced without relying on the code under test to grade itself. +func countBacktickToggles(text string) int { + n := 0 + for _, line := range strings.Split(text, "\n") { + if isFenceDelimiterLine(line) { + n++ + } + } + return n +} + +func tail(s string, n int) string { + r := []rune(s) + if len(r) <= n { + return s + } + return string(r[len(r)-n:]) +} diff --git a/internal/bridge/markdown/telegram.go b/internal/bridge/markdown/telegram.go new file mode 100644 index 0000000000..ffe3512ff8 --- /dev/null +++ b/internal/bridge/markdown/telegram.go @@ -0,0 +1,425 @@ +// ToTelegramHTML converts GFM-flavored markdown (as an LLM emits it) into +// Telegram's restricted HTML dialect (models.ParseModeHTML). Telegram's +// parser supports only a small, fixed tag set — /, /, +// /, //, , ,
,
+// 
, 
, — and +// requires escaping exactly three characters (&, <, >) everywhere else. +// See design.md "Telegram GFM -> HTML mapping" for the full construct +// table this file implements. +package markdown + +import ( + "regexp" + "strconv" + "strings" +) + +var ( + headingRe = regexp.MustCompile(`^#{1,6}\s+(.*)$`) + hrRe = regexp.MustCompile(`^(-{3,}|\*{3,}|_{3,})$`) + taskItemRe = regexp.MustCompile(`^(\s*)[-*+] \[([ xX])\] (.*)$`) + ulItemRe = regexp.MustCompile(`^(\s*)[-*+] (.*)$`) + inlineCodeRe = regexp.MustCompile("`([^`\n]+)`") +) + +// ToTelegramHTML converts s from GFM markdown to Telegram-safe HTML. +// +// Implementation shape (see design.md): code spans and fenced code +// blocks are extracted into opaque placeholder tokens FIRST, so no later +// inline-emphasis or link substitution can ever fire inside code +// content. Line-oriented block transforms (headings, lists, blockquotes, +// tables, horizontal rules) and inline transforms (bold/italic/strike/ +// links) run on what remains, escaping "&<>" in plain-text runs as they +// go. Finally the placeholders are restored as escaped /
+// content.
+//
+// Never panics; unbalanced markers (unclosed "**", "[label](" with no
+// closing paren) are left as literal escaped text rather than emitting a
+// broken tag. An unclosed ``` fence is the one exception called out by
+// the design: because we already commit to opening a 
 once we see
+// the opening delimiter, we auto-close it with whatever content follows,
+// rather than leaving a dangling, unparseable code fence marker.
+func ToTelegramHTML(s string) string {
+	if s == "" {
+		return ""
+	}
+	placeholders := map[string]string{}
+	counter := 0
+	withoutFences := extractFences(s, &counter, placeholders)
+	withoutCode := extractInlineCode(withoutFences, &counter, placeholders)
+	converted := convertBlocks(withoutCode)
+	return restorePlaceholders(converted, placeholders)
+}
+
+// newPlaceholder returns the next opaque placeholder token. The token
+// body is restricted to a NUL byte, the letters "md", and digits — none
+// of which are markdown metacharacters or HTML-escapable characters, so
+// the token can pass through every later transform untouched and is
+// restored verbatim in the final pass.
+func newPlaceholder(counter *int) string {
+	*counter++
+	return "\x00md" + strconv.Itoa(*counter) + "\x00"
+}
+
+// restorePlaceholders replaces every placeholder token with its final
+// (already-escaped) HTML content.
+func restorePlaceholders(s string, placeholders map[string]string) string {
+	if len(placeholders) == 0 {
+		return s
+	}
+	for token, html := range placeholders {
+		s = strings.ReplaceAll(s, token, html)
+	}
+	return s
+}
+
+// extractFences scans s line by line for ``` fences (reusing the same
+// countLeadingBackticks helper markdown.go's chunker uses) and replaces
+// each fenced block — delimiters, info string and all — with a single
+// placeholder line. A fence with no matching close consumes the rest of
+// the text as its content rather than being left unrecognized, so the
+// eventual restoration always emits a well-formed, closed 
.
+func extractFences(s string, counter *int, placeholders map[string]string) string {
+	if !strings.Contains(s, "```") {
+		return s
+	}
+	lines := strings.Split(s, "\n")
+	out := make([]string, 0, len(lines))
+	i := 0
+	for i < len(lines) {
+		line := lines[i]
+		trimmed := strings.TrimLeft(line, " \t")
+		n := countLeadingBackticks(trimmed)
+		if n < 3 {
+			out = append(out, line)
+			i++
+			continue
+		}
+		lang := strings.TrimSpace(trimmed[n:])
+
+		closeIdx := -1
+		for j := i + 1; j < len(lines); j++ {
+			t := strings.TrimLeft(lines[j], " \t")
+			cn := countLeadingBackticks(t)
+			if cn >= n && strings.TrimSpace(t[cn:]) == "" {
+				closeIdx = j
+				break
+			}
+		}
+
+		var content string
+		var next int
+		if closeIdx >= 0 {
+			content = strings.Join(lines[i+1:closeIdx], "\n")
+			next = closeIdx + 1
+		} else {
+			// Unclosed fence: treat everything to the end of the text
+			// as code content so we still emit a well-formed 
.
+			content = strings.Join(lines[i+1:], "\n")
+			next = len(lines)
+		}
+
+		token := newPlaceholder(counter)
+		placeholders[token] = renderFencedCode(lang, content)
+		out = append(out, token)
+		i = next
+	}
+	return strings.Join(out, "\n")
+}
+
+// renderFencedCode renders one fenced block's final HTML. Content is
+// escaped for "&<>" only — never reparsed for markdown — and whitespace/
+// newlines are preserved exactly.
+func renderFencedCode(lang, content string) string {
+	if lang == "" {
+		return "
" + escapeHTML(content) + "
" + } + return `
` + escapeHTML(content) + `
` +} + +// extractInlineCode replaces every `code` span (single-backtick, +// single-line) with a placeholder token holding its escaped +// content. +func extractInlineCode(s string, counter *int, placeholders map[string]string) string { + if !strings.Contains(s, "`") { + return s + } + return inlineCodeRe.ReplaceAllStringFunc(s, func(m string) string { + content := m[1 : len(m)-1] + token := newPlaceholder(counter) + placeholders[token] = "" + escapeHTML(content) + "" + return token + }) +} + +// convertBlocks applies the line-oriented GFM->HTML mapping (tables, +// blockquotes, headings, horizontal rules, task/unordered list items) +// and, for every other line, the inline mapping via processInline. +func convertBlocks(s string) string { + lines := strings.Split(s, "\n") + out := make([]string, 0, len(lines)) + i := 0 + for i < len(lines) { + line := lines[i] + trimmed := strings.TrimSpace(line) + + if isTableLine(line) { + j := i + for j < len(lines) && isTableLine(lines[j]) { + j++ + } + if j-i >= 2 { + run := strings.Join(lines[i:j], "\n") + out = append(out, "
"+escapeHTML(run)+"
") + i = j + continue + } + // A lone `|...|` line isn't a table run (design requires + // 2+ consecutive lines) — fall through to normal handling. + } + + if strings.HasPrefix(trimmed, ">") { + j := i + var quoteLines []string + for j < len(lines) { + t := strings.TrimSpace(lines[j]) + if !strings.HasPrefix(t, ">") { + break + } + inner := strings.TrimPrefix(t, ">") + inner = strings.TrimPrefix(inner, " ") + quoteLines = append(quoteLines, processInline(inner)) + j++ + } + out = append(out, "
"+strings.Join(quoteLines, "\n")+"
") + i = j + continue + } + + if m := headingRe.FindStringSubmatch(line); m != nil { + out = append(out, ""+processInline(m[1])+"") + i++ + continue + } + + if hrRe.MatchString(trimmed) { + out = append(out, strings.Repeat("\u2014", 10)) + i++ + continue + } + + if m := taskItemRe.FindStringSubmatch(line); m != nil { + box := "\u2610" + if strings.EqualFold(m[2], "x") { + box = "\u2611" + } + out = append(out, m[1]+box+" "+processInline(m[3])) + i++ + continue + } + + if m := ulItemRe.FindStringSubmatch(line); m != nil { + out = append(out, m[1]+"\u2022 "+processInline(m[2])) + i++ + continue + } + + out = append(out, processInline(line)) + i++ + } + return strings.Join(out, "\n") +} + +// isTableLine reports whether line, trimmed, both starts and ends with +// "|" — the shape shared by a GFM table's header, separator, and data +// rows. +func isTableLine(line string) bool { + t := strings.TrimSpace(line) + return len(t) >= 2 && strings.HasPrefix(t, "|") && strings.HasSuffix(t, "|") +} + +// processInline applies the inline GFM->HTML mapping (bold, italic, +// strikethrough, links, images) to s and escapes "&<>" in every plain- +// text run it does not otherwise transform. Delimiters with no matching +// close are emitted as literal (escaped) text rather than an unclosed +// tag. Bold/italic/strikethrough content is reprocessed recursively so +// nesting (e.g. italic inside bold) round-trips correctly. +func processInline(s string) string { + var b strings.Builder + i := 0 + n := len(s) + for i < n { + c := s[i] + switch { + case c == '!' && i+1 < n && s[i+1] == '[': + if label, url, newPos, ok := matchLink(s, i+1); ok { + if label == "" { + b.WriteString(escapeHTML(url)) + } else { + b.WriteString(`
`) + b.WriteString(escapeHTML(label)) + b.WriteString(``) + } + i = newPos + continue + } + b.WriteByte('!') + i++ + + case c == '[': + if label, url, newPos, ok := matchLink(s, i); ok { + b.WriteString(``) + b.WriteString(escapeHTML(label)) + b.WriteString(``) + i = newPos + continue + } + b.WriteByte('[') + i++ + + case i+1 < n && (s[i:i+2] == "**" || s[i:i+2] == "__"): + marker := s[i : i+2] + if inner, newPos, ok := matchDelim(s, i, marker); ok { + b.WriteString("") + b.WriteString(processInline(inner)) + b.WriteString("") + i = newPos + continue + } + b.WriteString(marker) + i += 2 + + case i+1 < n && s[i:i+2] == "~~": + if inner, newPos, ok := matchDelim(s, i, "~~"); ok { + b.WriteString("") + b.WriteString(processInline(inner)) + b.WriteString("") + i = newPos + continue + } + b.WriteString("~~") + i += 2 + + case c == '*': + if inner, newPos, ok := matchDelim(s, i, "*"); ok { + b.WriteString("") + b.WriteString(processInline(inner)) + b.WriteString("") + i = newPos + continue + } + b.WriteByte('*') + i++ + + case c == '_': + // Underscore emphasis requires a non-word boundary on both + // sides (CommonMark's own rule) so "snake_case_name" is + // never mistaken for italic markup. + leftOK := i == 0 || !isWordByte(s[i-1]) + if leftOK { + if inner, newPos, ok := matchDelim(s, i, "_"); ok { + rightOK := newPos >= n || !isWordByte(s[newPos]) + if rightOK { + b.WriteString("") + b.WriteString(processInline(inner)) + b.WriteString("") + i = newPos + continue + } + } + } + b.WriteByte('_') + i++ + + default: + switch c { + case '&': + b.WriteString("&") + case '<': + b.WriteString("<") + case '>': + b.WriteString(">") + default: + b.WriteByte(c) + } + i++ + } + } + return b.String() +} + +// matchDelim looks for marker again, starting right after the opening +// occurrence at pos, and returns the text between them plus the index +// just past the closing occurrence. ok is false when no closing +// occurrence exists (caller then treats the opening marker as literal +// text rather than opening a tag it cannot close). +func matchDelim(s string, pos int, marker string) (inner string, newPos int, ok bool) { + start := pos + len(marker) + if start > len(s) { + return "", 0, false + } + idx := strings.Index(s[start:], marker) + if idx < 0 { + return "", 0, false + } + return s[start : start+idx], start + idx + len(marker), true +} + +// matchLink parses a `[label](url)` construct starting at pos (where +// s[pos] must be '['). ok is false for `[label](` with no closing paren +// (or no closing bracket at all), so the caller falls back to literal +// text instead of an unclosed tag. +func matchLink(s string, pos int) (label, url string, newPos int, ok bool) { + if pos >= len(s) || s[pos] != '[' { + return "", "", 0, false + } + closeBracket := strings.IndexByte(s[pos+1:], ']') + if closeBracket < 0 { + return "", "", 0, false + } + closeBracket += pos + 1 + if closeBracket+1 >= len(s) || s[closeBracket+1] != '(' { + return "", "", 0, false + } + closeParen := strings.IndexByte(s[closeBracket+2:], ')') + if closeParen < 0 { + return "", "", 0, false + } + closeParen += closeBracket + 2 + return s[pos+1 : closeBracket], s[closeBracket+2 : closeParen], closeParen + 1, true +} + +// isWordByte reports whether b is an ASCII letter, digit, or underscore +// — CommonMark's definition of a "word" character used for the +// underscore-emphasis boundary check. +func isWordByte(b byte) bool { + return b == '_' || (b >= 'a' && b <= 'z') || (b >= 'A' && b <= 'Z') || (b >= '0' && b <= '9') +} + +// escapeHTML escapes the three characters Telegram HTML requires +// escaped in plain-text runs: "&" -> "&", "<" -> "<", +// ">" -> ">". Iterates by byte (not rune) — safe because none of the +// three ASCII targets ever appears as a continuation byte of a +// multi-byte UTF-8 sequence, so other bytes pass through unmodified and +// unmodified sequences stay valid UTF-8. +func escapeHTML(s string) string { + var b strings.Builder + b.Grow(len(s)) + for i := 0; i < len(s); i++ { + switch s[i] { + case '&': + b.WriteString("&") + case '<': + b.WriteString("<") + case '>': + b.WriteString(">") + default: + b.WriteByte(s[i]) + } + } + return b.String() +} diff --git a/internal/bridge/markdown/telegram_test.go b/internal/bridge/markdown/telegram_test.go new file mode 100644 index 0000000000..8b16ccb607 --- /dev/null +++ b/internal/bridge/markdown/telegram_test.go @@ -0,0 +1,366 @@ +package markdown + +import ( + "strings" + "testing" +) + +func TestToTelegramHTML(t *testing.T) { + tests := []struct { + name string + in string + want string + }{ + { + name: "bold double star", + in: "**bold**", + want: "bold", + }, + { + name: "bold double underscore", + in: "__bold__", + want: "bold", + }, + { + name: "italic single star", + in: "*italic*", + want: "italic", + }, + { + name: "italic single underscore", + in: "_italic_", + want: "italic", + }, + { + name: "strikethrough", + in: "~~strike~~", + want: "strike", + }, + { + name: "inline code", + in: "`code`", + want: "code", + }, + { + name: "fenced code with lang", + in: "```go\nfunc main() {}\n```", + want: "
func main() {}
", + }, + { + name: "fenced code without info string", + in: "```\nplain\n```", + want: "
plain
", + }, + { + name: "link", + in: "[label](https://example.com)", + want: `
label`, + }, + { + name: "image with alt", + in: "![alt text](https://example.com/x.png)", + want: `alt text`, + }, + { + name: "image with empty alt", + in: "![](https://example.com/x.png)", + want: "https://example.com/x.png", + }, + { + name: "heading level 1", + in: "# Title", + want: "Title", + }, + { + name: "heading level 6", + in: "###### Small heading", + want: "Small heading", + }, + { + name: "unordered list dash", + in: "- item one", + want: "\u2022 item one", + }, + { + name: "unordered list star", + in: "* item one", + want: "\u2022 item one", + }, + { + name: "unordered list plus with indentation", + in: " + item one", + want: " \u2022 item one", + }, + { + name: "ordered list kept verbatim", + in: "1. item one", + want: "1. item one", + }, + { + name: "task list unchecked", + in: "- [ ] task one", + want: "\u2610 task one", + }, + { + name: "task list checked lowercase", + in: "- [x] task one", + want: "\u2611 task one", + }, + { + name: "task list checked uppercase", + in: "- [X] task one", + want: "\u2611 task one", + }, + { + name: "single blockquote line", + in: "> quoted text", + want: "
quoted text
", + }, + { + name: "merged consecutive blockquote lines", + in: "> line one\n> line two", + want: "
line one\nline two
", + }, + { + name: "pipe table wrapped in pre", + in: "| a | b |\n| - | - |\n| 1 | 2 |", + want: "
| a | b |\n| - | - |\n| 1 | 2 |
", + }, + { + name: "horizontal rule dashes", + in: "---", + want: strings.Repeat("\u2014", 10), + }, + { + name: "horizontal rule stars", + in: "***", + want: strings.Repeat("\u2014", 10), + }, + { + name: "horizontal rule underscores", + in: "___", + want: strings.Repeat("\u2014", 10), + }, + { + name: "plain prose escaped", + in: "plain prose & ", + want: "plain prose & <tag>", + }, + + // --- adversarial cases ------------------------------------- + { + name: "underscores and angle bracket inside code are not emphasised or unescaped", + in: "use `a_b_c` and `x < y`", + want: "use a_b_c and x < y", + }, + { + name: "snake_case identifier stays plain, no italic", + in: "snake_case_name stays plain", + want: "snake_case_name stays plain", + }, + { + name: "nested italic inside bold", + in: "**bold with _nested italic_ inside**", + want: "bold with nested italic inside", + }, + { + name: "ampersand and angle brackets escaped in prose", + in: "a < b && c > d", + want: "a < b && c > d", + }, + { + name: "unclosed bold left as literal text", + in: "**unclosed bold", + want: "**unclosed bold", + }, + { + name: "unclosed link left as literal text", + in: "[label](https://example.com", + want: "[label](https://example.com", + }, + { + name: "ampersand in href is escaped", + in: "[a](https://x?p=1&q=2)", + want: `a`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := ToTelegramHTML(tt.in) + if got != tt.want { + t.Errorf("ToTelegramHTML(%q) = %q, want %q", tt.in, got, tt.want) + } + }) + } +} + +func TestToTelegramHTML_UnclosedFenceStillProducesClosedPre(t *testing.T) { + in := "```go\nfunc main() {\n x := 1\n" + got := ToTelegramHTML(in) + if !strings.HasPrefix(got, `
`) {
+		t.Fatalf("got = %q, want it to start with an opened 
 tag", got)
+	}
+	if !strings.HasSuffix(got, "
") { + t.Fatalf("got = %q, want a closed
, even though the source fence never closed", got) + } + assertBalancedTags(t, got) +} + +func TestToTelegramHTML_NeverPanics(t *testing.T) { + inputs := []string{ + "", + "**", + "__", + "~~", + "[", + "[a](", + "![", + "`", + "```", + "```lang", + "_", + "*", + strings.Repeat("*", 500), + "\x00md1\x00 literal placeholder-looking text", + } + for _, in := range inputs { + func() { + defer func() { + if r := recover(); r != nil { + t.Errorf("ToTelegramHTML(%q) panicked: %v", in, r) + } + }() + _ = ToTelegramHTML(in) + }() + } +} + +func TestToTelegramHTML_RealisticReplyProducesBalancedOutput(t *testing.T) { + in := "# Build Report\n\n" + + "Status: **passed** with _one_ warning.\n\n" + + "- Ran `go test ./...`\n" + + "- Coverage: 87%\n" + + "- [ ] Follow-up: investigate flaky test\n\n" + + "```go\nfunc main() {\n\tfmt.Println(\"ok\")\n}\n```\n\n" + + "| Name | Status |\n| --- | --- |\n| build | ok |\n\n" + + "See [the pipeline](https://ci.example.com/run?id=42&retry=1) for details.\n\n" + + "> Reviewer note: looks good \n" + + got := ToTelegramHTML(in) + + if strings.Contains(got, "\x00") { + t.Fatalf("output still contains a raw placeholder token: %q", got) + } + assertBalancedTags(t, got) + assertNoStrayAngleBrackets(t, got) + + for _, want := range []string{ + "Build Report", + "passed", + "one", + "go test ./...", + "\u2022 Coverage: 87%", + "\u2610 Follow-up", + `
`,
+		"
| Name | Status |",
+		`the pipeline`,
+		"
Reviewer note: looks good <needs a second pass>
", + } { + if !strings.Contains(got, want) { + t.Errorf("output missing %q; got %q", want, got) + } + } +} + +// --- test helpers ----------------------------------------------------- + +// supportedTelegramTags is the fixed tag set Telegram's HTML parse mode +// recognizes. assertBalancedTags scans for exactly these. +var supportedTelegramTags = []string{ + "b", "strong", "i", "em", "u", "ins", "s", "strike", "del", + "a", "code", "pre", "blockquote", "tg-spoiler", +} + +// assertBalancedTags verifies every opening tag in the supported set has +// a matching closing tag, in a simple stack-based scan. It does not +// validate proper nesting order beyond LIFO matching, which is all +// well-formed HTML requires. +func assertBalancedTags(t *testing.T, html string) { + t.Helper() + var stack []string + i := 0 + for i < len(html) { + if html[i] != '<' { + i++ + continue + } + end := strings.IndexByte(html[i:], '>') + if end < 0 { + t.Fatalf("unterminated tag starting at byte %d in %q", i, html) + } + tag := html[i+1 : i+end] + i += end + 1 + + closing := strings.HasPrefix(tag, "/") + if closing { + tag = tag[1:] + } + // Strip attributes (e.g. `a href="..."`) down to the bare name. + if sp := strings.IndexByte(tag, ' '); sp >= 0 { + tag = tag[:sp] + } + if !isSupportedTag(tag) { + t.Fatalf("unsupported tag <%s> in output: %q", tag, html) + } + if !closing { + stack = append(stack, tag) + continue + } + if len(stack) == 0 || stack[len(stack)-1] != tag { + t.Fatalf("unbalanced tag : stack=%v in %q", tag, stack, html) + } + stack = stack[:len(stack)-1] + } + if len(stack) != 0 { + t.Fatalf("unclosed tags remain: %v in %q", stack, html) + } +} + +func isSupportedTag(tag string) bool { + for _, t := range supportedTelegramTags { + if t == tag { + return true + } + } + return false +} + +// assertNoStrayAngleBrackets verifies every '<' in html opens a +// recognized tag and every '>' closes one — i.e. no unescaped '<'/'>' +// leaked through from plain text. +func assertNoStrayAngleBrackets(t *testing.T, html string) { + t.Helper() + i := 0 + for i < len(html) { + switch html[i] { + case '<': + end := strings.IndexByte(html[i:], '>') + if end < 0 { + t.Fatalf("stray '<' with no closing '>' at byte %d in %q", i, html) + } + tag := strings.TrimPrefix(html[i+1:i+end], "/") + if sp := strings.IndexByte(tag, ' '); sp >= 0 { + tag = tag[:sp] + } + if !isSupportedTag(tag) { + t.Fatalf("stray '<' not part of a recognized tag at byte %d in %q", i, html) + } + i += end + 1 + case '>': + t.Fatalf("stray unescaped '>' at byte %d in %q", i, html) + default: + i++ + } + } +} diff --git a/internal/bridge/slack/adapter.go b/internal/bridge/slack/adapter.go index 96746041a9..53655194fb 100644 --- a/internal/bridge/slack/adapter.go +++ b/internal/bridge/slack/adapter.go @@ -19,6 +19,7 @@ import ( "github.com/slack-go/slack/socketmode" "github.com/opencode-ai/opencode/internal/bridge" + "github.com/opencode-ai/opencode/internal/bridge/markdown" "github.com/opencode-ai/opencode/internal/logging" ) @@ -30,8 +31,30 @@ const ( // MaxFileSize is Slack's files.uploadV2 limit (1 GiB). Larger // attachments are rejected pre-upload. MaxFileSize int64 = 1 * 1024 * 1024 * 1024 + + // MarkdownPayloadBudget is Slack's documented cumulative character + // cap across ALL `markdown`-type blocks in one chat.postMessage + // payload. Exceeding it produces a blocks_too_long-class API error. + MarkdownPayloadBudget = 12_000 + + // MarkdownBlockTarget is the target size (in runes) of each + // individual `markdown` block. Chosen as a sub-limit well under any + // theoretical per-block cap, keeping each block a reasonably sized, + // independently renderable chunk of markdown. + MarkdownBlockTarget = 3_000 + + // MaxBlocksPerMessage is Slack's absolute ceiling on blocks per + // message (shared across all block types, not just `markdown`). + // Enforced defensively here so a future change to the budget/target + // constants above can never silently exceed it. + MaxBlocksPerMessage = 50 ) +// markdownTruncationMarker is appended to the last emitted `markdown` +// block when outbound text exceeds MarkdownPayloadBudget. Italic prose so +// it is visually distinct from agent-authored content. +const markdownTruncationMarker = markdown.TruncationMarker + // Identity configures one Slack app identity. type Identity struct { ID string @@ -89,6 +112,15 @@ type Adapter struct { lastInboundAt atomic.Int64 lastFailureAt atomic.Int64 + // markdownBlocksUnsupported is a sticky, per-Adapter-instance latch: + // once a chat.postMessage call rejects `markdown` blocks with a + // block-related API error, this identity's workspace/app is treated + // as incapable of rendering them for the rest of the adapter's + // lifetime, and Send skips straight to the plain-text path. This is + // a workspace/app-level capability, not per-message state, so unlike + // lastError/lastFailureAt it is never reset. + markdownBlocksUnsupported atomic.Bool + // fileBaseURL overrides the URL prefix for file_private downloads. // Tests set this so the adapter fetches from their mock server. fileBaseURL atomic.Value // string @@ -888,21 +920,15 @@ func (a *Adapter) Send(ctx context.Context, out bridge.Outbound) bridge.SendResu return bridge.SendResult{Err: ErrInvalidPeerID} } + // Prepend the mention FIRST so it lands inside the rendered content + // (blocks or plain text) rather than being a separate, unstyled + // prefix — see the double-mention-prepend regression test. text := bridge.PrependMentionIfMissing(out.Mention, out.Text) - // Slack counts MaxTextLength in characters, not bytes. Slicing at a - // byte boundary that lands mid-codepoint produces invalid UTF-8 that - // the API can reject and renders as the replacement character. Cap - // by rune so the cut always lands on a codepoint boundary. - text = truncateRunes(text, MaxTextLength) // Text part first. resolved := "" if text != "" { - opts := []slackgo.MsgOption{slackgo.MsgOptionText(text, false)} - if peer.ThreadTS != "" { - opts = append(opts, slackgo.MsgOptionTS(peer.ThreadTS)) - } - _, ts, err := a.api.PostMessageContext(ctx, peer.ChannelID, opts...) + ts, err := a.sendText(ctx, peer, text) if err != nil { a.recordFailure(err) return bridge.SendResult{Err: fmt.Errorf("slack postMessage: %w", err)} @@ -946,6 +972,151 @@ func (a *Adapter) Send(ctx context.Context, out bridge.Outbound) bridge.SendResu return bridge.SendResult{Delivered: true, ResolvedPeer: resolved} } +// sendText posts the text part of an outbound message. It prefers +// Block Kit `markdown` blocks — real GFM rendering, chunked to Slack's +// 12,000-character cumulative budget across all `markdown` blocks in one +// payload — over the legacy mrkdwn `text` field. The top-level `text` +// field is still set alongside the blocks: Slack uses it as the +// notification/accessibility fallback (it is not shown in the message +// body when `blocks` is present), so it carries the rune-truncated plain +// text rather than the full markdown content. +// +// If blocks are rejected by a block-related Slack API error (see +// isBlockError), this identity's markdownBlocksUnsupported latch is set +// and the SAME send is retried once via the plain-text path before +// returning. Once latched, subsequent calls skip the blocks attempt +// entirely. A truncation performed by markdown.BuildBlockChunks is +// expected, user-visible (via the marker) behavior, not an error — it +// never triggers the plain-text fallback. +func (a *Adapter) sendText(ctx context.Context, peer Peer, text string) (string, error) { + // Slack counts MaxTextLength in characters, not bytes. Slicing at a + // byte boundary that lands mid-codepoint produces invalid UTF-8 that + // the API can reject and renders as the replacement character. Cap + // by rune so the cut always lands on a codepoint boundary. + fallback := truncateRunes(text, MaxTextLength) + + if a.markdownBlocksUnsupported.Load() { + return a.postPlainText(ctx, peer, fallback) + } + + chunks, _ := markdown.BuildBlockChunks(text, MarkdownPayloadBudget, MarkdownBlockTarget, markdownTruncationMarker) + if len(chunks) == 0 { + return a.postPlainText(ctx, peer, fallback) + } + if len(chunks) > MaxBlocksPerMessage { + chunks = chunks[:MaxBlocksPerMessage] + } + blocks := make([]slackgo.Block, 0, len(chunks)) + for _, c := range chunks { + blocks = append(blocks, slackgo.NewMarkdownBlock("", c)) + } + + opts := []slackgo.MsgOption{ + slackgo.MsgOptionBlocks(blocks...), + slackgo.MsgOptionText(fallback, false), + } + if peer.ThreadTS != "" { + opts = append(opts, slackgo.MsgOptionTS(peer.ThreadTS)) + } + _, ts, err := a.api.PostMessageContext(ctx, peer.ChannelID, opts...) + if err == nil { + return ts, nil + } + if !isBlockError(err) { + return "", err + } + + // Only an unambiguous block-capability rejection latches: an + // ambiguous error (e.g. invalid_arguments, which Slack also returns + // for unrelated reasons like a bad channel or timestamp) self-heals + // on its own — the next Send simply tries blocks again, costing at + // most one extra API call if it really was block-related. Latching + // on an ambiguous error would permanently downgrade formatting for + // this identity because of a failure that may have nothing to do + // with block support. + if isBlockCapabilityError(err) && a.markdownBlocksUnsupported.CompareAndSwap(false, true) { + logging.Warn("bridge: slack markdown blocks rejected, falling back to plain text", + "identity", a.id.ID, "err", err) + } + return a.postPlainText(ctx, peer, fallback) +} + +// postPlainText posts text via the legacy mrkdwn `text` field only — no +// blocks. Used both for the sticky post-latch path and the one-time +// retry after a block-related API rejection. +func (a *Adapter) postPlainText(ctx context.Context, peer Peer, text string) (string, error) { + opts := []slackgo.MsgOption{slackgo.MsgOptionText(text, false)} + if peer.ThreadTS != "" { + opts = append(opts, slackgo.MsgOptionTS(peer.ThreadTS)) + } + _, ts, err := a.api.PostMessageContext(ctx, peer.ChannelID, opts...) + return ts, err +} + +// isBlockError reports whether err looks like a Slack API rejection of +// the message's Block Kit blocks, as opposed to an unrelated failure +// (e.g. channel_not_found). slack-go surfaces API errors as +// SlackErrorResponse, whose Error() is just the bare API error code, so +// case-insensitive substring matching against the known block-related +// codes is sufficient. Matching is deliberately broad — it includes +// invalid_arguments, which Slack also returns for many non-block +// reasons (bad channel, bad timestamp, ...) — because this predicate +// only gates the one-time plain-text RETRY for the current send, not +// the sticky latch: a false positive here just costs one extra +// plain-text send this time, which is always safe; a false negative +// just retries the blocks path again on the next Send. See +// isBlockCapabilityError for the narrower predicate that gates the +// sticky latch. +func isBlockError(err error) bool { + if err == nil { + return false + } + s := strings.ToLower(err.Error()) + for _, needle := range []string{ + "invalid_blocks", + "invalid_block", + "blocks_too_long", + "invalid_arguments", + "msg_blocks_too_long", + "invalid_block_id", + } { + if strings.Contains(s, needle) { + return true + } + } + return false +} + +// isBlockCapabilityError reports whether err unambiguously indicates +// that this Slack workspace/app cannot use Block Kit `markdown` blocks +// at all, as opposed to a merely block-shaped but potentially unrelated +// error. Deliberately narrower than isBlockError: it excludes +// invalid_arguments, which Slack also returns for reasons that have +// nothing to do with block support (bad channel, bad ts, ...) — setting +// the sticky markdownBlocksUnsupported latch on such an ambiguous error +// would permanently downgrade formatting for this identity because of +// an unrelated failure. Only an error code that Slack documents as +// specifically about block content/shape justifies paying that +// permanent cost. +func isBlockCapabilityError(err error) bool { + if err == nil { + return false + } + s := strings.ToLower(err.Error()) + for _, needle := range []string{ + "invalid_blocks", + "invalid_block", + "blocks_too_long", + "msg_blocks_too_long", + "invalid_block_id", + } { + if strings.Contains(s, needle) { + return true + } + } + return false +} + func (a *Adapter) recordFailure(err error) { if err == nil { return diff --git a/internal/bridge/slack/adapter_markdown_test.go b/internal/bridge/slack/adapter_markdown_test.go new file mode 100644 index 0000000000..3bdaddc4d5 --- /dev/null +++ b/internal/bridge/slack/adapter_markdown_test.go @@ -0,0 +1,343 @@ +package slack + +import ( + "context" + "encoding/json" + "strings" + "testing" + "unicode/utf8" + + "github.com/opencode-ai/opencode/internal/bridge" + "github.com/opencode-ai/opencode/internal/bridge/markdown" +) + +// decodedBlock mirrors the JSON shape of a slackgo.MarkdownBlock, used to +// decode the "blocks" form field the mock server captures. +type decodedBlock struct { + Type string `json:"type"` + Text string `json:"text"` +} + +func decodeMarkdownBlocks(t *testing.T, raw string) []decodedBlock { + t.Helper() + if raw == "" { + return nil + } + var blocks []decodedBlock + if err := json.Unmarshal([]byte(raw), &blocks); err != nil { + t.Fatalf("decode blocks %q: %v", raw, err) + } + return blocks +} + +func TestSendGFMProseProducesMarkdownBlocks(t *testing.T) { + t.Parallel() + a, mock, _ := newAdapter(t, Identity{ID: "default", BotToken: "xoxb-test", AppToken: "xapp-test"}) + + gfm := "## Heading\n\n**bold** text and a [link](https://example.com)\n\n- item one\n- item two\n\n| a | b |\n| - | - |\n| 1 | 2 |" + r := a.Send(context.Background(), bridge.Outbound{ + Peer: bridge.PeerRef{Channel: "slack", Identity: "default", PeerID: "D123"}, + Text: gfm, + }) + if !r.Delivered { + t.Fatalf("send: %v", r.Err) + } + + posts := mock.Posts() + if len(posts) != 1 { + t.Fatalf("posts = %d, want 1", len(posts)) + } + blocks := decodeMarkdownBlocks(t, posts[0].Blocks) + if len(blocks) != 1 { + t.Fatalf("blocks = %d, want 1 (short GFM text fits in one chunk)", len(blocks)) + } + if blocks[0].Type != "markdown" { + t.Errorf("block type = %q, want markdown", blocks[0].Type) + } + // GFM syntax must be preserved verbatim — we do NOT rewrite it, we + // let Slack's markdown-block parser render it. + for _, want := range []string{"**bold**", "## Heading", "- item one", "[link](https://example.com)", "| a | b |"} { + if !strings.Contains(blocks[0].Text, want) { + t.Errorf("block text missing %q; got %q", want, blocks[0].Text) + } + } + + // The top-level text fallback must still be set (notification / + // accessibility preview). + if posts[0].Text == "" { + t.Errorf("top-level text fallback is empty") + } +} + +func TestSendNormalizesLegacyLinkSyntaxInBlocks(t *testing.T) { + t.Parallel() + a, mock, _ := newAdapter(t, Identity{ID: "default", BotToken: "xoxb-test", AppToken: "xapp-test"}) + + r := a.Send(context.Background(), bridge.Outbound{ + Peer: bridge.PeerRef{Channel: "slack", Identity: "default", PeerID: "D123"}, + Text: "See for details.", + }) + if !r.Delivered { + t.Fatalf("send: %v", r.Err) + } + + posts := mock.Posts() + blocks := decodeMarkdownBlocks(t, posts[0].Blocks) + if len(blocks) != 1 { + t.Fatalf("blocks = %d, want 1", len(blocks)) + } + if !strings.Contains(blocks[0].Text, "[label](https://example.com)") { + t.Errorf("block text = %q, want normalized link", blocks[0].Text) + } + if strings.Contains(blocks[0].Text, "") { + t.Errorf("block text still contains raw mrkdwn link syntax: %q", blocks[0].Text) + } +} + +func TestSendMentionAppearsExactlyOnceInBlocks(t *testing.T) { + t.Parallel() + a, mock, _ := newAdapter(t, Identity{ID: "default", BotToken: "xoxb-test", AppToken: "xapp-test"}) + + r := a.Send(context.Background(), bridge.Outbound{ + Peer: bridge.PeerRef{Channel: "slack", Identity: "default", PeerID: "D123"}, + Mention: "<@U999>", + Text: "your build finished", + }) + if !r.Delivered { + t.Fatalf("send: %v", r.Err) + } + + posts := mock.Posts() + blocks := decodeMarkdownBlocks(t, posts[0].Blocks) + if len(blocks) != 1 { + t.Fatalf("blocks = %d, want 1", len(blocks)) + } + if n := strings.Count(blocks[0].Text, "<@U999>"); n != 1 { + t.Errorf("mention appears %d times in block text, want 1: %q", n, blocks[0].Text) + } + // Mention must land INSIDE the rendered content, not as a separate + // unstyled prefix outside of it. + if !strings.HasPrefix(blocks[0].Text, "<@U999>") { + t.Errorf("block text = %q, want mention-prefixed content", blocks[0].Text) + } +} + +func TestSendOversizedTextChunksWithinBudgetAndMarker(t *testing.T) { + t.Parallel() + a, mock, _ := newAdapter(t, Identity{ID: "default", BotToken: "xoxb-test", AppToken: "xapp-test"}) + + text := strings.Repeat("word ", 3000) // ~15,000 runes, over the 12,000 budget + r := a.Send(context.Background(), bridge.Outbound{ + Peer: bridge.PeerRef{Channel: "slack", Identity: "default", PeerID: "D123"}, + Text: text, + }) + if !r.Delivered { + t.Fatalf("send: %v", r.Err) + } + + posts := mock.Posts() + blocks := decodeMarkdownBlocks(t, posts[0].Blocks) + if len(blocks) == 0 || len(blocks) > 4 { + t.Fatalf("blocks = %d, want 1..4", len(blocks)) + } + sum := 0 + for _, b := range blocks { + sum += utf8.RuneCountInString(b.Text) + } + if sum > MarkdownPayloadBudget { + t.Errorf("sum(runeLen(blocks)) = %d, want <= %d", sum, MarkdownPayloadBudget) + } + last := blocks[len(blocks)-1].Text + if !strings.HasSuffix(last, markdown.TruncationMarker) { + t.Errorf("last block does not end with truncation marker: %q", tailRunes(last, 40)) + } +} + +func TestSendBlockErrorFallsBackAndLatches(t *testing.T) { + t.Parallel() + a, mock, _ := newAdapter(t, Identity{ID: "default", BotToken: "xoxb-test", AppToken: "xapp-test"}) + mock.postMessageErrors = []string{"invalid_blocks"} + + r := a.Send(context.Background(), bridge.Outbound{ + Peer: bridge.PeerRef{Channel: "slack", Identity: "default", PeerID: "D123"}, + Text: "hello **world**", + }) + if !r.Delivered { + t.Fatalf("send: %v (want delivered via plain-text fallback)", r.Err) + } + + posts := mock.Posts() + if len(posts) != 2 { + t.Fatalf("posts = %d, want 2 (blocks attempt + plain-text retry)", len(posts)) + } + if posts[0].Blocks == "" { + t.Errorf("first attempt carried no blocks; expected a blocks attempt first") + } + if posts[1].Blocks != "" { + t.Errorf("retry attempt carried blocks; want plain text only") + } + if posts[1].Text != "hello **world**" { + t.Errorf("retry text = %q, want original text", posts[1].Text) + } + + // Second, unrelated Send on the same Adapter: the latch must skip + // the blocks attempt entirely — exactly one more plain-text post, + // no blocks. + r2 := a.Send(context.Background(), bridge.Outbound{ + Peer: bridge.PeerRef{Channel: "slack", Identity: "default", PeerID: "D123"}, + Text: "second message", + }) + if !r2.Delivered { + t.Fatalf("second send: %v", r2.Err) + } + posts = mock.Posts() + if len(posts) != 3 { + t.Fatalf("posts = %d, want 3 (latched: no blocks attempt on second Send)", len(posts)) + } + if posts[2].Blocks != "" { + t.Errorf("post after latch carried blocks; want plain text only") + } + if posts[2].Text != "second message" { + t.Errorf("post after latch text = %q, want %q", posts[2].Text, "second message") + } +} + +func TestSendAmbiguousBlockErrorRetriesWithoutLatching(t *testing.T) { + t.Parallel() + a, mock, _ := newAdapter(t, Identity{ID: "default", BotToken: "xoxb-test", AppToken: "xapp-test"}) + mock.postMessageErrors = []string{"invalid_arguments"} + + r := a.Send(context.Background(), bridge.Outbound{ + Peer: bridge.PeerRef{Channel: "slack", Identity: "default", PeerID: "D123"}, + Text: "hello **world**", + }) + if !r.Delivered { + t.Fatalf("send: %v (want delivered via plain-text fallback)", r.Err) + } + + posts := mock.Posts() + if len(posts) != 2 { + t.Fatalf("posts = %d, want 2 (blocks attempt + plain-text retry)", len(posts)) + } + if posts[0].Blocks == "" { + t.Errorf("first attempt carried no blocks; expected a blocks attempt first") + } + if posts[1].Blocks != "" { + t.Errorf("retry attempt carried blocks; want plain text only") + } + + // invalid_arguments is ambiguous (Slack also returns it for + // unrelated reasons) so it must NOT set the sticky latch: the next + // Send should still attempt blocks. + r2 := a.Send(context.Background(), bridge.Outbound{ + Peer: bridge.PeerRef{Channel: "slack", Identity: "default", PeerID: "D123"}, + Text: "second message", + }) + if !r2.Delivered { + t.Fatalf("second send: %v", r2.Err) + } + posts = mock.Posts() + if len(posts) != 3 { + t.Fatalf("posts = %d, want 3", len(posts)) + } + if posts[2].Blocks == "" { + t.Errorf("post after ambiguous error carried no blocks; want blocks attempted again (latch NOT set)") + } +} + +func TestSendUnrelatedErrorDoesNotRetry(t *testing.T) { + t.Parallel() + a, mock, _ := newAdapter(t, Identity{ID: "default", BotToken: "xoxb-test", AppToken: "xapp-test"}) + mock.postMessageErrors = []string{"channel_not_found"} + + r := a.Send(context.Background(), bridge.Outbound{ + Peer: bridge.PeerRef{Channel: "slack", Identity: "default", PeerID: "D123"}, + Text: "hello", + }) + if r.Delivered { + t.Fatalf("delivered = true, want false (unrelated error)") + } + if r.Err == nil || !strings.Contains(r.Err.Error(), "channel_not_found") { + t.Errorf("err = %v, want channel_not_found", r.Err) + } + + posts := mock.Posts() + if len(posts) != 1 { + t.Fatalf("posts = %d, want exactly 1 (no retry for an unrelated error)", len(posts)) + } +} + +func TestSendEmptyTextWithAttachmentsSkipsTextPost(t *testing.T) { + t.Parallel() + a, mock, _ := newAdapter(t, Identity{ID: "default", BotToken: "xoxb-test", AppToken: "xapp-test"}) + + r := a.Send(context.Background(), bridge.Outbound{ + Peer: bridge.PeerRef{Channel: "slack", Identity: "default", PeerID: "D123"}, + Attachments: []bridge.Attachment{ + {FileName: "f.txt", Content: []byte("data")}, + }, + }) + if !r.Delivered { + t.Fatalf("send: %v", r.Err) + } + if posts := mock.Posts(); len(posts) != 0 { + t.Errorf("posts = %d, want 0 (no text part)", len(posts)) + } + if uploads := mock.Uploads(); len(uploads) != 1 { + t.Errorf("uploads = %d, want 1", len(uploads)) + } +} + +func TestSendThreadingPreservedWithBlocks(t *testing.T) { + t.Parallel() + a, mock, _ := newAdapter(t, Identity{ID: "default", BotToken: "xoxb-test", AppToken: "xapp-test"}) + + // Thread peer: MsgOptionTS must still be applied. + r1 := a.Send(context.Background(), bridge.Outbound{ + Peer: bridge.PeerRef{Channel: "slack", Identity: "default", PeerID: "C123|1700000000.000100"}, + Text: "**hi** thread", + }) + if !r1.Delivered { + t.Fatalf("thread send: %v", r1.Err) + } + if r1.ResolvedPeer != "" { + t.Errorf("ResolvedPeer = %q, want empty for an already-composite peer", r1.ResolvedPeer) + } + + // Channel-only peer: still returns the rewritten ResolvedPeer. + r2 := a.Send(context.Background(), bridge.Outbound{ + Peer: bridge.PeerRef{Channel: "slack", Identity: "default", PeerID: "C0DEF456"}, + Text: "**hi** channel", + }) + if !r2.Delivered { + t.Fatalf("channel send: %v", r2.Err) + } + if r2.ResolvedPeer != "C0DEF456|1700000123.000200" { + t.Errorf("ResolvedPeer = %q", r2.ResolvedPeer) + } + + posts := mock.Posts() + if len(posts) != 2 { + t.Fatalf("posts = %d, want 2", len(posts)) + } + if posts[0].ThreadTS != "1700000000.000100" { + t.Errorf("post 0 ThreadTS = %q, want thread ts", posts[0].ThreadTS) + } + if posts[1].ThreadTS != "" { + t.Errorf("post 1 ThreadTS = %q, want empty (channel-only peer)", posts[1].ThreadTS) + } + // Both still went through the blocks path. + for i, p := range posts { + if p.Blocks == "" { + t.Errorf("post %d carried no blocks", i) + } + } +} + +func tailRunes(s string, n int) string { + r := []rune(s) + if len(r) <= n { + return s + } + return string(r[len(r)-n:]) +} diff --git a/internal/bridge/slack/adapter_test.go b/internal/bridge/slack/adapter_test.go index e5d6cdcc9b..92ba40d527 100644 --- a/internal/bridge/slack/adapter_test.go +++ b/internal/bridge/slack/adapter_test.go @@ -32,12 +32,23 @@ type mockSlackServer struct { uploads []uploadCall opens []string files map[string]string // file ID → body + + // postMessageErrors, when non-empty, is consulted by call index (0- + // based) to decide whether the Nth chat.postMessage call should + // fail with a Slack API error instead of succeeding. Used by the + // markdown-block fallback/latch tests to simulate a workspace that + // rejects `markdown` blocks (or an unrelated failure) on the first + // attempt. + postMessageErrors []string } type postCall struct { Channel string Text string ThreadTS string + // Blocks is the raw JSON array from the "blocks" form field, empty + // when the call carried no Block Kit blocks (plain-text path). + Blocks string } type uploadCall struct { @@ -124,10 +135,20 @@ func (m *mockSlackServer) handleAPI(w http.ResponseWriter, r *http.Request) { Channel: r.FormValue("channel"), Text: r.FormValue("text"), ThreadTS: r.FormValue("thread_ts"), + Blocks: r.FormValue("blocks"), } m.mu.Lock() + idx := len(m.posts) m.posts = append(m.posts, call) + var injectedErr string + if idx < len(m.postMessageErrors) { + injectedErr = m.postMessageErrors[idx] + } m.mu.Unlock() + if injectedErr != "" { + m.respond(w, map[string]any{"ok": false, "error": injectedErr}) + return + } m.respond(w, map[string]any{ "ok": true, "channel": call.Channel, diff --git a/internal/bridge/telegram/adapter.go b/internal/bridge/telegram/adapter.go index b49b96c9e0..187a8c8fef 100644 --- a/internal/bridge/telegram/adapter.go +++ b/internal/bridge/telegram/adapter.go @@ -21,13 +21,14 @@ import ( "github.com/go-telegram/bot/models" "github.com/opencode-ai/opencode/internal/bridge" + "github.com/opencode-ai/opencode/internal/bridge/markdown" "github.com/opencode-ai/opencode/internal/logging" ) // Constants matching the TS bridge. const ( - // MaxTextLength is Telegram's per-message text cap. Longer messages - // are chunked at Send time. + // MaxTextLength is Telegram's per-message text cap, counted AFTER + // entity parsing, not on the raw source text we send. MaxTextLength = 4096 // MaxCaptionLength is Telegram's per-attachment caption cap. @@ -36,6 +37,16 @@ const ( // MaxFileSize is Telegram's bot-API upload limit (50 MiB). Larger // attachments are rejected pre-upload with an error. MaxFileSize int64 = 50 * 1024 * 1024 + + // MarkdownChunkLimit is the conservative SOURCE-markdown chunk size + // (in runes) used to split outbound text before HTML conversion. + // Telegram's MaxTextLength cap applies to the PARSED, visible text — + // but HTML tag overhead (, , etc.) and "&<>" + // escaping only ever grow a chunk's raw byte/rune count relative to + // its converted form. Chunking the source at a limit well under + // MaxTextLength keeps both the raw markdown chunk and its converted + // HTML comfortably inside the 4,096 cap. + MarkdownChunkLimit = 3_500 ) // AccessMode is the per-identity access policy for a Telegram bot. @@ -844,8 +855,9 @@ func (a *Adapter) replyText(ctx context.Context, chatID, text string) { // Send implements bridge.Adapter. The platform's per-part shapes // (sendMessage / sendPhoto / sendAudio / sendDocument) are chosen from -// MIME-type sniffing on attachments; text-only outbound chunks at -// MaxTextLength. +// MIME-type sniffing on attachments; text is chunked (at +// MarkdownChunkLimit, on the SOURCE markdown) and each chunk is +// converted to Telegram HTML independently before sending. func (a *Adapter) Send(ctx context.Context, out bridge.Outbound) bridge.SendResult { chatID, err := ParsePeerID(out.Peer.PeerID) if err != nil { @@ -860,14 +872,18 @@ func (a *Adapter) Send(ctx context.Context, out bridge.Outbound) bridge.SendResu // attachment, and any leftover text streams as a final sendMessage // in chunks. For simplicity we send text first, then each attachment // with no caption — closer to the multi-platform fan-out semantics. - for _, chunk := range chunkText(text, MaxTextLength) { + // + // maxChunks=0 (unlimited): Telegram has no per-message payload + // budget analogous to Slack's 12,000-char block cap — a long agent + // reply is allowed to become several messages, same as today's + // chunkText behavior. TruncationMarker is passed for interface + // symmetry with the Slack chunker but never actually applied here. + chunks, _ := markdown.Split(text, MarkdownChunkLimit, 0, markdown.TruncationMarker) + for _, chunk := range chunks { if chunk == "" { continue } - if _, err := a.bot.SendMessage(ctx, &tgbot.SendMessageParams{ - ChatID: chatID, - Text: chunk, - }); err != nil { + if err := a.sendTextChunk(ctx, chatID, chunk); err != nil { a.recordFailure(err) return bridge.SendResult{Err: fmt.Errorf("telegram sendMessage: %w", err)} } @@ -883,6 +899,63 @@ func (a *Adapter) Send(ctx context.Context, out bridge.Outbound) bridge.SendResu return bridge.SendResult{Delivered: true} } +// sendTextChunk sends one source-markdown chunk converted to Telegram +// HTML. If the send fails with what looks like an entity/parse error +// (see isParseError), it retries THIS chunk once, unchanged, with no +// ParseMode — a plain-text degrade. There is deliberately no sticky +// latch here (contrast with the Slack blocks latch): an HTML parse +// failure is specific to this chunk's content (an edge case in the +// GFM->HTML conversion, or genuinely malformed markup), not a platform +// capability the bot lacks, so the next chunk/message attempts +// ParseModeHTML fresh. +func (a *Adapter) sendTextChunk(ctx context.Context, chatID int64, chunk string) error { + html := markdown.ToTelegramHTML(chunk) + _, err := a.bot.SendMessage(ctx, &tgbot.SendMessageParams{ + ChatID: chatID, + Text: html, + ParseMode: models.ParseModeHTML, + }) + if err == nil { + return nil + } + if !isParseError(err) { + return err + } + _, retryErr := a.bot.SendMessage(ctx, &tgbot.SendMessageParams{ + ChatID: chatID, + Text: chunk, + }) + return retryErr +} + +// isParseError reports whether err looks like Telegram rejecting the +// message's HTML entities, as opposed to an unrelated failure (e.g. +// "chat not found"). Matching is deliberately broad — case-insensitive +// substring checks against known entity-parsing error phrasings — because +// the only consequence of a false positive is retrying with plain, +// unformatted text: always safe, never a lost message. A false negative +// just surfaces the original error, same as today's behavior. +func isParseError(err error) bool { + if err == nil { + return false + } + s := strings.ToLower(err.Error()) + for _, needle := range []string{ + "can't parse entities", + "cant parse entities", + "unsupported start tag", + "unclosed start tag", + "wrong end tag", + "entity", + "bad request: can't parse", + } { + if strings.Contains(s, needle) { + return true + } + } + return false +} + // sendAttachment picks the right Telegram method based on the // attachment's MIME prefix or filename extension. func (a *Adapter) sendAttachment(ctx context.Context, chatID int64, att bridge.Attachment) error { @@ -923,51 +996,8 @@ func (a *Adapter) sendAttachment(ctx context.Context, chatID int64, att bridge.A } } -// chunkText splits text into chunks of at most max UTF-8 codepoints -// (NOT bytes). Telegram's MaxTextLength is counted in characters, not -// bytes — and slicing a UTF-8 string at a byte boundary that lands -// mid-codepoint produces invalid UTF-8 that the Telegram API rejects -// outright. We walk the string by rune and split at codepoint -// boundaries. -func chunkText(text string, max int) []string { - if text == "" { - return nil - } - if max <= 0 { - return []string{text} - } - // Fast path — count runes; if the whole text fits, no split needed. - if utf8RuneCount(text) <= max { - return []string{text} - } - var out []string - var buf strings.Builder - count := 0 - for _, r := range text { - buf.WriteRune(r) - count++ - if count >= max { - out = append(out, buf.String()) - buf.Reset() - count = 0 - } - } - if buf.Len() > 0 { - out = append(out, buf.String()) - } - return out -} - -// utf8RuneCount returns the number of runes (codepoints) in s without -// allocating. Avoids the standard utf8.RuneCountInString import only to -// keep the diff narrowly scoped here. -func utf8RuneCount(s string) int { - n := 0 - for range s { - n++ - } - return n -} +// Note: the old rune-only chunkText helper (no markdown awareness) was +// removed in favor of markdown.Split, whose only caller was Send. // downloadMediaAttachments fetches every media file referenced by the // incoming message and returns the resulting bridge.Attachment values. diff --git a/internal/bridge/telegram/adapter_markdown_test.go b/internal/bridge/telegram/adapter_markdown_test.go new file mode 100644 index 0000000000..535589825f --- /dev/null +++ b/internal/bridge/telegram/adapter_markdown_test.go @@ -0,0 +1,276 @@ +package telegram + +import ( + "context" + "strings" + "testing" + + "github.com/go-telegram/bot/models" + + "github.com/opencode-ai/opencode/internal/bridge" +) + +// TestSendGFMProseProducesHTMLParseMode mirrors the Slack-side +// TestSendGFMProseProducesMarkdownBlocks: a GFM outbound message is sent +// with ParseMode HTML and the converted text carries the expected tags. +func TestSendGFMProseProducesHTMLParseMode(t *testing.T) { + t.Parallel() + a, mock, _ := newAdapter(t, Identity{ID: "default", Token: "tg-token"}) + + gfm := "# Heading\n\n**bold** and `inline code`\n\n```go\nfunc main() {}\n```" + res := a.Send(context.Background(), bridge.Outbound{ + Peer: bridge.PeerRef{Channel: "telegram", Identity: "default", PeerID: "12345"}, + Text: gfm, + }) + if !res.Delivered { + t.Fatalf("Send err: %v", res.Err) + } + + mock.mu.Lock() + defer mock.mu.Unlock() + if len(mock.sendMsg) != 1 { + t.Fatalf("sendMessage calls = %d, want 1", len(mock.sendMsg)) + } + call := mock.sendMsg[0] + if call.ParseMode != string(models.ParseModeHTML) { + t.Errorf("ParseMode = %q, want %q", call.ParseMode, models.ParseModeHTML) + } + for _, want := range []string{"Heading", "bold", "inline code", `
`} {
+		if !strings.Contains(call.Text, want) {
+			t.Errorf("sent text missing %q; got %q", want, call.Text)
+		}
+	}
+}
+
+// TestSendMultiChunkPreservesContentAndParseMode verifies a message
+// longer than MarkdownChunkLimit is split into multiple sendMessage
+// calls, each independently converted with ParseMode HTML, and that no
+// word is lost across the split.
+func TestSendMultiChunkPreservesContentAndParseMode(t *testing.T) {
+	t.Parallel()
+	a, mock, _ := newAdapter(t, Identity{ID: "default", Token: "tg-token"})
+
+	var sb strings.Builder
+	for i := 0; i < 300; i++ {
+		sb.WriteString("word")
+		sb.WriteString("_")
+		sb.WriteString("0123456789 ")
+		if (i+1)%6 == 0 {
+			sb.WriteString("\n\n")
+		}
+	}
+	text := sb.String()
+	if len(text) <= MarkdownChunkLimit {
+		t.Fatalf("fixture too short (%d runes) to force chunking", len(text))
+	}
+
+	res := a.Send(context.Background(), bridge.Outbound{
+		Peer: bridge.PeerRef{Channel: "telegram", Identity: "default", PeerID: "12345"},
+		Text: text,
+	})
+	if !res.Delivered {
+		t.Fatalf("Send err: %v", res.Err)
+	}
+
+	mock.mu.Lock()
+	calls := append([]sendCall(nil), mock.sendMsg...)
+	mock.mu.Unlock()
+	if len(calls) < 2 {
+		t.Fatalf("sendMessage calls = %d, want >= 2 (chunking expected)", len(calls))
+	}
+	for i, c := range calls {
+		if c.ParseMode != string(models.ParseModeHTML) {
+			t.Errorf("call %d ParseMode = %q, want %q", i, c.ParseMode, models.ParseModeHTML)
+		}
+		if len([]rune(c.Text)) > MaxTextLength {
+			t.Errorf("call %d text rune length %d exceeds MaxTextLength %d", i, len([]rune(c.Text)), MaxTextLength)
+		}
+	}
+
+	// No content loss: every "wordN_digits" token from the source
+	// appears in the concatenation of all sent (HTML-escaped) chunks.
+	wantWords := strings.Fields(text)
+	var gotConcat strings.Builder
+	for _, c := range calls {
+		gotConcat.WriteString(c.Text)
+		gotConcat.WriteString(" ")
+	}
+	got := gotConcat.String()
+	for _, w := range wantWords {
+		if !strings.Contains(got, w) {
+			t.Errorf("word %q missing from concatenated sent output", w)
+		}
+	}
+}
+
+// TestSendParseErrorRetriesChunkWithoutParseMode verifies that a
+// can't-parse-entities-class error on the HTML attempt triggers exactly
+// one retry for that chunk with no ParseMode and the original markdown
+// text, and that Send still reports success.
+func TestSendParseErrorRetriesChunkWithoutParseMode(t *testing.T) {
+	t.Parallel()
+	a, mock, _ := newAdapter(t, Identity{ID: "default", Token: "tg-token"})
+	mock.sendMessageErrors = []string{"Bad Request: can't parse entities: Unsupported start tag \"x\" at byte offset 4"}
+
+	res := a.Send(context.Background(), bridge.Outbound{
+		Peer: bridge.PeerRef{Channel: "telegram", Identity: "default", PeerID: "12345"},
+		Text: "hello **world**",
+	})
+	if !res.Delivered {
+		t.Fatalf("Send err: %v (want delivered via plain-text retry)", res.Err)
+	}
+
+	mock.mu.Lock()
+	calls := append([]sendCall(nil), mock.sendMsg...)
+	mock.mu.Unlock()
+	if len(calls) != 2 {
+		t.Fatalf("sendMessage calls = %d, want 2 (HTML attempt + plain-text retry)", len(calls))
+	}
+	if calls[0].ParseMode != string(models.ParseModeHTML) {
+		t.Errorf("first call ParseMode = %q, want HTML", calls[0].ParseMode)
+	}
+	if calls[1].ParseMode != "" {
+		t.Errorf("retry call ParseMode = %q, want empty (no ParseMode)", calls[1].ParseMode)
+	}
+	if calls[1].Text != "hello **world**" {
+		t.Errorf("retry text = %q, want original markdown text", calls[1].Text)
+	}
+}
+
+// TestSendUnrelatedErrorDoesNotRetryAndSurfaces verifies an unrelated
+// send failure (not entity/parse-shaped) is not retried and IS
+// surfaced as the Send result's error.
+func TestSendUnrelatedErrorDoesNotRetryAndSurfaces(t *testing.T) {
+	t.Parallel()
+	a, mock, _ := newAdapter(t, Identity{ID: "default", Token: "tg-token"})
+	mock.sendMessageErrors = []string{"Bad Request: chat not found"}
+
+	res := a.Send(context.Background(), bridge.Outbound{
+		Peer: bridge.PeerRef{Channel: "telegram", Identity: "default", PeerID: "12345"},
+		Text: "hello",
+	})
+	if res.Delivered {
+		t.Fatalf("Delivered = true, want false (unrelated error)")
+	}
+	if res.Err == nil || !strings.Contains(res.Err.Error(), "chat not found") {
+		t.Errorf("err = %v, want it to mention chat not found", res.Err)
+	}
+
+	mock.mu.Lock()
+	got := len(mock.sendMsg)
+	mock.mu.Unlock()
+	if got != 1 {
+		t.Fatalf("sendMessage calls = %d, want exactly 1 (no retry for an unrelated error)", got)
+	}
+}
+
+// TestSendParseErrorDoesNotLatchAcrossMessages verifies there is no
+// sticky latch: after a parse-error retry on one Send, the NEXT Send
+// still attempts ParseModeHTML fresh.
+func TestSendParseErrorDoesNotLatchAcrossMessages(t *testing.T) {
+	t.Parallel()
+	a, mock, _ := newAdapter(t, Identity{ID: "default", Token: "tg-token"})
+	mock.sendMessageErrors = []string{"Bad Request: can't parse entities: bad tag"}
+
+	res1 := a.Send(context.Background(), bridge.Outbound{
+		Peer: bridge.PeerRef{Channel: "telegram", Identity: "default", PeerID: "12345"},
+		Text: "first **message**",
+	})
+	if !res1.Delivered {
+		t.Fatalf("first Send err: %v", res1.Err)
+	}
+
+	res2 := a.Send(context.Background(), bridge.Outbound{
+		Peer: bridge.PeerRef{Channel: "telegram", Identity: "default", PeerID: "12345"},
+		Text: "second **message**",
+	})
+	if !res2.Delivered {
+		t.Fatalf("second Send err: %v", res2.Err)
+	}
+
+	mock.mu.Lock()
+	calls := append([]sendCall(nil), mock.sendMsg...)
+	mock.mu.Unlock()
+	// 2 calls for the first message (HTML fail + plain retry), 1 call
+	// for the second (HTML attempt succeeds — no latch skipping it).
+	if len(calls) != 3 {
+		t.Fatalf("sendMessage calls = %d, want 3", len(calls))
+	}
+	if calls[2].ParseMode != string(models.ParseModeHTML) {
+		t.Errorf("third call (second message) ParseMode = %q, want HTML — latch must not persist across messages", calls[2].ParseMode)
+	}
+}
+
+// TestSendMentionSurvivesConversionExactlyOnce verifies the mention
+// prepend still happens exactly once and survives HTML conversion.
+func TestSendMentionSurvivesConversionExactlyOnce(t *testing.T) {
+	t.Parallel()
+	a, mock, _ := newAdapter(t, Identity{ID: "default", Token: "tg-token"})
+
+	res := a.Send(context.Background(), bridge.Outbound{
+		Peer:    bridge.PeerRef{Channel: "telegram", Identity: "default", PeerID: "12345"},
+		Mention: "@reviewer",
+		Text:    "your build finished",
+	})
+	if !res.Delivered {
+		t.Fatalf("Send err: %v", res.Err)
+	}
+
+	mock.mu.Lock()
+	defer mock.mu.Unlock()
+	if len(mock.sendMsg) != 1 {
+		t.Fatalf("sendMessage calls = %d, want 1", len(mock.sendMsg))
+	}
+	got := mock.sendMsg[0].Text
+	if n := strings.Count(got, "@reviewer"); n != 1 {
+		t.Errorf("mention appears %d times, want 1: %q", n, got)
+	}
+	if !strings.HasPrefix(got, "@reviewer") {
+		t.Errorf("text = %q, want mention-prefixed content", got)
+	}
+}
+
+// TestSendEmptyTextIsNoOp verifies empty outbound text sends no
+// sendMessage call and still reports delivered (nothing to attach).
+func TestSendEmptyTextIsNoOp(t *testing.T) {
+	t.Parallel()
+	a, mock, _ := newAdapter(t, Identity{ID: "default", Token: "tg-token"})
+
+	res := a.Send(context.Background(), bridge.Outbound{
+		Peer: bridge.PeerRef{Channel: "telegram", Identity: "default", PeerID: "12345"},
+		Text: "",
+	})
+	if !res.Delivered {
+		t.Fatalf("Send err: %v", res.Err)
+	}
+	mock.mu.Lock()
+	defer mock.mu.Unlock()
+	if len(mock.sendMsg) != 0 {
+		t.Errorf("sendMessage calls = %d, want 0", len(mock.sendMsg))
+	}
+}
+
+// TestSendEscapesPlainAngleBracketsAndAmpersand verifies literal
+// "<", ">", "&" outside any markdown construct are escaped in the sent
+// HTML without corrupting adjacent emitted tags.
+func TestSendEscapesPlainAngleBracketsAndAmpersand(t *testing.T) {
+	t.Parallel()
+	a, mock, _ := newAdapter(t, Identity{ID: "default", Token: "tg-token"})
+
+	res := a.Send(context.Background(), bridge.Outbound{
+		Peer: bridge.PeerRef{Channel: "telegram", Identity: "default", PeerID: "12345"},
+		Text: "a < b && c > d, and **bold** stays bold",
+	})
+	if !res.Delivered {
+		t.Fatalf("Send err: %v", res.Err)
+	}
+	mock.mu.Lock()
+	defer mock.mu.Unlock()
+	got := mock.sendMsg[0].Text
+	if !strings.Contains(got, "a < b && c > d") {
+		t.Errorf("text = %q, want escaped angle brackets/ampersand", got)
+	}
+	if !strings.Contains(got, "bold") {
+		t.Errorf("text = %q, want bold tag preserved alongside escaped prose", got)
+	}
+}
diff --git a/internal/bridge/telegram/adapter_test.go b/internal/bridge/telegram/adapter_test.go
index dd5aefc944..41f7af62f5 100644
--- a/internal/bridge/telegram/adapter_test.go
+++ b/internal/bridge/telegram/adapter_test.go
@@ -33,6 +33,12 @@ type mockTelegramServer struct {
 	// fileBody is what the adapter receives when downloading inbound
 	// files (the /file/bot/ endpoint).
 	fileBody string
+	// sendMessageErrors, when non-empty, is consulted by call index (0-
+	// based) to decide whether the Nth sendMessage call should fail with
+	// a Telegram Bad Request error carrying the given description
+	// instead of succeeding. Empty string (or index past the slice)
+	// means "succeed". Used by the HTML-parse-error fallback tests.
+	sendMessageErrors []string
 }
 
 type editTextCall struct {
@@ -52,6 +58,9 @@ type sendCall struct {
 	ChatID  any
 	Text    string
 	Caption string
+	// ParseMode is the raw parse_mode form value (empty when the call
+	// carried no ParseMode field — the plain-text fallback path).
+	ParseMode string
 	// Multipart filename when the call is sendPhoto/sendAudio/sendDocument.
 	Filename string
 	// FileData contains the bytes sent in the multipart upload.
@@ -113,8 +122,17 @@ func (m *mockTelegramServer) handleBotMethod(w http.ResponseWriter, r *http.Requ
 	case "sendMessage":
 		call := captureSendMessage(r)
 		m.mu.Lock()
+		idx := len(m.sendMsg)
 		m.sendMsg = append(m.sendMsg, call)
+		var injectedErr string
+		if idx < len(m.sendMessageErrors) {
+			injectedErr = m.sendMessageErrors[idx]
+		}
 		m.mu.Unlock()
+		if injectedErr != "" {
+			m.respondError(w, injectedErr)
+			return
+		}
 		m.respond(w, models.Message{ID: 1})
 	case "sendPhoto":
 		call := captureMultipartCall(r, "photo")
@@ -185,6 +203,20 @@ func (m *mockTelegramServer) respond(w http.ResponseWriter, result any) {
 	})
 }
 
+// respondError writes a Telegram-format API error envelope. The bot
+// library (raw_request.go) switches on the body's error_code field, not
+// the HTTP status, so a plain 200 with ok:false is sufficient to drive
+// its error path.
+func (m *mockTelegramServer) respondError(w http.ResponseWriter, description string) {
+	w.Header().Set("Content-Type", "application/json")
+	w.WriteHeader(http.StatusOK)
+	_ = json.NewEncoder(w).Encode(map[string]any{
+		"ok":          false,
+		"error_code":  400,
+		"description": description,
+	})
+}
+
 // captureSendMessage parses sendMessage's multipart form (the bot library
 // uses multipart for every method, not JSON).
 func captureSendMessage(r *http.Request) sendCall {
@@ -201,6 +233,9 @@ func captureSendMessage(r *http.Request) sendCall {
 	if vals, ok := r.MultipartForm.Value["caption"]; ok && len(vals) > 0 {
 		call.Caption = vals[0]
 	}
+	if vals, ok := r.MultipartForm.Value["parse_mode"]; ok && len(vals) > 0 {
+		call.ParseMode = vals[0]
+	}
 	return call
 }
 
diff --git a/openspec/changes/bridge-slack-native-markdown/.openspec.yaml b/openspec/changes/bridge-slack-native-markdown/.openspec.yaml
new file mode 100644
index 0000000000..2e24cfa4fa
--- /dev/null
+++ b/openspec/changes/bridge-slack-native-markdown/.openspec.yaml
@@ -0,0 +1,2 @@
+schema: spec-driven
+created: 2026-09-07
diff --git a/openspec/changes/bridge-slack-native-markdown/design.md b/openspec/changes/bridge-slack-native-markdown/design.md
new file mode 100644
index 0000000000..d0daff6bbd
--- /dev/null
+++ b/openspec/changes/bridge-slack-native-markdown/design.md
@@ -0,0 +1,314 @@
+# Design: bridge-slack-native-markdown
+
+## Context
+
+See `proposal.md § Why` for motivation. The relevant existing architecture:
+
+- `internal/bridge/slack/adapter.go:901` builds the outbound text part with
+  `slackgo.MsgOptionText(text, false)` — Slack's top-level `text` field, parsed as mrkdwn.
+- `internal/bridge/slack/adapter.go:27-28` defines `MaxTextLength = 39_000` (Slack's
+  `text`-field character cap) and `truncateRunes` (`:973-989`) trims to that cap without an
+  ellipsis.
+- `internal/bridge/slack/render.go` is a separate, already-correct code path: the
+  `RichRenderer` (`renderToolCall`, `renderToolResult`, `renderList`, `renderTable`,
+  `renderStatus`) hand-authors Block Kit `section`/`context` blocks with `slackgo.MarkdownType`
+  text objects, which is mrkdwn — appropriate there because the content (tool params,
+  previews) is composed directly in mrkdwn syntax, not passed through unmodified from an
+  LLM. This path is unaffected by this change.
+- `internal/bridge/telegram/adapter.go:849-884` (`Send`) calls `chunkText` (`:926-960`, rune-
+  safe, no markdown awareness) and posts each chunk via `bot.SendMessage` with **no**
+  `ParseMode` field set at all.
+- `internal/bridge/telegram/adapter.go:31` defines `MaxTextLength = 4096` (Telegram's
+  `sendMessage` character cap, counted after entity parsing, not before).
+- `internal/bridge/telegram/render.go:255-262` documents why the `RichRenderer` path uses
+  `models.ParseModeMarkdownV1` (legacy Markdown) rather than MarkdownV2: "MarkdownV2 is more
+  forgiving... tool params and previews contain too many otherwise-reserved chars to escape
+  reliably (e.g. '.' in decimal durations is reserved in MarkdownV2)." That reasoning
+  motivates going one step further here and using Telegram HTML for the prose `Send` path,
+  which needs zero syntax escaping beyond three characters.
+- `internal/bridge/mattermost/adapter.go:514-570` (`Send`) posts `text` directly as
+  `CreatePostInput.Message`; Mattermost's server parses `Post.Message` as standard GFM, so
+  this path already renders correctly and needs no change. It is the reference for what
+  "renders correctly" looks like.
+- `internal/bridge/service/dispatch.go:585-621` (`handleTerminalEvent`) and `:627-640`
+  (`agentMessageText`) concatenate the agent's `TextContent` parts (GFM, as the LLM emits
+  it) into `Outbound.Text` and fan it out via `Service.SendBySessionID` to each bound
+  adapter's `Send`. This change does not touch the dispatcher — it only changes how each
+  adapter's `Send` renders the `Outbound.Text` it receives.
+- `internal/bridge/bridge.go` (package `bridge`) is deliberately dependency-free — it
+  defines only the shared `Adapter` interface and value types, imported by every platform
+  package. `internal/bridge/markdown` follows the same discipline (stdlib only) so it can be
+  imported by `slack`, `telegram`, and (if ever needed) `bridge` itself without a cycle.
+
+## Goals / Non-Goals
+
+**Goals:**
+
+- Slack and Telegram render the same GFM prose Mattermost already renders correctly, with no
+  behavior change to Mattermost.
+- Neither platform drops a message because of a formatting failure — automatic degradation
+  to plain text/no-parse-mode on error, not a lost send.
+- No new dependency, no new config field, no schema change.
+- The existing `RichRenderer` tool-card paths are untouched.
+
+**Non-Goals:**
+
+- Hand-rolling a full GFM→mrkdwn converter (see Rejected Alternatives).
+- A config opt-out for the new rendering behavior (see Rejected Alternatives).
+- Telegram MarkdownV2 (see Rejected Alternatives).
+- File-upload-on-truncation fallback (see Rejected Alternatives).
+- Any change to Mattermost's adapter, the `RichRenderer` paths, or the dispatcher's message
+  assembly (`handleTerminalEvent` / `agentMessageText`).
+
+## Dialect comparison: mrkdwn vs. GFM vs. Mattermost
+
+| Construct | Slack `text`/mrkdwn (today, broken) | Slack `markdown` block (this change) | Mattermost `Post.Message` (unchanged, reference) |
+|---|---|---|---|
+| `**bold**` / `__bold__` | Not recognized (mrkdwn bold is single `*bold*`) — renders literal `**bold**` | Recognized, renders bold | Recognized, renders bold |
+| `*italic*` / `_italic_` | mrkdwn italic is single `_italic_`; `*italic*` instead renders BOLD (mrkdwn's own bold syntax) — wrong emphasis, not literal text | Recognized, renders italic | Recognized, renders italic |
+| `## heading` | Not recognized — renders literal `## heading` | Recognized; all heading levels render the same size (Slack doc-confirmed) | Recognized, renders sized headings |
+| `- item` / `* item` | Not recognized as a list — renders literal `- item` | Recognized, renders a bulleted list (ordered, unordered, and task lists) | Recognized, renders a bulleted list |
+| `` `code` `` | Recognized (mrkdwn also uses single backtick) — renders correctly today | Recognized, renders correctly | Recognized, renders correctly |
+| ` ```lang\ncode\n``` ` | Fence recognized but **no syntax highlighting**; `lang` info string ignored | Recognized WITH syntax highlighting per Slack docs | Recognized WITH syntax highlighting |
+| `[label](url)` | Not recognized — renders literal `[label](url)`; mrkdwn's own link syntax is `` | Recognized, renders a real hyperlink | Recognized, renders a real hyperlink |
+| `~~strike~~` | mrkdwn strike is single `~strike~`; `~~strike~~` instead renders STRIKETHROUGH with a stray leftover tilde (the outer `~`/`~` pair triggers mrkdwn's strikethrough, leaving one `~` literal) — not simply unrecognized | Recognized, renders strikethrough | Recognized, renders strikethrough |
+| `> quote` | Recognized (mrkdwn also uses `>`) | Recognized, renders blockquote | Recognized, renders blockquote |
+| `\| a \| b \|` tables | Not recognized — renders literal pipe text | Recognized, renders a real table | Recognized, renders a real table |
+| `---` divider | Not recognized — renders literal dashes | Recognized, renders a divider | Recognized, renders a horizontal rule |
+| `![alt](url)` images | Not recognized — renders literal text | Rendered as a hyperlink (per Slack docs; not an inline image) | Recognized, renders an inline image |
+| `` (mrkdwn's own link syntax, sometimes emitted by agents) | Recognized (this IS the native mrkdwn syntax) | NOT recognized by the `markdown` block parser — must be normalized to `[label](url)` before emission (see `NormalizeSlackLinks`) | N/A (Mattermost never receives this syntax) |
+| `<@U123>` user mention | Recognized | Recognized (passed through unmodified) | N/A (different mention syntax) |
+
+Slack facts cited from `docs.slack.dev/reference/block-kit/blocks/markdown-block/` and the
+blocks reference: the `markdown` block type is available on the Messages surface; the
+cumulative character limit across all `markdown` blocks in one payload is 12,000 characters;
+`block_id` is accepted but ignored; a message may contain at most 50 blocks total (all block
+types combined, not just `markdown`).
+
+## Slack budget arithmetic
+
+- **Payload budget**: 12,000 characters, cumulative across every `markdown` block in one
+  `chat.postMessage` call. This is Slack's documented hard limit — exceeding it produces a
+  `blocks_too_long`-class API error.
+- **Per-block target**: 3,000 characters. Chosen as a sub-limit well under any theoretical
+  per-block cap, keeping each block a reasonably sized, independently renderable chunk of
+  markdown (relevant for the fence-aware chunker, which must reason about "how much room is
+  left in this block").
+- **Chunk-count derivation**: `blockCount = ceil(budget / perBlockTarget)` =
+  `ceil(12000 / 3000)` = 4 blocks. The chunker then divides the *actual budget* evenly across
+  that many blocks: `perChunkLimit = floor(budget / blockCount)` = `floor(12000 / 4)` = 3,000.
+  This guarantees `sum(chunkLengths) <= budget` exactly, rather than allowing 4 blocks each
+  chunked independently at 3,000 which could total up to 12,000 but risks off-by-one
+  overruns from multi-byte rune boundaries; deriving from the same budget/count pair keeps
+  the arithmetic exact.
+- **Block-count cap**: 50 total blocks per message is Slack's absolute ceiling (shared across
+  all block types in the payload, not just `markdown`). The adapter MUST cap emitted
+  `markdown` blocks such that this can never be exceeded — with a 4-block target this is
+  never in practice a binding constraint, but the cap MUST be enforced defensively (e.g. if a
+  future change lowers `perBlockTarget`, the block count must not silently grow past 50).
+- If content exceeds the total 12,000-character budget across all blocks, the excess is
+  dropped and the visible truncation marker (see below) is appended to the last emitted
+  block, with room for the marker reserved from the budget *before* the final chunk cut is
+  computed (not appended after, which could itself overflow the budget).
+
+## Chunking algorithm (shared, `internal/bridge/markdown`)
+
+Applies to both the Slack per-block chunker and the Telegram source-chunker, parameterized
+by a caller-supplied `limit` (Slack: derived per-chunk limit from the budget arithmetic
+above; Telegram: ~3,500).
+
+1. If the input fits within `limit` characters (rune count, not byte count), return it as a
+   single chunk with `truncated = false`. No further steps run.
+2. Otherwise, find the best cut point at or before `limit`, in priority order:
+   a. The last blank line (`\n\n`) at or before `limit` — preferred because it is the
+      strongest semantic boundary (paragraph break).
+   b. Failing that, the last markdown heading line (`^#{1,6} `) at or before `limit` — a
+      weaker but still meaningful semantic boundary.
+   c. Failing that, the last newline at or before `limit` — any line boundary.
+   A semantic boundary from (a) or (b) is accepted only if it retains at least half of
+   `limit` characters in the chunk (i.e. the cut point is not absurdly early in the buffer);
+   otherwise the algorithm falls through to (c), and if even a line boundary retains less
+   than half of `limit`, falls through to step 4 (hard-wrap).
+3. The cut point MUST NOT fall inside an open ` ``` ` fence. Track fence state (open/closed,
+   and the fence's info string, e.g. `python`) by scanning lines up to the candidate cut
+   point. If the candidate cut point is inside an open fence, close the fence at the cut
+   (append a synthetic ` ``` ` line to the outgoing chunk) and reopen it at the start of the
+   next chunk (prepend ` ```\n` before the continuation content). This keeps
+   every emitted chunk independently valid markdown.
+4. If a single line (with no internal blank-line or heading boundary) exceeds `limit` on its
+   own — e.g. a very long unbroken paragraph or a single long code line — hard-wrap it at
+   exactly `limit` runes, walking the string rune-by-rune (never splitting a multi-byte UTF-8
+   codepoint), and continue chunking the remainder as a new logical line.
+5. On the LAST chunk the caller is willing to emit (i.e. the point where the per-message
+   budget — Slack's 12,000/4-block cap, or the "no more chunks" decision for Telegram —
+   would be exceeded by continuing), the chunker packs content to fill the limit exactly
+   rather than stopping at the nearest tidy semantic boundary; this chunk is where truncation
+   happens (step 6), so maximizing content density here matters more than aesthetics.
+6. If content remains after the last permitted chunk (truncation is occurring): reserve room
+   for the truncation marker FIRST — recompute the last chunk's cut point at
+   `limit - len(marker)`, not `limit`, so the marker is added within budget rather than
+   pushing the chunk over — then append the marker (`\n\n_…truncated…_`, italic prose so it
+   is visually distinct from agent content). If this final re-cut broke an open fence, close
+   it (step 3's fence-closing logic applies again) before appending the marker, so the marker
+   itself never renders inside a code block.
+7. Every function implementing this algorithm returns `(chunks []string, truncated bool)` (or
+   `(chunk string, truncated bool)` for a single-chunk caller); callers MUST propagate
+   `truncated` so upstream code (tests, logs) can assert on it without re-deriving it from
+   chunk count.
+
+## Telegram GFM → HTML mapping
+
+Telegram HTML (`models.ParseModeHTML`) supports exactly: ` `, ` `, `
+`, `  `, ``, ``, `
`, `
`, `
` (and `
`), ``. Escaping +is required only for `&` → `&`, `<` → `<`, `>` → `>`, and only in text runs — never +inside an already-emitted tag's attribute syntax or inside code content (Telegram does not +recognize entities inside ``/`
`, so code content must still be escaped for the three
+characters but not further transformed).
+
+| GFM construct | Telegram HTML output | Notes |
+|---|---|---|
+| `**bold**` / `__bold__` | `bold` | |
+| `*italic*` / `_italic_` | `italic` | |
+| `~~strike~~` | `strike` | |
+| `` `code` `` | `code` | Content escaped for `&<>`, not further parsed |
+| ` ```lang\ncode\n``` ` | `
code
` | `lang` omitted if no info string | +| `[label](url)` | `
label` | `url` escaped for `&<>` in the attribute | +| `# / ## / ... heading` | `heading text` followed by a line break | No native heading tag; bold is the closest degrade | +| `- item` / `* item` (unordered list) | `• item` (bullet glyph substitution, one per line) | No native list tag | +| `1. item` (ordered list) | `1. item` (kept verbatim; numbering is already plain text) | No native list tag; GFM numbering is preserved as-is | +| `> quote` | `
quote
` | | +| `\| a \| b \|` table | Wrapped in `
...
` preserving the original column-aligned text | Preserves alignment as monospace; no native table tag | +| `---` horizontal rule | A literal line of dashes (e.g. `——————————`) | No native rule; visual approximation | +| `![alt](url)` image | `alt` (or `url` if `alt` is empty) | Same hyperlink degrade as Slack's `markdown` block | +| Plain prose text | Escaped for `&<>` only | No tag wrapping | + +Implementation shape: `ToTelegramHTML` first extracts code spans and fenced code blocks into +placeholders (so inline emphasis/link parsing never fires inside code content), performs +line-oriented and inline substitution for the remaining constructs, escapes `&<>` in plain +text runs, then restores the placeholders as properly escaped ``/`
` content. This
+placeholder-extraction order is the safeguard against, e.g., an underscore inside a code span
+being misinterpreted as italic markup.
+
+## Fallback / latch state machine
+
+### Slack
+
+```
+        send with markdown blocks
+                 |
+         success ----------------------------> done (latch unaffected)
+                 |
+     block-related API error
+     (invalid_blocks / invalid_block /
+      invalid_arguments / blocks_too_long)
+                 |
+                 v
+     log one-shot WARN; set adapter-level
+     latch (sticky, in-memory, per Adapter
+     instance) = "blocks disabled"
+                 |
+                 v
+     retry the SAME logical send as plain
+     text (today's MsgOptionText behavior)
+                 |
+                 v
+              done (message delivered either way)
+
+  Any subsequent Send on this Adapter instance,
+  while the latch is set, skips the markdown-block
+  path entirely and goes straight to plain text —
+  no repeated failed attempts, no repeated WARN spam.
+```
+
+The latch is a boolean guarded the same way `lastError` / `lastFailureAt` already are
+(`atomic.Value` / `atomic.Bool` on the `Adapter` struct) — no new locking primitive. It is
+per-`Adapter`-instance (i.e. per Slack identity/workspace connection), not global, since block
+support is a workspace/app-level capability that will not vary within one adapter's lifetime
+but could differ across identities.
+
+### Telegram
+
+```
+        send chunk with ParseModeHTML
+                 |
+         success ----------------------------> next chunk / done
+                 |
+      entity/parse error
+      (e.g. "can't parse entities")
+                 |
+                 v
+     retry THIS chunk, unchanged text,
+     with NO ParseMode field
+                 |
+                 v
+       done for this chunk (no latch —
+       next message's chunks attempt
+       ParseModeHTML fresh)
+```
+
+No sticky latch on Telegram: unlike Slack's block-type support (a capability of the
+workspace/app), a Telegram entity-parse failure is specific to the content of one chunk (an
+edge case in the GFM→HTML conversion producing invalid nesting, or a genuinely malformed
+tag), not a capability the bot lacks. Latching would incorrectly degrade all future messages
+because of one bad chunk.
+
+## Rejected alternatives
+
+1. **Hand-roll a GFM→mrkdwn converter for Slack, instead of using the `markdown` block.**
+   Rejected: mrkdwn cannot represent headings, tables, ordered lists, or syntax-highlighted
+   code fences at all — no converter output could round-trip those constructs faithfully, so
+   any hand-rolled converter would still lose information the LLM intended to convey. Using
+   Slack's native `markdown` block, which the platform itself parses as standard Markdown,
+   eliminates the entire class of "which mrkdwn escape sequence approximates a GFM table"
+   problems by not needing an approximation at all.
+
+2. **A config opt-out flag (e.g. `router.slackNativeMarkdownEnabled`).** Rejected: the
+   change is a bug fix, not a new feature with a debatable default — mrkdwn rendering of raw
+   GFM is strictly worse for every existing user in every existing configuration; there is no
+   scenario where a user would want literal `**bold**` asterisks over rendered bold text. A
+   flag would only add a schema-update obligation (per `CLAUDE.md`) and a permanently-true
+   toggle nobody would ever set to `false`. The automatic per-message fallback already
+   provides the safety net a flag would otherwise exist for.
+
+3. **Telegram MarkdownV2 instead of HTML.** Rejected: MarkdownV2 requires escaping roughly 18
+   reserved characters (`_ * [ ] ( ) ~ \` > # + - = | { } . !`) anywhere they appear outside
+   of an intentional markup construct, including inside things as mundane as a decimal
+   number (`.`) or a version string (`-`). `internal/bridge/telegram/render.go`'s own
+   comment already documents this exact problem for the `RichRenderer` path and chose
+   `ParseModeMarkdownV1` for that reason. HTML requires escaping only 3 characters and has an
+   unambiguous, well-specified tag grammar, making the converter far less likely to produce
+   an unparseable message from LLM-authored GFM.
+
+4. **File upload on truncation, mirroring `c2-agent`'s orchestrator.** Rejected for this
+   change's scope: it requires plumbing a file-upload path through both adapters' truncation
+   handling, choosing a filename/extension convention, and deciding how the two mechanisms
+   (visible marker vs. attached file) compose when both platforms are bound to the same
+   session. The visible truncation marker is a strictly smaller, self-contained change that
+   already solves the "silent data loss" problem the marker exists to prevent; file upload on
+   truncation can be proposed as an independent follow-up if truncation in practice proves
+   disruptive.
+
+## Risks / Trade-offs
+
+**[Risk] Slack `markdown` block support could vary by workspace/app configuration in ways
+not fully covered by the documented error codes.** Mitigated by treating any block-related
+API error class as latch-triggering (broad error-code matching, not an exhaustive enum) —
+false positives (latching for an unrelated but coincidentally block-shaped error) degrade to
+plain text, which is always safe; false negatives (a block failure not recognized as such)
+would retry as blocks again next message rather than latching, at worst repeating one log
+line per occurrence.
+
+**[Risk] Telegram HTML conversion producing invalid nesting from adversarial or malformed
+GFM input.** Mitigated by the per-chunk fallback retry (no `ParseMode`) — a conversion bug
+degrades that one chunk to plain text rather than failing the send.
+
+**[Trade-off] The chunking algorithm is more complex than a naive length-based split.** The
+complexity (semantic boundaries, fence-awareness, budget-aware final-chunk packing) is
+justified because a naive split reintroduces exactly the failure mode this change fixes: a
+fence split mid-block breaks syntax highlighting and can leave a dangling, unclosed code
+block for the rest of the conversation view.
+
+## Open Questions
+
+None that would change the spec, approach, or task breakdown.
diff --git a/openspec/changes/bridge-slack-native-markdown/proposal.md b/openspec/changes/bridge-slack-native-markdown/proposal.md
new file mode 100644
index 0000000000..fb532a7f20
--- /dev/null
+++ b/openspec/changes/bridge-slack-native-markdown/proposal.md
@@ -0,0 +1,117 @@
+## Why
+
+The bridge sends the agent's final assistant text to Slack via
+`slackgo.MsgOptionText(text, false)` (`internal/bridge/slack/adapter.go:901`) — Slack's
+top-level `text` field, which is parsed as **mrkdwn**, a dialect that is NOT GitHub-Flavored
+Markdown (GFM). LLM output is GFM. Observed in production: `## headings`, `**bold**`, `- `
+bullets, `[label](url)` links, and pipe tables all render as literal characters; only
+backtick code spans survive by coincidence (both dialects use single backticks for inline
+code). Mattermost renders the identical text correctly today because `Post.Message` is
+parsed as real GFM server-side. Telegram has the mirror-image bug: `Send`
+(`internal/bridge/telegram/adapter.go:849`) sets **no** `ParseMode` at all, so every markup
+character — including code fences — renders verbatim as plain text.
+
+## What Changes
+
+- **Slack: post agent prose as Block Kit `markdown` blocks.** Slack's `markdown` block type
+  (`{"type":"markdown","text":...}`) is parsed as real standard Markdown server-side —
+  distinct from the legacy mrkdwn dialect used by `text` and `section` blocks. The adapter
+  builds one or more `slack.NewMarkdownBlock(...)` blocks (already available in
+  `github.com/slack-go/slack v0.25.0`, no new dependency) instead of passing raw text to
+  `MsgOptionText`. The top-level `text` field is still sent alongside the blocks as the
+  notification / accessibility fallback.
+- **Telegram: convert GFM to Telegram HTML (`ParseModeHTML`), not MarkdownV2.** Telegram
+  HTML supports a small fixed tag set (`      
+  
`) and requires escaping only three characters (`& < >`), making it far more + robust than MarkdownV2's ~18 reserved characters. `internal/bridge/telegram/render.go` + already documents why MarkdownV2 was rejected for the `RichRenderer` path; this change + extends that same reasoning to the `Send` prose path. +- **New shared package `internal/bridge/markdown` (stdlib-only).** Provides: + `NormalizeSlackLinks` (rewrites legacy `` / `` mrkdwn link syntax some + agents emit into `[label](url)` / bare `url`, without touching `<@U123>` mentions or + incidental `<`/`>` characters), a fence-aware chunker shared by both platforms (never + splits inside a ` ``` ` fence; closes and reopens fences across chunk boundaries; packs the + final permitted chunk to the limit instead of stopping at a tidy boundary; hard-wraps + rune-safely any single line that exceeds the limit on its own), a visible truncation + marker appended when content is dropped, and `ToTelegramHTML` (the GFM→HTML converter, + which protects code spans/fences from inline transformation before escaping `& < >` in + prose runs). This package has no dependency on `internal/bridge` (which is deliberately + dependency-free) and is imported by both the `slack` and `telegram` adapter packages + without introducing an import cycle. +- **Slack payload budget:** 12,000 characters cumulative across all `markdown` blocks in one + `chat.postMessage` call (Slack's documented per-payload limit for the block type), a + 3,000-character per-block target, and a hard cap of 50 blocks per message (Slack's + overall block-count limit). The chunk limit is derived by dividing the budget into + `ceil(budget/3000)` equal parts so the sum across emitted blocks never exceeds the budget. +- **Telegram chunk budget:** the *source* markdown is chunked at a conservative ~3,500 + characters so that neither the raw chunk nor its HTML-converted, entity-expanded output + can exceed Telegram's 4,096-character `sendMessage` limit; each chunk is a self-contained, + independently valid markdown fragment (fences closed/reopened across chunk boundaries) so + conversion per chunk is correct in isolation. +- **Resilience — a formatting failure MUST NEVER drop a message on either platform.** + - Slack: if `chat.postMessage` fails with a block-related API error (`invalid_blocks`, + `invalid_block`, `invalid_arguments`, `blocks_too_long`, or similar), the adapter + transparently retries the *same* send as plain text (today's behavior), logs a one-shot + WARN, and latches a per-adapter flag so subsequent sends on that adapter instance skip + the blocks path entirely — a workspace/app that cannot render `markdown` blocks degrades + permanently to plain text rather than failing repeatedly. + - Telegram: if `sendMessage` fails with an entity/parse error (e.g. `can't parse + entities`), the adapter retries that one chunk, unchanged, with no `ParseMode` and the + original markdown text. This is content-specific (a single malformed chunk, not a + platform-wide capability gap), so there is no sticky latch — every message gets a fresh + attempt at HTML rendering. + +### Out of scope + +- The Slack `RichRenderer` tool-card path (`internal/bridge/slack/render.go`) and the + Telegram `RichRenderer` (`internal/bridge/telegram/render.go`) are unchanged — they + hand-author valid per-platform markup already and are not affected by this change. +- `SendQueuedAck` / `UpdateMessage` short status strings stay plain text on both platforms. +- Mattermost is unchanged — its existing GFM rendering via `Post.Message` is already + correct. +- No file-upload-on-truncation fallback. `c2-agent`'s orchestrator has one; this change + accepts a visible truncation marker (`_…truncated…_`) instead, to keep the blast radius + small. +- No new `.opencode.json` config field. The corrected rendering is always on with automatic + per-platform degradation on failure; there is no opt-out switch. + +## Capabilities + +### Modified Capabilities + +- `chat-bridge-adapters`: amends the Slack adapter requirement so prose outbound text is + rendered as GFM via Block Kit `markdown` blocks (with plain-text fallback on + block-rendering failure), and amends the Telegram adapter requirement so prose outbound + text is rendered as GFM converted to Telegram HTML (with plain-text fallback on + parse-entity failure). Adds the shared chunking/truncation-marker contract both adapters + must honor. + +## Impact + +**`github.com/opencode-ai/opencode`** + +- `internal/bridge/markdown/` (new package): `NormalizeSlackLinks`, the fence-aware + chunker, the truncation marker, `ToTelegramHTML`. Stdlib only — no new dependency. +- `internal/bridge/slack/adapter.go` (`Send`, ~lines 882-947): replace + `slackgo.MsgOptionText(text, false)` for the prose text part with `markdown` block + construction; add the block-error detection, plain-text retry, and per-adapter sticky + latch. +- `internal/bridge/telegram/adapter.go` (`Send`, ~lines 849-884; `chunkText`, ~926-960): + replace the plain `chunkText` + no-`ParseMode` `sendMessage` with markdown-aware chunking + (source-level, via `internal/bridge/markdown`) + `ToTelegramHTML` conversion + + `models.ParseModeHTML`; add the per-chunk entity-error retry with no `ParseMode`. +- `internal/bridge/slack/render.go`, `internal/bridge/telegram/render.go`: unchanged (out of + scope, confirmed by reading — the `RichRenderer` paths hand-author valid markup already). +- `internal/bridge/mattermost/adapter.go`: unchanged (out of scope). +- `internal/bridge/service/dispatch.go` (`handleTerminalEvent`, `agentMessageText`): + unchanged — these produce the `Outbound.Text` this change formats differently at the + adapter tier; no change to the dispatcher itself. +- No new dependency: `github.com/slack-go/slack v0.25.0` (already in `go.mod`) already + provides `slack.NewMarkdownBlock` / `slack.MBTMarkdown`; Telegram HTML uses + `models.ParseModeHTML`, already present in `github.com/go-telegram/bot`. +- No `.opencode.json` / `Config` field changes — no `cmd/schema/main.go` or + `opencode-schema.json` update is required by this change. +- No database schema change. +- `docs/bridge.md`: out of scope for this planning change per the task boundary (planning + artifacts only); a follow-up implementation PR should note the corrected rendering + behavior there. diff --git a/openspec/changes/bridge-slack-native-markdown/specs/chat-bridge-adapters/spec.md b/openspec/changes/bridge-slack-native-markdown/specs/chat-bridge-adapters/spec.md new file mode 100644 index 0000000000..8f9f68596d --- /dev/null +++ b/openspec/changes/bridge-slack-native-markdown/specs/chat-bridge-adapters/spec.md @@ -0,0 +1,228 @@ +## Purpose + +Delta spec for the `chat-bridge-adapters` capability. Amends the Slack and Telegram adapter +requirements so outbound prose text (the agent's GFM-formatted assistant messages) renders +correctly on each platform instead of showing literal markdown syntax, and adds the shared +chunking/truncation and failure-degradation contract both adapters must honor. The +`RichRenderer` requirements (tool-call cards, lists, tables, status) and the Mattermost +adapter requirement are unaffected. + +## MODIFIED Requirements + +### Requirement: Slack adapter via slack-go/slack + +The Slack adapter SHALL use `github.com/slack-go/slack` Socket Mode. The adapter MUST +handle: `app_mention`, `message.im`, file uploads. Files via `files.upload`. Socket Mode +handshake retries and reconnects MUST rely on the library's built-in behavior rather than +re-implementing in-process retry logic. + +**Outbound prose text MUST be rendered as GitHub-Flavored Markdown (GFM), not the legacy +mrkdwn dialect.** `chat.postMessage` calls for agent-authored prose (the `Send` method's +text part) SHALL construct one or more Block Kit `markdown` blocks +(`slack.NewMarkdownBlock`, `type: "markdown"`) from the outbound text, rather than passing +the text only through the top-level `text` field. The `markdown` block type is parsed by +Slack as standard Markdown server-side — headings, bold/italic, strikethrough, ordered and +unordered lists (including task lists), links, blockquotes, syntax-highlighted fenced code +blocks, tables, and `---` dividers all render correctly, unlike mrkdwn which recognizes none +of these except single-backtick inline code and `>` blockquotes. The top-level `text` field +MUST still be set (as the notification/accessibility fallback) alongside the blocks. + +Before block construction, outbound text SHALL be passed through +`internal/bridge/markdown.NormalizeSlackLinks`, which rewrites legacy mrkdwn link syntax +(`` → `[label](https://x)`, `` → `https://x`) that some agents +emit, without altering `<@U123>`-style mentions or unrelated angle-bracket text. Text SHALL +be split into `markdown` blocks using the shared fence-aware chunker +(`internal/bridge/markdown`), respecting a cumulative 12,000-character budget across all +`markdown` blocks in one `chat.postMessage` call and a 50-block-per-message cap, with a +visible truncation marker appended when content is dropped. + +If `chat.postMessage` with `markdown` blocks fails with a block-related API error +(`invalid_blocks`, `invalid_block`, `invalid_arguments`, `blocks_too_long`, +`msg_blocks_too_long`, or `invalid_block_id`), the adapter MUST transparently retry the same +send as plain text via the pre-existing `MsgOptionText` path. Only the unambiguous subset of +that same list — MINUS `invalid_arguments` — MUST also set a per-`Adapter`-instance sticky +latch (with a one-shot WARN) so subsequent `Send` calls on that adapter instance skip the +`markdown`-block path and go directly to plain text; `invalid_arguments` is excluded from the +latch because Slack also returns it for reasons unrelated to block support (e.g. a bad +channel or timestamp), and latching on it would permanently downgrade formatting for the +identity based on an ambiguous signal. A message MUST NEVER be dropped because of a +block-construction or block-rendering failure. + +#### Scenario: Bot mentioned in channel + +- **WHEN** the Slack `app_mention` event arrives for the configured bot identity +- **THEN** the adapter normalizes the event to an `Inbound` (stripping the bot mention) and forwards it to the orchestrator + +#### Scenario: File attached to inbound message + +- **WHEN** a Slack message includes a file attachment +- **THEN** the adapter downloads the file to the bridge media store and the agent receives the local path as an attachment + +#### Scenario: GFM prose renders correctly via markdown blocks + +- **GIVEN** the agent's outbound text contains `## Summary`, a `**bold**` phrase, a + `- bullet` list, a `[link](https://example.com)`, and a fenced ` ```go ` code block +- **WHEN** `Send` posts this text to a Slack channel +- **THEN** `chat.postMessage` is called with one or more `markdown`-type blocks containing + the text verbatim (after link normalization); Slack renders a real heading, bold text, a + bulleted list, a clickable link, and a syntax-highlighted code block — none of the source + markdown characters are visible literally in the rendered message + +#### Scenario: Legacy mrkdwn link syntax is normalized before block construction + +- **GIVEN** the agent's outbound text contains `` +- **WHEN** `Send` posts this text +- **THEN** `NormalizeSlackLinks` rewrites it to `[click here](https://example.com)` before + the `markdown` block is constructed, so the `markdown` block parser (which does not + understand mrkdwn's `` syntax) renders it as a proper hyperlink + +#### Scenario: User mentions are left untouched by link normalization + +- **GIVEN** the agent's outbound text contains `<@U12345>` and the literal comparison + `a < b && b > c` +- **WHEN** `NormalizeSlackLinks` processes the text +- **THEN** `<@U12345>` and `a < b && b > c` are returned unchanged — neither is mistaken for + URL link syntax + +#### Scenario: Block-related API error falls back to plain text and latches + +- **GIVEN** a Slack workspace/app whose `chat.postMessage` rejects `markdown` blocks with an + `invalid_blocks` error +- **WHEN** `Send` is called with agent prose +- **THEN** the adapter retries the same send as plain text via `MsgOptionText`, the message + is delivered, a one-shot WARN is logged, and a per-adapter latch is set + +#### Scenario: Latched adapter skips the blocks path on subsequent sends + +- **GIVEN** the per-adapter latch from the prior scenario is set +- **WHEN** a second, unrelated `Send` call is made on the same `Adapter` instance +- **THEN** the adapter sends plain text directly via `MsgOptionText` without attempting + `markdown` blocks first, and without emitting a second WARN for the same latch + +#### Scenario: Ambiguous invalid_arguments failure retries as plain text but does not latch + +- **GIVEN** a Slack workspace/app whose `chat.postMessage` rejects `markdown` blocks with an + `invalid_arguments` error (which Slack also returns for unrelated reasons such as a bad + channel or timestamp) +- **WHEN** `Send` is called with agent prose +- **THEN** the adapter retries the same send as plain text via `MsgOptionText` and the message + is delivered, but no per-adapter latch is set — a subsequent, unrelated `Send` call on the + same `Adapter` instance attempts `markdown` blocks again rather than skipping straight to + plain text + +#### Scenario: Oversized prose is truncated with a visible marker, not silently cut + +- **GIVEN** the agent's outbound text exceeds the 12,000-character cumulative `markdown` + block budget +- **WHEN** `Send` posts this text +- **THEN** the emitted `markdown` blocks together stay within the 12,000-character budget, + the last block ends with a visible truncation marker (e.g. `_…truncated…_`), and any code + fence open at the truncation point is closed before the marker so the marker does not + render inside a code block + +### Requirement: Telegram adapter via go-telegram/bot + +The Telegram adapter SHALL use `github.com/go-telegram/bot` for long-polling. The adapter +MUST implement: private/public access mode per identity, mention extraction, media download +into the bridge media store, outbound text chunking, file upload, and reply-to-thread. The +adapter MUST NOT use webhook mode (no inbound HTTP exposure required). + +**Outbound prose text MUST be rendered as GitHub-Flavored Markdown (GFM) converted to +Telegram HTML, not sent unparsed.** The `Send` method's outbound text part SHALL be +converted via `internal/bridge/markdown.ToTelegramHTML` and sent with +`models.ParseModeHTML`, rather than the current behavior of setting no `ParseMode` at all +(which renders every markdown character, including code fences, as literal text). Telegram +HTML supports `
 
`; the converter maps GFM +headings to bold text, unordered list items to `•`-prefixed lines, tables to `
`-wrapped
+column-aligned text, and `---` to a literal dash rule, and escapes only `& < >` in prose
+text runs (never inside already-converted tags or code content beyond the same three
+characters).
+
+Before conversion, outbound text SHALL be split at the *source markdown* level using the
+shared fence-aware chunker (`internal/bridge/markdown`) at a conservative ~3,500-character
+limit — not the raw 4,096-character `MaxTextLength` — so that neither the pre-conversion
+chunk nor its HTML-entity-expanded, post-conversion form can exceed Telegram's 4,096-
+character `sendMessage` cap. Each chunk MUST be a self-contained, independently valid
+markdown fragment (any code fence open at a chunk boundary is closed on the outgoing chunk
+and reopened with the same info string on the next), since each chunk is converted to HTML
+independently.
+
+If `sendMessage` with `ParseModeHTML` fails with an entity/parse error (e.g. `can't parse
+entities`), the adapter MUST retry that one chunk, unchanged, with no `ParseMode` field and
+the original markdown text — a content-specific retry, not a sticky per-adapter latch (each
+message's chunks attempt HTML conversion fresh). A chunk MUST NEVER be dropped because of an
+HTML conversion or entity-parse failure.
+
+#### Scenario: Long-poll loop
+
+- **WHEN** the Telegram adapter starts for an identity with a valid token
+- **THEN** it begins long-polling `getUpdates`; received messages are normalized to the bridge's `Inbound` type and forwarded to the orchestrator
+
+#### Scenario: Pairing-code flow
+
+- **WHEN** a peer sends a pairing code matching the `pairingCodeHash` configured under `router.channels.telegram.bots[].pairingCodeHash`
+- **THEN** the peer is added to `bridge_allowlist` for that identity
+
+#### Scenario: Inbound media
+
+- **WHEN** an inbound Telegram message contains a photo or document
+- **THEN** the adapter downloads the file to `/bridge/media/` and the orchestrator passes the path to the agent as an attachment
+
+#### Scenario: GFM prose renders correctly via Telegram HTML
+
+- **GIVEN** the agent's outbound text contains `## Summary`, a `**bold**` phrase, a
+  `- bullet` list, a `[link](https://example.com)`, and a fenced ` ```go ` code block
+- **WHEN** `Send` posts this text to a Telegram chat
+- **THEN** the text is converted to Telegram HTML (`Summary` for the heading,
+  `bold`, a `•`-prefixed line for the bullet, `link`,
+  and `
...
` for the fenced block) and sent with + `ParseModeHTML`; none of the source markdown characters are visible literally in the + rendered message + +#### Scenario: Ampersand, less-than, and greater-than are escaped in prose but not in tags + +- **GIVEN** the agent's outbound text contains the literal string `a < b && b > c` +- **WHEN** `ToTelegramHTML` converts this text +- **THEN** the output is `a < b && b > c`, and this escaping does not corrupt + any ``, ``, ``, ``, or `
` tags the converter itself emitted
+  for other constructs in the same message
+
+#### Scenario: Entity/parse error falls back to no-ParseMode for that chunk only
+
+- **GIVEN** a chunk's converted HTML triggers a `can't parse entities` error from
+  `sendMessage`
+- **WHEN** `Send` processes that chunk
+- **THEN** the adapter retries the same chunk with no `ParseMode` field and the original
+  markdown text, the message is delivered (with visible markdown syntax for that chunk
+  only), and no per-adapter latch is set — the next message's chunks attempt
+  `ParseModeHTML` normally
+
+#### Scenario: Long prose is chunked at the source markdown level, not after conversion
+
+- **GIVEN** the agent's outbound text is 6,000 characters of GFM prose containing a fenced
+  code block that straddles the natural 3,500-character cut point
+- **WHEN** `Send` chunks and converts this text
+- **THEN** the source text is split into two markdown-valid chunks (the fence is closed on
+  the first chunk and reopened with the same info string on the second) before either chunk
+  is converted to HTML, and neither resulting `sendMessage` call exceeds the 4,096-character
+  Telegram limit
+
+## ADDED Requirements
+
+### Requirement: Outbound prose rendering never drops a message on formatting failure
+
+Neither the Slack nor the Telegram adapter MAY drop an outbound message as a consequence of
+a markdown-rendering failure. Every rendering failure path (Slack block-construction/
+API-rejection, Telegram HTML-conversion/entity-parse-rejection) MUST have a defined
+degradation to a simpler, always-valid representation (Slack: plain `MsgOptionText`;
+Telegram: `sendMessage` with no `ParseMode`), and that degraded send MUST be attempted before
+the adapter reports the `Send` call as failed.
+
+#### Scenario: Formatting failure never surfaces as a lost message
+
+- **GIVEN** either adapter's markdown-rendering path fails for any reason (API rejection,
+  conversion bug, or unexpected input)
+- **WHEN** `Send` is called
+- **THEN** the adapter's defined plain-text degradation path is attempted and, if it
+  succeeds, `Send` returns a delivered result; the message is not silently discarded solely
+  because the richer rendering path failed
diff --git a/openspec/changes/bridge-slack-native-markdown/tasks.md b/openspec/changes/bridge-slack-native-markdown/tasks.md
new file mode 100644
index 0000000000..d332a483ae
--- /dev/null
+++ b/openspec/changes/bridge-slack-native-markdown/tasks.md
@@ -0,0 +1,213 @@
+# Tasks: bridge-slack-native-markdown
+
+## 1. Shared `internal/bridge/markdown` package
+
+- [x] 1.1 Create `internal/bridge/markdown/markdown.go` (new package, stdlib-only, no
+  dependency on `internal/bridge` or any platform SDK — importable by both `slack` and
+  `telegram` packages without an import cycle).
+
+- [x] 1.2 Implement `NormalizeSlackLinks(text string) string`: rewrites ``
+  → `[label](https://x)` and bare `` → `https://x`. MUST leave `<@U123>` mentions
+  and non-URL angle-bracket text (e.g. `a < b && b > c`) untouched. Only match content
+  starting with a URL scheme (`http://` / `https://`) inside the angle brackets.
+
+- [x] 1.3 Implement the fence-aware chunker per `design.md § Chunking algorithm`
+  (implemented as `Split(text string, limit, maxChunks int, marker string) (chunks
+  []string, truncated bool)` — named `Split`, not `SplitMarkdown`, and takes an explicit
+  `maxChunks` cap rather than only a budget derivation, per the task's "adjust names for
+  idiomatic Go" latitude):
+  `SplitMarkdown(text string, limit int, marker string) (chunks []string, truncated bool)`
+  (or equivalent signature). Must: (a) prefer blank-line, then heading-line, then
+  any-line-boundary cuts, accepting a semantic boundary only if it retains at least half of
+  `limit`; (b) never cut inside an open ` ``` ` fence — close and reopen the fence across the
+  boundary, preserving the info string; (c) hard-wrap rune-safely (never split a UTF-8
+  codepoint) any single line exceeding `limit` on its own; (d) pack the last chunk to the
+  limit rather than stopping at a tidy boundary; (e) reserve room for `marker` before the
+  final cut when truncating, append the marker, and re-close any fence the final cut broke.
+
+- [x] 1.4 Implement `BuildBlockChunks(text string, payloadBudget, perBlockTarget int,
+  marker string) (chunks []string, truncated bool)` (named `BuildBlockChunks`, not
+  `SplitMarkdownBudget`): derives `blockCount = ceil(payloadBudget/perBlockTarget)` and
+  `perChunkLimit = floor(payloadBudget/blockCount)` (see deviation note below — design.md's
+  own worked example uses floor, not the task text's `ceil(budget/blockCount)`, and floor is
+  required for the `sum(len(chunks)) <= budgetChars` guarantee this item itself demands),
+  per `design.md § Slack budget arithmetic`.
+
+- [x] 1.5 Implement `ToTelegramHTML(text string) string` per `design.md § Telegram GFM → HTML
+  mapping`: extract code spans/fences to placeholders first (protect from inline
+  transformation), convert headings/bold/italic/strikethrough/links/lists/blockquotes/tables/
+  `---`/images per the mapping table, escape `& < >` in prose text runs only, then restore
+  placeholders as escaped ``/`
` content.
+
+- [x] 1.6 Define the truncation marker as an exported constant, e.g.
+  `const TruncationMarker = "\n\n_\u2026truncated\u2026_"` (italic prose).
+
+- [x] 1.7 Unit tests in `internal/bridge/markdown/markdown_test.go` (deviation: `ToTelegramHTML`
+  tests landed in a sibling file, `internal/bridge/markdown/telegram_test.go`, mirroring the
+  `telegram.go`/`markdown.go` file split — same package, same `go test` target):
+  - `NormalizeSlackLinks`: rewrites `` and bare ``; leaves `<@U123>` and
+    `a < b && b > c` untouched; table-driven.
+  - `SplitMarkdown`/chunker: fits-in-one-chunk fast path; blank-line boundary preferred;
+    heading-line boundary when no blank line; line-boundary fallback; fence-spanning split
+    closes and reopens the fence with the same info string; hard-wrap of an oversized single
+    line never splits a multi-byte rune (test with multi-byte UTF-8 content, e.g. emoji or
+    CJK characters, at the wrap boundary); last-chunk packing behavior; truncation marker
+    reserved-room arithmetic (marker never pushes a chunk over `limit`); truncation marker
+    closes a fence broken by the re-cut.
+  - `SplitMarkdownBudget`: derived per-chunk limit arithmetic matches
+    `design.md`'s worked example (12,000 / 3,000-target → 4 chunks of 3,000); sum of chunk
+    lengths never exceeds the budget.
+  - `ToTelegramHTML`: each row of the GFM→HTML mapping table in `design.md` as a distinct
+    test case; escaping test for `& < >` in prose without corrupting emitted tags (the
+    `a < b && b > c` scenario from the spec); code span/fence content is escaped for `&<>`
+    but not further transformed (e.g. underscores inside a code span are not turned into
+    ``).
+
+## 2. Slack adapter: markdown-block prose rendering
+
+- [x] 2.1 In `internal/bridge/slack/adapter.go`'s `Send` (~lines 885-947): replace the
+  `slackgo.MsgOptionText(text, false)` call for the prose text part with construction of one
+  or more `slackgo.NewMarkdownBlock("", chunk)` blocks via
+  `internal/bridge/markdown.BuildBlockChunks` (12,000-char budget, 3,000-char per-block
+  target, `internal/bridge/markdown.TruncationMarker` via the local
+  `markdownTruncationMarker` alias). Pass the resulting blocks via
+  `slackgo.MsgOptionBlocks(...)`. Keep the top-level `text` field set (as the notification
+  fallback) alongside the blocks — do not remove it.
+
+- [x] 2.2 Call `internal/bridge/markdown.NormalizeSlackLinks` on the outbound text before
+  chunking (done inside `BuildBlockChunks` itself), so legacy `` mrkdwn link
+  syntax an agent might emit is rewritten to `[label](url)` before block construction.
+
+- [x] 2.3 Enforce the 50-block-per-message cap defensively (e.g. an assertion/clamp after
+  chunking) so a future change to the budget constants cannot silently exceed Slack's block
+  count ceiling.
+
+- [x] 2.4 Add a per-`Adapter`-instance sticky latch (implemented as
+  `markdownBlocksUnsupported atomic.Bool`, following the existing `lastError`/`lastFailureAt`
+  pattern on `Adapter`). When set, `Send` skips markdown-block construction entirely and
+  calls `slackgo.MsgOptionText` directly.
+
+- [x] 2.5 Detect block-related API errors from `PostMessageContext` (error string/code
+  matching `invalid_blocks`, `invalid_block`, `invalid_arguments`, `blocks_too_long`,
+  `msg_blocks_too_long`, `invalid_block_id` via `isBlockError`). On match: log a one-shot WARN
+  (`logging.Warn("bridge: slack markdown blocks rejected, falling back to plain text", ...)`
+  — only once per latch transition, not per send, via `atomic.Bool.CompareAndSwap`), set the
+  latch, and retry the same logical send as plain text via `MsgOptionText` before returning
+  the `Send` result.
+
+- [x] 2.6 Verify `truncateRunes` (existing, ~lines 973-989) and `MaxTextLength = 39_000`
+  (existing, ~line 28) remain applied to the top-level `text` field exactly as today — this
+  change does not alter the notification-fallback truncation behavior, only the blocks path.
+
+- [x] 2.7 (Follow-up refinement, added during the Telegram half of this change) Split the
+  latch-triggering predicate from the retry-triggering predicate: `isBlockError` (retry-only,
+  keeps `invalid_arguments`) stays broad because a false positive there just costs one extra
+  plain-text send; a new `isBlockCapabilityError` (latch-only, excludes `invalid_arguments`)
+  gates `markdownBlocksUnsupported.CompareAndSwap` so an ambiguous, possibly-unrelated error
+  code can no longer permanently downgrade formatting for the identity. Covered by
+  `TestSendAmbiguousBlockErrorRetriesWithoutLatching` in `adapter_markdown_test.go`.
+
+## 3. Telegram adapter: HTML prose rendering
+
+- [x] 3.1 In `internal/bridge/telegram/adapter.go`'s `Send` (~lines 849-884): replace the
+  `chunkText(text, MaxTextLength)` call (rune-count chunking with no markdown awareness) with
+  `internal/bridge/markdown.SplitMarkdown` at a conservative source-level limit (~3,500
+  characters) plus `internal/bridge/markdown.TruncationMarker` for any content dropped beyond
+  what the adapter chooses to send.
+
+- [x] 3.2 For each source chunk, convert via `internal/bridge/markdown.ToTelegramHTML` and
+  send with `&tgbot.SendMessageParams{ChatID: chatID, Text: htmlChunk, ParseMode:
+  models.ParseModeHTML}` — replacing the current no-`ParseMode` call.
+
+- [x] 3.3 Detect entity/parse errors from `SendMessage` (error string matching `can't parse
+  entities` or equivalent go-telegram/bot error surface). On match: retry the same chunk
+  (original markdown text, unconverted) with no `ParseMode` field set, before returning the
+  `Send` result for that chunk. Do NOT set any sticky/latched state — every message's chunks
+  attempt `ParseModeHTML` fresh.
+
+- [x] 3.4 Confirm `chunkText` (existing, ~lines 926-960) is either removed if no longer used
+  by any caller, or retained only if still referenced elsewhere (e.g. by
+  `SendQueuedAck`/`UpdateQueuedAck`, which per the proposal's out-of-scope section stay plain
+  text) — check all call sites before deleting. (`chunkText`'s only caller was `Send`; removed.
+  `SendQueuedAck`/`UpdateQueuedAck` were not routed through it and remain unchanged.)
+
+- [x] 3.5 Verify `MaxTextLength = 4096` (existing, ~line 31) is still respected as the hard
+  ceiling for each `sendMessage` call's *post-conversion* text — the ~3,500-character
+  source-level chunk limit exists specifically to keep the converted output under this cap
+  with margin for HTML tag overhead.
+
+## 4. Tests: Slack adapter
+
+- [x] 4.1 `internal/bridge/slack/adapter_test.go` (existing file, add cases) or a new
+  `internal/bridge/slack/adapter_markdown_test.go`:
+  - `Send` with GFM prose (headings, bold, list, link, fenced code) produces a
+    `chat.postMessage` call whose blocks include `markdown`-type blocks with the expected
+    (link-normalized) text; assert against the mock/httptest Slack API used by existing
+    adapter tests.
+  - `Send` with text exceeding the 12,000-character budget produces blocks whose combined
+    length stays within budget and whose last block ends with `TruncationMarker`.
+  - `Send` where the mock API returns an `invalid_blocks`-class error on the blocks attempt:
+    assert a subsequent plain-text `MsgOptionText` call is made and `Send` reports delivered.
+  - A second `Send` call on the same `Adapter` after the latch is set: assert no blocks
+    attempt is made (mock records only a plain-text call), and no duplicate WARN log fires
+    for the same latch.
+  - `Send` with `` legacy link syntax: assert the posted block text contains
+    `[label](url)` (or the equivalent post-normalization form), not the raw mrkdwn syntax.
+
+## 5. Tests: Telegram adapter
+
+- [x] 5.1 `internal/bridge/telegram/adapter_test.go` (existing file, add cases) or a new
+  `internal/bridge/telegram/adapter_markdown_test.go` (landed as the new file):
+  - `Send` with GFM prose produces a `sendMessage` call with `ParseMode: ParseModeHTML` and
+    text matching the expected HTML mapping (bold heading, ``, ``, `
`, etc.) per the mapping table in `design.md`.
+  - `Send` with text longer than the ~3,500-character source chunk limit, containing a fence
+    that straddles the cut, produces two (or more) `sendMessage` calls, each independently
+    valid HTML, each under Telegram's 4,096-character cap, with the fence correctly closed on
+    the first chunk and reopened on the second.
+  - `Send` where the mock bot API returns a `can't parse entities`-class error for one chunk:
+    assert a retry `sendMessage` call is made for that chunk with no `ParseMode` field and
+    the original markdown text; assert the *next* message's `Send` still attempts
+    `ParseModeHTML` (no sticky latch).
+  - Escaping test: outbound text containing literal `<`, `>`, `&` outside of any markdown
+    construct is escaped correctly in the sent HTML without corrupting adjacent emitted tags.
+
+## 6. Confirm out-of-scope paths are untouched
+
+- [x] 6.1 Confirm (via `git diff` review, not code edits) that
+  `internal/bridge/slack/render.go` (the `RichRenderer` Block Kit path for tool calls,
+  lists, tables, status) has zero changes.
+- [x] 6.2 Confirm `internal/bridge/telegram/render.go` (the `RichRenderer` path using
+  `ParseModeMarkdownV1`) has zero changes.
+- [x] 6.3 Confirm `internal/bridge/mattermost/adapter.go` has zero changes.
+- [x] 6.4 Confirm `SendQueuedAck` / `UpdateQueuedAck` on both the Slack and Telegram adapters
+  (`slack/adapter.go:994-1033`, and the Telegram equivalent) are NOT routed through the new
+  markdown rendering — they continue sending plain short status strings unchanged.
+- [x] 6.5 Confirm `internal/bridge/service/dispatch.go`'s `handleTerminalEvent` and
+  `agentMessageText` (~lines 585-640) have zero changes — this change only affects how each
+  adapter's `Send` renders the `Outbound.Text` it already receives.
+- [x] 6.6 Confirm no `Config` struct field, `cmd/schema/main.go` entry, or
+  `opencode-schema.json` change is introduced — this change has no `.opencode.json` surface.
+  No schema regeneration step is required because no `Config` field changes.
+
+## 7. Tests + Verification
+
+- [x] 7.1 `go test -short ./internal/bridge/...` passes, including the new
+  `internal/bridge/markdown` package tests and the updated Slack adapter tests (Telegram
+  adapter tests, including the new `ToTelegramHTML`/`Send` HTML-path coverage, pass as-is).
+- [x] 7.2 `go test -race ./internal/bridge/...` passes (no data race from the new
+  per-adapter `atomic.Bool` latch or any shared state in `internal/bridge/markdown`).
+- [x] 7.3 `make test` run for this change; see the workhorse report for the verbatim output.
+  `opencode-schema.json` and generated mocks are unchanged (`git status --short` confirms —
+  this change introduces no `Config` field).
+- [x] 7.4 `./scripts/check_hidden_chars.sh` passes (no hidden Unicode introduced by the
+  truncation marker's ellipsis character or any other new string constants — verify the
+  marker uses an explicit `\u2026` escape rather than a literal `…` glyph, or confirm the
+  script's allowlist covers it).
+- [x] 7.5 `go vet ./internal/bridge/...` clean.
+
+## 8. Docs
+
+- [x] 8.1 `docs/bridge.md`: document outbound prose rendering per platform (Slack Block Kit
+  `markdown` blocks + latch, Telegram HTML + per-chunk fallback, Mattermost unchanged) and note
+  the `RichRenderer` tool-card paths are separate and untouched.

From 7ad7948f6557ac9359e0b73a8f7fad015af238e4 Mon Sep 17 00:00:00 2001
From: Artem Obukhov 
Date: Mon, 7 Sep 2026 18:14:04 +0400
Subject: [PATCH 2/2] =?UTF-8?q?fix(bridge):=20review=20fixes=20=E2=80=94?=
 =?UTF-8?q?=20chunker=20termination,=20Telegram=20entity=20nesting,=20href?=
 =?UTF-8?q?=20escaping?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Three defects found reviewing the native-markdown change, plus the test and
doc gaps that let them through.

markdown.Split could loop forever. The synthetic fence-reopen delimiter is
pushed BACK onto the unconsumed remainder, so the loop only terminates when a
pass consumes strictly more than it pushes back. A fence whose info string
rivals the chunk limit broke that and grew `remaining` without bound; a limit
smaller than the closing delimiter shrank the cut to zero and made no progress
at all. Reproduced at Telegram's real 3,500 limit, so a single agent message
could hang the send goroutine and leak memory. Fixed with an unconditional
forward-progress guard plus a 64-rune clamp on the info string carried into
the synthetic delimiter (the original opening line is still emitted verbatim).
Content is always preserved; only the fence's highlighting degrades.

ToTelegramHTML emitted code/pre nested inside other entities. Telegram's
nesting rules say bold/italic/underline/strikethrough/spoiler "can contain and
can be part of any other entities, except pre and code", and "all other
entities can't contain each other" — the only legal nesting is
. A heading containing a code span
("## Fix `foo.go`", ubiquitous in agent prose) produced ......,
which Telegram rejects, dropping the whole chunk to raw markdown. Code spans
landing inside a heading, emphasis, link label, blockquote or table cell are
now flattened to escaped plain text.

Link URLs were not attribute-escaped. escapeHTML covers only &<>, so a URL
containing a double quote closed the href early and injected an unsupported
attribute. Added escapeAttr (" -> ", a documented named entity).

Also: the 3,500-limit comment claimed HTML conversion can only shrink the
text. Telegram measures its cap after entities parsing, so tags and escaping
do not count — but a `---` rule expands to 10 em-dashes, and rule-dominated
text still breaches 4,096. Corrected the comment and added "message is too
long" to isParseError so that case degrades to plain text instead of dropping
the message.

The Slack latch is race-free (atomic.Bool) but no test called Send
concurrently, so -race proved nothing about it. Added a concurrent-Send test;
one Adapter is shared by every session bound to an identity.

Regression coverage for all three defects. docs/bridge.md, the delta spec and
design.md corrected — two spec claims were factually false (post-conversion
length was asserted as bounded, and the termination invariant was never
stated).

Tests: go test -race ./internal/bridge/... green (522); make test green;
gofmt/vet clean; openspec validate --all --strict 34 passed.
---
 docs/bridge.md                                |   2 +-
 internal/bridge/markdown/markdown.go          |  65 ++++-
 .../bridge/markdown/markdown_progress_test.go | 126 +++++++++
 internal/bridge/markdown/telegram.go          | 149 ++++++++---
 .../bridge/markdown/telegram_nesting_test.go  | 181 +++++++++++++
 .../bridge/slack/adapter_concurrency_test.go  | 104 ++++++++
 internal/bridge/telegram/adapter.go           |  31 ++-
 .../bridge/telegram/adapter_markdown_test.go  |  34 +++
 .../.openspec.yaml                            |   0
 .../design.md                                 |  14 +
 .../proposal.md                               |   0
 .../specs/chat-bridge-adapters/spec.md        |  81 +++++-
 .../tasks.md                                  |  17 ++
 openspec/specs/chat-bridge-adapters/spec.md   | 248 +++++++++++++++++-
 14 files changed, 985 insertions(+), 67 deletions(-)
 create mode 100644 internal/bridge/markdown/markdown_progress_test.go
 create mode 100644 internal/bridge/markdown/telegram_nesting_test.go
 create mode 100644 internal/bridge/slack/adapter_concurrency_test.go
 rename openspec/changes/{bridge-slack-native-markdown => archive/2026-09-07-bridge-slack-native-markdown}/.openspec.yaml (100%)
 rename openspec/changes/{bridge-slack-native-markdown => archive/2026-09-07-bridge-slack-native-markdown}/design.md (94%)
 rename openspec/changes/{bridge-slack-native-markdown => archive/2026-09-07-bridge-slack-native-markdown}/proposal.md (100%)
 rename openspec/changes/{bridge-slack-native-markdown => archive/2026-09-07-bridge-slack-native-markdown}/specs/chat-bridge-adapters/spec.md (72%)
 rename openspec/changes/{bridge-slack-native-markdown => archive/2026-09-07-bridge-slack-native-markdown}/tasks.md (92%)

diff --git a/docs/bridge.md b/docs/bridge.md
index 448338f3fe..e2b1af229b 100644
--- a/docs/bridge.md
+++ b/docs/bridge.md
@@ -195,7 +195,7 @@ A relay channel with **no chat platform of its own**. Outbound messages and ques
 Agent replies are authored as GFM (GitHub-flavored Markdown) — headings, bold/italic, links, lists, tables, fenced code. Each adapter's `Send` renders that same `Outbound.Text` into whatever markup dialect its platform actually understands, with automatic degradation to plain text if rendering is rejected. The shared, stdlib-only chunking and conversion helpers live in `internal/bridge/markdown`.
 
 - **Slack**: rendered as Block Kit `markdown` blocks (real GFM parsing, unlike the legacy mrkdwn `text` field, which cannot represent headings, tables, or fenced code with syntax highlighting). Text is chunked to Slack's 12,000-character cumulative budget across all blocks in one payload (`internal/bridge/markdown.BuildBlockChunks`, 3,000-char per-block target). The top-level `text` field is still sent alongside the blocks as the notification/accessibility fallback. If Slack rejects the blocks with an unambiguous block-capability error (`invalid_blocks`, `invalid_block`, `blocks_too_long`, `msg_blocks_too_long`, `invalid_block_id`), the adapter retries as plain text and sets a sticky per-identity latch so subsequent sends skip the blocks attempt entirely. A more ambiguous error (`invalid_arguments`, which Slack also returns for unrelated reasons) still retries as plain text for that one send but does **not** latch — the next send tries blocks again.
-- **Telegram**: rendered as Telegram HTML (`internal/bridge/markdown.ToTelegramHTML`), the restricted tag set Telegram's `ParseMode: HTML` supports (``, ``, ``, ``/`
`, `
`, etc.) — chosen over MarkdownV2 because it requires escaping only three characters instead of ~18 reserved ones. Source markdown is chunked at 3,500 characters (`MarkdownChunkLimit`) before conversion, conservative headroom under Telegram's 4,096-character post-parse cap to absorb HTML tag overhead and `&`-style escaping. If a chunk's HTML is rejected with a parse/entity error, that one chunk is retried unformatted (no `ParseMode`) — there is no sticky latch, since a parse failure is specific to that chunk's content, not a platform capability. +- **Telegram**: rendered as Telegram HTML (`internal/bridge/markdown.ToTelegramHTML`), the restricted tag set Telegram's `ParseMode: HTML` supports (``, ``, ``, ``/`
`, `
`, etc.) — chosen over MarkdownV2 because it requires escaping only three characters instead of ~18 reserved ones. Telegram also forbids `code`/`pre` entities nested inside any other entity (the sole exception being `
`), so a code span landing inside a heading, emphasis, link label, blockquote or table cell is emitted as plain escaped text rather than a nested `` tag. Source markdown is chunked at 3,500 characters (`MarkdownChunkLimit`) before conversion — headroom under Telegram's 4,096-character *post-parse* cap, which HTML tags and `&`-style escaping do not count against. If a chunk's HTML is rejected with a parse/entity error — or with `message is too long`, which the `---` → 10-em-dash expansion can still provoke — that one chunk is retried unformatted (no `ParseMode`) — there is no sticky latch, since a parse failure is specific to that chunk's content, not a platform capability.
 - **Mattermost**: unchanged — `Post.Message` is sent as native GFM and Mattermost's own server-side parser already renders it correctly.
 
 The `RichRenderer` tool-card paths (`internal/bridge/slack/render.go`, `internal/bridge/telegram/render.go`) that hand-author Block Kit / legacy Markdown for tool calls, lists, tables, and status previews are separate code paths, untouched by the above — they compose their own markup directly rather than converting agent-authored GFM.
diff --git a/internal/bridge/markdown/markdown.go b/internal/bridge/markdown/markdown.go
index 4bb406c13b..06b829914e 100644
--- a/internal/bridge/markdown/markdown.go
+++ b/internal/bridge/markdown/markdown.go
@@ -1,10 +1,11 @@
 // Package markdown provides shared, stdlib-only markdown utilities used by
-// the chat-bridge adapters (Slack today; Telegram in a follow-up change).
-// It has no dependency on internal/bridge or any platform SDK — mirroring
-// the dependency-free discipline of internal/bridge itself — so it can be
-// imported by every platform adapter package without an import cycle.
+// the chat-bridge adapters (Slack and Telegram). It has no dependency on
+// internal/bridge or any platform SDK — mirroring the dependency-free
+// discipline of internal/bridge itself — so it can be imported by every
+// platform adapter package without an import cycle.
 //
-// Two responsibilities live here:
+// Two responsibilities live here (a third, the GFM -> Telegram-HTML
+// converter, lives in telegram.go):
 //
 //   - NormalizeSlackLinks rewrites Slack mrkdwn's own link syntax
 //     ( / ) into standard Markdown
@@ -81,6 +82,32 @@ type fenceState struct {
 	info      string
 }
 
+// maxFenceInfoRunes bounds the info string retained for the purpose of
+// REOPENING a fence on the far side of a chunk boundary. A real info string
+// is a language tag ("go", "json", "python"); text far longer than that is
+// not a tag at all, and carrying it verbatim into the synthetic reopen
+// delimiter would let the delimiter rival the chunk limit itself — which
+// made Split spin forever, since the reopen text is pushed back onto the
+// unconsumed remainder. Clamping affects only the synthetic delimiter; the
+// original opening line is always emitted untouched.
+const maxFenceInfoRunes = 64
+
+// clampRunes truncates s to at most n runes, always cutting on a codepoint
+// boundary.
+func clampRunes(s string, n int) string {
+	if n <= 0 {
+		return ""
+	}
+	count := 0
+	for i := range s {
+		if count == n {
+			return s[:i]
+		}
+		count++
+	}
+	return s
+}
+
 // scanFence walks s line by line, updating fs for every fence-toggling line
 // it finds (a line whose left-trimmed content starts with 3+ backticks).
 // It is meant to be called incrementally, once per emitted chunk, so state
@@ -95,7 +122,7 @@ func scanFence(fs *fenceState, s string) {
 		if !fs.open {
 			fs.open = true
 			fs.markerLen = n
-			fs.info = strings.TrimSpace(trimmed[n:])
+			fs.info = clampRunes(strings.TrimSpace(trimmed[n:]), maxFenceInfoRunes)
 			continue
 		}
 		// Already inside a fence: only a run at least as long as the
@@ -290,6 +317,32 @@ func Split(text string, limit, maxChunks int, marker string) ([]string, bool) {
 
 		cut := findBoundary(remaining, limit)
 		head, newFS, reopen, usedCut := buildChunk(remRunes, cut, limit, 0, fs)
+
+		// Forward-progress guarantee. `reopen` is synthetic text pushed
+		// BACK onto the unconsumed remainder, so this loop only terminates
+		// if every pass consumes strictly more than it pushes back. Two
+		// pathological shapes break that: a cut shrunk all the way to zero
+		// (a limit smaller than the fence delimiter itself), and a reopen
+		// delimiter at least as long as the slice consumed. Either one used
+		// to spin forever while growing `remaining` without bound. Give up
+		// the close/reopen fixup for this one boundary instead: the content
+		// is still emitted verbatim and in full, only the fence's syntax
+		// highlighting is lost across the split.
+		if usedCut <= 0 || utf8.RuneCountInString(reopen) >= usedCut {
+			hardCut := cut
+			if hardCut < 1 {
+				hardCut = 1
+			}
+			if hardCut > len(remRunes) {
+				hardCut = len(remRunes)
+			}
+			head = string(remRunes[:hardCut])
+			newFS = fs
+			scanFence(&newFS, head)
+			reopen = ""
+			usedCut = hardCut
+		}
+
 		chunks = append(chunks, head)
 		fs = newFS
 		remaining = reopen + string(remRunes[usedCut:])
diff --git a/internal/bridge/markdown/markdown_progress_test.go b/internal/bridge/markdown/markdown_progress_test.go
new file mode 100644
index 0000000000..7192ee02f9
--- /dev/null
+++ b/internal/bridge/markdown/markdown_progress_test.go
@@ -0,0 +1,126 @@
+package markdown
+
+import (
+	"strings"
+	"testing"
+	"time"
+	"unicode/utf8"
+)
+
+// splitWithTimeout runs Split on a watchdog. Split used to be able to spin
+// forever on inputs where the synthetic fence-reopen delimiter was at least
+// as long as the slice consumed per pass, so a plain call would hang the
+// whole test binary rather than fail it.
+func splitWithTimeout(t *testing.T, text string, limit, maxChunks int, marker string) []string {
+	t.Helper()
+	type result struct {
+		chunks []string
+	}
+	done := make(chan result, 1)
+	go func() {
+		c, _ := Split(text, limit, maxChunks, marker)
+		done <- result{chunks: c}
+	}()
+	select {
+	case r := <-done:
+		return r.chunks
+	case <-time.After(5 * time.Second):
+		t.Fatalf("Split did not terminate within 5s (limit=%d, maxChunks=%d) — forward-progress regression", limit, maxChunks)
+		return nil
+	}
+}
+
+// TestSplitTerminatesOnLongFenceInfoString covers the non-termination bug:
+// a fence whose info string rivals the chunk limit produced a reopen
+// delimiter longer than the content consumed, so `remaining` grew every
+// pass instead of shrinking.
+func TestSplitTerminatesOnLongFenceInfoString(t *testing.T) {
+	cases := []struct {
+		name  string
+		text  string
+		limit int
+	}{
+		{
+			name:  "info string longer than limit",
+			text:  "```" + strings.Repeat("x", 200) + "\ncode body here\n```\ntail",
+			limit: 100,
+		},
+		{
+			name:  "info string near the telegram limit",
+			text:  "```" + strings.Repeat("x", 3600) + "\ncode\n```\ntail",
+			limit: 3500,
+		},
+		{
+			name:  "limit smaller than the fence delimiter itself",
+			text:  "````\nabc",
+			limit: 4,
+		},
+		{
+			name:  "limit of one rune with an open fence",
+			text:  "```go\nabcdef\n```",
+			limit: 1,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			chunks := splitWithTimeout(t, tc.text, tc.limit, 0, TruncationMarker)
+			if len(chunks) == 0 {
+				t.Fatal("Split returned no chunks")
+			}
+			for i, c := range chunks {
+				if !utf8.ValidString(c) {
+					t.Errorf("chunk %d is not valid UTF-8: %q", i, c)
+				}
+				if n := utf8.RuneCountInString(c); n > tc.limit {
+					t.Errorf("chunk %d has %d runes, limit is %d: %q", i, n, tc.limit, c)
+				}
+			}
+		})
+	}
+}
+
+// TestSplitPreservesEveryWordOnLongFenceInfoString asserts the
+// non-termination fix degrades formatting only — never content.
+func TestSplitPreservesEveryWordOnLongFenceInfoString(t *testing.T) {
+	body := "alpha bravo charlie delta echo foxtrot golf hotel india juliet"
+	text := "```" + strings.Repeat("x", 200) + "\n" + body + "\n```\ntail words here"
+
+	chunks := splitWithTimeout(t, text, 100, 0, TruncationMarker)
+
+	joined := strings.Join(chunks, "")
+	for _, w := range strings.Fields(body + " tail words here") {
+		if !strings.Contains(joined, w) {
+			t.Errorf("word %q lost across chunk boundaries; joined = %q", w, joined)
+		}
+	}
+}
+
+// TestSplitClampsReopenedFenceInfoString documents that the info string is
+// clamped only on the SYNTHETIC reopen delimiter — the original opening
+// line always survives verbatim.
+func TestSplitClampsReopenedFenceInfoString(t *testing.T) {
+	info := strings.Repeat("y", maxFenceInfoRunes+40)
+	text := "```" + info + "\n" + strings.Repeat("code line\n", 40) + "```"
+
+	chunks := splitWithTimeout(t, text, 200, 0, TruncationMarker)
+	if len(chunks) < 2 {
+		t.Fatalf("expected the input to split, got %d chunk(s)", len(chunks))
+	}
+	if !strings.Contains(chunks[0], "```"+info) {
+		t.Errorf("first chunk lost the original opening delimiter: %q", chunks[0])
+	}
+	for i, c := range chunks[1:] {
+		for _, line := range strings.Split(c, "\n") {
+			trimmed := strings.TrimLeft(line, " \t")
+			n := countLeadingBackticks(trimmed)
+			if n < 3 {
+				continue
+			}
+			if got := utf8.RuneCountInString(strings.TrimSpace(trimmed[n:])); got > maxFenceInfoRunes {
+				t.Errorf("chunk %d reopened a fence with a %d-rune info string, max is %d",
+					i+1, got, maxFenceInfoRunes)
+			}
+		}
+	}
+}
diff --git a/internal/bridge/markdown/telegram.go b/internal/bridge/markdown/telegram.go
index ffe3512ff8..0f994cd22f 100644
--- a/internal/bridge/markdown/telegram.go
+++ b/internal/bridge/markdown/telegram.go
@@ -22,6 +22,24 @@ var (
 	inlineCodeRe = regexp.MustCompile("`([^`\n]+)`")
 )
 
+// placeholder holds the two renderings of one extracted code construct.
+//
+// html is the real Telegram tag (.../
...), used when the
+// construct sits at the top level of the message. plain is the same
+// content escaped but UNTAGGED, used when the construct would otherwise
+// land inside another entity: the Bot API's "Formatting options" nesting
+// rules state that bold/italic/underline/strikethrough/spoiler entities
+// "can contain and can be part of any other entities, EXCEPT pre and
+// code", and that "all other entities can't contain each other". A
+//  nested in  (e.g. the very common "## Fix `foo.go`" heading),
+// in 
 (a table cell holding a code span) or in  is therefore
+// rejected by Telegram with "can't parse entities", costing the whole
+// chunk its formatting. Flattening to plain keeps the message valid.
+type placeholder struct {
+	html  string
+	plain string
+}
+
 // ToTelegramHTML converts s from GFM markdown to Telegram-safe HTML.
 //
 // Implementation shape (see design.md): code spans and fenced code
@@ -31,7 +49,8 @@ var (
 // tables, horizontal rules) and inline transforms (bold/italic/strike/
 // links) run on what remains, escaping "&<>" in plain-text runs as they
 // go. Finally the placeholders are restored as escaped /
-// content.
+// content — except where a placeholder ended up inside another entity,
+// in which case it is flattened to plain escaped text (see placeholder).
 //
 // Never panics; unbalanced markers (unclosed "**", "[label](" with no
 // closing paren) are left as literal escaped text rather than emitting a
@@ -43,11 +62,11 @@ func ToTelegramHTML(s string) string {
 	if s == "" {
 		return ""
 	}
-	placeholders := map[string]string{}
+	placeholders := map[string]placeholder{}
 	counter := 0
 	withoutFences := extractFences(s, &counter, placeholders)
 	withoutCode := extractInlineCode(withoutFences, &counter, placeholders)
-	converted := convertBlocks(withoutCode)
+	converted := convertBlocks(withoutCode, placeholders)
 	return restorePlaceholders(converted, placeholders)
 }
 
@@ -61,14 +80,30 @@ func newPlaceholder(counter *int) string {
 	return "\x00md" + strconv.Itoa(*counter) + "\x00"
 }
 
-// restorePlaceholders replaces every placeholder token with its final
-// (already-escaped) HTML content.
-func restorePlaceholders(s string, placeholders map[string]string) string {
+// restorePlaceholders replaces every placeholder token still present with
+// its final (already-escaped) tagged HTML content. Tokens that were
+// flattened earlier by flattenPlaceholders are already gone by this point.
+func restorePlaceholders(s string, placeholders map[string]placeholder) string {
 	if len(placeholders) == 0 {
 		return s
 	}
-	for token, html := range placeholders {
-		s = strings.ReplaceAll(s, token, html)
+	for token, ph := range placeholders {
+		s = strings.ReplaceAll(s, token, ph.html)
+	}
+	return s
+}
+
+// flattenPlaceholders replaces every placeholder token in s with its
+// UNTAGGED escaped content. Callers apply it to text they are about to
+// wrap in an entity tag (, , , , 
, 
), because +// Telegram forbids code/pre entities nested inside any other entity — +// see the placeholder type for the exact rule and the failure mode. +func flattenPlaceholders(s string, placeholders map[string]placeholder) string { + if len(placeholders) == 0 || !strings.Contains(s, "\x00") { + return s + } + for token, ph := range placeholders { + s = strings.ReplaceAll(s, token, ph.plain) } return s } @@ -79,7 +114,7 @@ func restorePlaceholders(s string, placeholders map[string]string) string { // placeholder line. A fence with no matching close consumes the rest of // the text as its content rather than being left unrecognized, so the // eventual restoration always emits a well-formed, closed
.
-func extractFences(s string, counter *int, placeholders map[string]string) string {
+func extractFences(s string, counter *int, placeholders map[string]placeholder) string {
 	if !strings.Contains(s, "```") {
 		return s
 	}
@@ -120,7 +155,10 @@ func extractFences(s string, counter *int, placeholders map[string]string) strin
 		}
 
 		token := newPlaceholder(counter)
-		placeholders[token] = renderFencedCode(lang, content)
+		placeholders[token] = placeholder{
+			html:  renderFencedCode(lang, content),
+			plain: escapeHTML(content),
+		}
 		out = append(out, token)
 		i = next
 	}
@@ -140,14 +178,17 @@ func renderFencedCode(lang, content string) string {
 // extractInlineCode replaces every `code` span (single-backtick,
 // single-line) with a placeholder token holding its escaped 
 // content.
-func extractInlineCode(s string, counter *int, placeholders map[string]string) string {
+func extractInlineCode(s string, counter *int, placeholders map[string]placeholder) string {
 	if !strings.Contains(s, "`") {
 		return s
 	}
 	return inlineCodeRe.ReplaceAllStringFunc(s, func(m string) string {
 		content := m[1 : len(m)-1]
 		token := newPlaceholder(counter)
-		placeholders[token] = "" + escapeHTML(content) + ""
+		placeholders[token] = placeholder{
+			html:  "" + escapeHTML(content) + "",
+			plain: escapeHTML(content),
+		}
 		return token
 	})
 }
@@ -155,7 +196,7 @@ func extractInlineCode(s string, counter *int, placeholders map[string]string) s
 // convertBlocks applies the line-oriented GFM->HTML mapping (tables,
 // blockquotes, headings, horizontal rules, task/unordered list items)
 // and, for every other line, the inline mapping via processInline.
-func convertBlocks(s string) string {
+func convertBlocks(s string, ph map[string]placeholder) string {
 	lines := strings.Split(s, "\n")
 	out := make([]string, 0, len(lines))
 	i := 0
@@ -170,7 +211,7 @@ func convertBlocks(s string) string {
 			}
 			if j-i >= 2 {
 				run := strings.Join(lines[i:j], "\n")
-				out = append(out, "
"+escapeHTML(run)+"
") + out = append(out, "
"+flattenPlaceholders(escapeHTML(run), ph)+"
") i = j continue } @@ -188,16 +229,17 @@ func convertBlocks(s string) string { } inner := strings.TrimPrefix(t, ">") inner = strings.TrimPrefix(inner, " ") - quoteLines = append(quoteLines, processInline(inner)) + quoteLines = append(quoteLines, processInline(inner, ph)) j++ } - out = append(out, "
"+strings.Join(quoteLines, "\n")+"
") + body := flattenPlaceholders(strings.Join(quoteLines, "\n"), ph) + out = append(out, "
"+body+"
") i = j continue } if m := headingRe.FindStringSubmatch(line); m != nil { - out = append(out, ""+processInline(m[1])+"") + out = append(out, ""+flattenPlaceholders(processInline(m[1], ph), ph)+"") i++ continue } @@ -213,18 +255,18 @@ func convertBlocks(s string) string { if strings.EqualFold(m[2], "x") { box = "\u2611" } - out = append(out, m[1]+box+" "+processInline(m[3])) + out = append(out, m[1]+box+" "+processInline(m[3], ph)) i++ continue } if m := ulItemRe.FindStringSubmatch(line); m != nil { - out = append(out, m[1]+"\u2022 "+processInline(m[2])) + out = append(out, m[1]+"\u2022 "+processInline(m[2], ph)) i++ continue } - out = append(out, processInline(line)) + out = append(out, processInline(line, ph)) i++ } return strings.Join(out, "\n") @@ -243,8 +285,10 @@ func isTableLine(line string) bool { // text run it does not otherwise transform. Delimiters with no matching // close are emitted as literal (escaped) text rather than an unclosed // tag. Bold/italic/strikethrough content is reprocessed recursively so -// nesting (e.g. italic inside bold) round-trips correctly. -func processInline(s string) string { +// nesting (e.g. italic inside bold) round-trips correctly; code +// placeholders inside any emitted tag are flattened to plain text, +// because Telegram rejects code/pre nested in another entity. +func processInline(s string, ph map[string]placeholder) string { var b strings.Builder i := 0 n := len(s) @@ -254,13 +298,9 @@ func processInline(s string) string { case c == '!' && i+1 < n && s[i+1] == '[': if label, url, newPos, ok := matchLink(s, i+1); ok { if label == "" { - b.WriteString(escapeHTML(url)) + b.WriteString(flattenPlaceholders(escapeHTML(url), ph)) } else { - b.WriteString(`
`) - b.WriteString(escapeHTML(label)) - b.WriteString(``) + writeLink(&b, label, url, ph) } i = newPos continue @@ -270,11 +310,7 @@ func processInline(s string) string { case c == '[': if label, url, newPos, ok := matchLink(s, i); ok { - b.WriteString(``) - b.WriteString(escapeHTML(label)) - b.WriteString(``) + writeLink(&b, label, url, ph) i = newPos continue } @@ -285,7 +321,7 @@ func processInline(s string) string { marker := s[i : i+2] if inner, newPos, ok := matchDelim(s, i, marker); ok { b.WriteString("") - b.WriteString(processInline(inner)) + b.WriteString(flattenPlaceholders(processInline(inner, ph), ph)) b.WriteString("") i = newPos continue @@ -296,7 +332,7 @@ func processInline(s string) string { case i+1 < n && s[i:i+2] == "~~": if inner, newPos, ok := matchDelim(s, i, "~~"); ok { b.WriteString("") - b.WriteString(processInline(inner)) + b.WriteString(flattenPlaceholders(processInline(inner, ph), ph)) b.WriteString("") i = newPos continue @@ -307,7 +343,7 @@ func processInline(s string) string { case c == '*': if inner, newPos, ok := matchDelim(s, i, "*"); ok { b.WriteString("") - b.WriteString(processInline(inner)) + b.WriteString(flattenPlaceholders(processInline(inner, ph), ph)) b.WriteString("") i = newPos continue @@ -325,7 +361,7 @@ func processInline(s string) string { rightOK := newPos >= n || !isWordByte(s[newPos]) if rightOK { b.WriteString("") - b.WriteString(processInline(inner)) + b.WriteString(flattenPlaceholders(processInline(inner, ph), ph)) b.WriteString("") i = newPos continue @@ -352,6 +388,21 @@ func processInline(s string) string { return b.String() } +// writeLink emits one label. The URL goes through +// escapeAttr (not escapeHTML) because it lands inside a double-quoted +// attribute: a URL containing `"` would otherwise close the attribute +// early and inject arbitrary attributes into the tag, which Telegram +// rejects as an unsupported tag shape. Label and URL are additionally +// flattened so a code span inside either never becomes a nested +// in the entity. +func writeLink(b *strings.Builder, label, url string, ph map[string]placeholder) { + b.WriteString(``) + b.WriteString(flattenPlaceholders(escapeHTML(label), ph)) + b.WriteString(``) +} + // matchDelim looks for marker again, starting right after the opening // occurrence at pos, and returns the text between them plus the index // just past the closing occurrence. ok is false when no closing @@ -423,3 +474,27 @@ func escapeHTML(s string) string { } return b.String() } + +// escapeAttr escapes a value destined for a double-quoted HTML attribute: +// everything escapeHTML handles, plus '"' -> """ so the value can +// never terminate the attribute early. " is one of the four named +// entities the Bot API documents as supported. +func escapeAttr(s string) string { + var b strings.Builder + b.Grow(len(s)) + for i := 0; i < len(s); i++ { + switch s[i] { + case '&': + b.WriteString("&") + case '<': + b.WriteString("<") + case '>': + b.WriteString(">") + case '"': + b.WriteString(""") + default: + b.WriteByte(s[i]) + } + } + return b.String() +} diff --git a/internal/bridge/markdown/telegram_nesting_test.go b/internal/bridge/markdown/telegram_nesting_test.go new file mode 100644 index 0000000000..620c015f29 --- /dev/null +++ b/internal/bridge/markdown/telegram_nesting_test.go @@ -0,0 +1,181 @@ +package markdown + +import ( + "strings" + "testing" +) + +// Telegram's Bot API nesting rules (see "Formatting options"): +// +// bold, italic, underline, strikethrough, and spoiler entities can +// contain and can be part of any other entities, except pre and code. +// [...] All other entities can't contain each other. +// +// So a or
 inside ANY other tag is rejected with "can't parse
+// entities", which costs the whole chunk its formatting via the plain-text
+// retry. These tests pin that no such nesting is ever emitted.
+func TestToTelegramHTMLNeverNestsCodeInsideAnotherEntity(t *testing.T) {
+	cases := []struct {
+		name string
+		in   string
+	}{
+		{"heading with inline code", "## Fix `foo.go` now"},
+		{"heading with code and bold", "### Use `--flag` for **speed**"},
+		{"bold wrapping inline code", "**run `make test` first**"},
+		{"italic wrapping inline code", "*see `config.yaml`*"},
+		{"underscore italic wrapping code", "_see `config.yaml` here_"},
+		{"strikethrough wrapping code", "~~old `api.Call()` removed~~"},
+		{"link label containing code", "[`pkg.Func`](https://example.com)"},
+		{"blockquote with inline code", "> note: run `go vet`\n> then build"},
+		{"table cell with inline code", "| col | val |\n| --- | --- |\n| `code` | x |"},
+		{"nested bold inside italic with code", "*outer **inner `c`** tail*"},
+		{"list item with code (top level, allowed)", "- run `make test`"},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			got := ToTelegramHTML(tc.in)
+			assertNoNestedCodeOrPre(t, got)
+		})
+	}
+}
+
+// assertNoNestedCodeOrPre walks html and fails if a  or 
 open
+// tag appears while any other tag is still open, or if any tag is open
+// inside a /
. The one exception is the documented
+// `
` language-specifier form.
+func assertNoNestedCodeOrPre(t *testing.T, html string) {
+	t.Helper()
+	var stack []string
+	i := 0
+	for i < len(html) {
+		lt := strings.IndexByte(html[i:], '<')
+		if lt < 0 {
+			return
+		}
+		i += lt
+		gt := strings.IndexByte(html[i:], '>')
+		if gt < 0 {
+			t.Fatalf("unterminated tag at byte %d in %q", i, html)
+		}
+		raw := html[i+1 : i+gt]
+		i += gt + 1
+
+		closing := strings.HasPrefix(raw, "/")
+		name := strings.TrimPrefix(raw, "/")
+		if sp := strings.IndexByte(name, ' '); sp >= 0 {
+			name = name[:sp]
+		}
+
+		if closing {
+			if len(stack) > 0 && stack[len(stack)-1] == name {
+				stack = stack[:len(stack)-1]
+			}
+			continue
+		}
+
+		if name == "code" || name == "pre" {
+			// `
` is the one documented,
+			// supported nesting.
+			preCode := name == "code" && len(stack) == 1 && stack[0] == "pre" &&
+				strings.Contains(raw, `class="language-`)
+			if len(stack) > 0 && !preCode {
+				t.Fatalf("<%s> nested inside %v — Telegram rejects code/pre inside another entity: %q",
+					name, stack, html)
+			}
+		} else if len(stack) > 0 && (stack[len(stack)-1] == "code" || stack[len(stack)-1] == "pre") {
+			t.Fatalf("<%s> nested inside <%s> — code/pre may not contain other entities: %q",
+				name, stack[len(stack)-1], html)
+		}
+		stack = append(stack, name)
+	}
+}
+
+// TestToTelegramHTMLFlattensCodeButKeepsContent asserts the flattening
+// degrades markup only, never the code text itself.
+func TestToTelegramHTMLFlattensCodeButKeepsContent(t *testing.T) {
+	cases := []struct {
+		name string
+		in   string
+		want string
+	}{
+		{
+			name: "heading",
+			in:   "## Fix `foo.go`",
+			want: "Fix foo.go",
+		},
+		{
+			name: "bold",
+			in:   "**run `make test`**",
+			want: "run make test",
+		},
+		{
+			name: "blockquote",
+			in:   "> run `go vet`",
+			want: "
run go vet
", + }, + { + name: "code content is still escaped when flattened", + in: "## compare `a < b && c > d`", + want: "compare a < b && c > d", + }, + { + name: "top-level code keeps its tag", + in: "plain `code` here", + want: "plain code here", + }, + { + name: "top-level fence keeps pre+code", + in: "```go\nx := 1\n```", + want: "
x := 1
", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := ToTelegramHTML(tc.in); got != tc.want { + t.Errorf("ToTelegramHTML(%q)\n got: %q\nwant: %q", tc.in, got, tc.want) + } + }) + } +} + +// TestToTelegramHTMLEscapesHrefAttribute covers the attribute-injection +// bug: escapeHTML does not escape '"', so a URL containing one closed the +// href early and injected an unsupported attribute into the tag, +// which Telegram rejects. +func TestToTelegramHTMLEscapesHrefAttribute(t *testing.T) { + cases := []struct { + name string + in string + want string + }{ + { + name: "quote in url cannot break out of the attribute", + in: `[label](https://x.com" onclick="alert(1))`, + want: `label)`, + }, + { + name: "ampersand in query string stays escaped once", + in: `[a](https://x?a=1&b=2)`, + want: `a`, + }, + { + name: "angle brackets in url are escaped", + in: `[a](https://x?q=