feat: best-effort markdown → Slack Block Kit conversion (13 LLM-input repairs + surface-uniform routing + text fallback) - #9
Merged
Conversation
Introduces internal/converter/normalizer/, the entry point for repairing LLM-emitted markdown malformations before goldmark parses. This commit ships only the orchestration shell — the concrete per-pattern repairs land in subsequent commits, one or two patterns at a time. Files: - normalizer.go: Normalize(src, opts) entry point with the documented pipeline order (structural-space first, then URL hygiene, then inline structure, then opt-in passes, then paragraph-level balance for unclosed constructs). The pipeline body is currently a no-op so downstream wiring can land independently of any individual repair. Also contains the firedSet helper that deduplicates catalog codes while preserving pipeline order. - fence_state.go: classify(src) walks the source once and tags every line with its CommonMark context (prose, fence-open/content/close, indented code, table, blank). Every repair will consult this state before touching bytes so we never corrupt code spans, fenced blocks, or table delimiter rows. - options.go: gates the two opt-in repairs (V11 entity decode, V6 asterisk balance) that touch broadcast-safety or false-positive- risky paragraphs. Tests: - normalizer_test.go: empty-input handling, no-op passthrough across five well-formed inputs, opt-in flag plumbing, firedSet dedup ordering. - fence_state_test.go: 12-row table-driven classifier suite covering fences (backtick and tilde, mismatched, with language tags), indented code (4-space and tab), GFM tables (with and without leading pipes), and blank-line boundaries. Round-trip invariant for reassemble. - normalizer_property_test.go: four safety-invariant property tests using testing/quick — idempotence under composition, 4×+1KiB length bound, no broadcast-token smuggling (with the opt-in entity decoder on), and byte-for-byte preservation of well-formed fenced code block contents. - normalizer_fuzz_test.go: FuzzNormalize with one seed per catalog code plus a broadcast-token seed; asserts the same invariants (idempotence + length bound) across arbitrary input. Fuzzed 791k inputs in 10s locally with no failures. Coverage on the new package: 98.8%. No new dependencies. No behavior change to the converter — the package is unused this commit. Catalog reference: docs/llm-input-recovery.md (lands with the user-facing surfaces in the final commit).
…numbered
Four catalog repairs that, taken together, fully recover the user's
production screenshot (a HubSpot deal summary where every header, every
bullet, and every Drive link rendered as literal markdown text in
Slack).
Repairs:
- V5 (atx_header_space.go): inserts a space between the `#` run and
heading text when missing. Regex anchored at line start with
`( {0,3})(#{1,6})([^\s#])` so it never touches mid-paragraph
`#hashtag` or `####` closer lines. Guards verified by
TestApplyATXHeaderSpace_FalsePositiveGuard.
- C3 (bullet_no_space.go): inserts a space after `-`/`*`/`+` when
missing. False-positive guard requires (1) the next character is
non-digit (so `-1 means undefined` passes through) AND (2) at least
one adjacent line shares the same indent + marker (so single-line
prose like "-loner" passes through).
- C4 (numbered_no_space.go): same shape for `1.item` / `1)item`.
Same peer-presence guard.
- V8 (split_link.go): merges `[label]\n(url)` into `[label](url)`.
Operates on line pairs; consumes blank lines between bracket and
paren (paragraph-break variant). False-positive guard restricts the
(url) content to URL-ish strings (must contain `:`, `/`, `@`, or
`#`), so prose parentheticals like "[Bar]\n(actually Baz)" are
not collapsed.
Pipeline order (normalizer.go):
V8 → V5 → C3 → C4
V8 runs first because the merge changes line count; line-local repairs
then operate on the final layout. This ordering is required for
idempotence — the test
TestNormalize_ScreenshotInput_RepairsAllExpectedPatterns demonstrates
why: without it, V8's merge creates new C3 peer relationships that
fire on a second pass.
Tests:
- 4 per-pattern test files, each covering positive cases,
false-positive guards, fence-safety, and idempotence (16 test
functions total).
- integration_test.go runs the full pipeline against
testdata/screenshot_hubspot_summary.md (the actual bot output from
the user's production Slack screenshot), asserts the expected three
repair codes fire (V8, V5, C3) and the resulting markdown contains
every header, bullet, and collapsed-link fragment we expect.
- All five existing property tests still pass (idempotence, length
bound, no broadcast smuggling, code-block preservation).
- FuzzNormalize seeded with one input per catalog code; 906k
iterations in 15s with no failures.
Coverage on the normalizer package: 94.1%.
Catalog reference: docs/llm-input-recovery.md (lands in commit 8).
…1 single-tilde, C6 borderless table, R8 br tag Five more catalog repairs. Coverage rises to 95.2% on the normalizer package; lint clean; 30s fuzz with no failures. - V7 (url_unicode.go): rewrites Unicode look-alike characters (en-dash, em-dash, curly quotes, ellipsis) to their ASCII equivalents inside markdown-link URLs and autolinks ONLY. Em-dashes in prose stay untouched (legitimate typographic choice). Uses utf8.AppendRune; gosec G115 clean. - C5/C9 (trailing_whitespace.go): strips trailing whitespace per line EXCEPT the two-trailing-spaces CommonMark hard line-break marker (§6.7). Universally safe; rolls in C5 (delimiter row) and C9 (list item) since the repair shape is identical. - C1 (tilde_in_word.go): escapes `<word>~<word>` so downstream parsers don't treat the stray tilde as a broken GFM strikethrough opener. Replaces a regex-based implementation with a single byte-walk that handles overlapping matches in one pass — a fuzz regression on the input `~0~0~0` proved the regex pipeline was not idempotent (ReplaceAllString consumes the trailing word character, so the second tilde only got caught on the second Normalize pass). Skips lines that already contain `~~` (preserves authorial strikethrough intent). - C6 (borderless_table.go): adds leading and trailing pipes to GFM table rows missing them. Operates only on lines the fence walker has classified as LineTable; preserves the original leading indent. - R8 (br_tag.go): replaces `<br>`/`<br/>`/`<br />` (any case) with newlines OUTSIDE table cells and with spaces INSIDE table cells. Slack renders neither raw HTML nor in-cell line breaks natively; this gives the visual break the author intended without leaving `<br>` text visible. Pipeline ordering in normalizer.go: R8 → V8 → C5 → V5 → C3 → C4 → V7 → C1 → C6 R8 first because it can split one line into many. V8 next so split- link merges settle before whitespace and space-insertion repairs see the final line layout. C5 (trailing whitespace) precedes the line-pattern regexes because trailing whitespace would break their end anchors. V7/C1/C6 are line-local and order-independent among themselves. Tests: 5 per-pattern test files with positive/false-positive/fence- safety/idempotence shapes. All four property tests still pass (idempotence, length bound, no broadcast smuggling, code preservation). 30s fuzz run; the surviving crash from a previous run (~0~0~0) is now fixed and the corpus seed exercises it explicitly.
…losed inline code, V1 unclosed emphasis
Four paragraph-level balance repairs for unclosed CommonMark
constructs. These are the catalog's highest-severity patterns —
unclosed fenced code in particular consumes the rest of the document.
- V3 (unclosed_fence.go): detects via the fence walker that the input
ended inside a fenced code block (last LineFenceOpen had no matching
LineFenceClose) and appends a closer matching the opener's character
(backtick/tilde), length, and indent. The single highest-severity
repair in the catalog — without it an orphaned triple-backtick
swallows every trailing header, list, and paragraph into one giant
rich_text_preformatted block.
- V4 (fence_lang_newline.go): splits a one-line ```go fmt.Println("hi")```
into the canonical three-line form. Two regexes (backtick and tilde)
because Go's regexp engine doesn't support backreferences.
Conservative knownLanguages whitelist (~50 entries) prevents false
positives on inputs like ```hello world``` where the first token
isn't a real language tag. Operates on both LineProse AND
LineFenceOpen lines because the fence walker treats one-line fences
as openers with the body+closer crammed into the info string.
- V2 (unclosed_inline_code.go): appends a closer for an unmatched
backtick opener. Pair-matching algorithm follows CommonMark 6.1:
greedy leftmost-leftmost pairing where intervening backticks become
span content (so ``a `b`` with runs [2,1,2] is correctly recognized
as balanced). Appends exactly ONE closer of the first unmatched
run's length — subsequent unmatched runs collapse into the new
span's content. Two idempotence guards:
1. Skip lines that already end in a backtick (an append would
merge into a larger run and re-arm the repair).
2. Skip lines whose unmatched opener has no non-backtick content
after it (no content to wrap, would just keep growing).
- V1 (unclosed_emphasis.go): appends a matching closer when a
paragraph contains EXACTLY one unmatched **double-star** (bold) or
standalone *single-star* (italic) run with word content after it.
Deliberately conservative — the general balancer is V6's job
(gated behind RepairMismatchedEmphasis). Inputs with mixed or
ambiguous asterisk patterns are intentionally untouched: repairing
them risks non-idempotent output.
Pipeline order in normalizer.go (full order now):
R8 -> V8 -> C5 -> V5 -> C3 -> C4 -> V7 -> C1 -> C6 -> V4 -> V3 -> V1 -> V2
V1 runs BEFORE V2: an emphasis appender that turns a line ending in a
backtick into one ending in an asterisk would re-arm V2 on the next
pass. Pinned by FuzzNormalize against the *0000000` regression that
forced this ordering.
Tests: 4 per-pattern test files with positive / false-positive /
fence-safety / idempotence shapes. End-to-end test
TestApplyUnclosedFence_DoesNotConsumeRestOfDocument demonstrates V3's
business value: prose after an orphaned fence emerges as its own
block instead of being swallowed.
Validation:
- All package tests pass.
- All five property tests still hold (idempotence, length bound,
no broadcast smuggling, code-block preservation).
- 60s fuzz: 35k execs, no failures (multiple intermediate fuzz
failures were caught and fixed during development).
- make lint clean.
- go test -race ./... all green.
Coverage: 94.3% on the normalizer package.
Adds the catalog's trickiest repair: collapsing mismatched emphasis-marker pairs like **italic* and *bold** to the smaller count. Gated behind Options.RepairMismatchedEmphasis (default false) because the algorithm is the most failure-prone in the catalog and false positives corrupt prose with deliberate asymmetric asterisks. Implementation (asterisk_balance.go): - Per-paragraph processing. - Reject paragraphs with an odd number of asterisk runs, any run of length >= 3 (bold-italic territory), or any pair whose inner content contains an asterisk (signal that the author meant a larger bold span with literal inner asterisks). - hasInnerAsteriskOverlap rejects paragraphs where consecutive pairs have no whitespace between them — the **a*b*c** shape where each "pair" actually shares content with its neighbor. Pipeline placement: V6 runs BEFORE V1 in the tail-balance phase. Collapsing a mismatched pair to *italic* can remove an unmatched opener that V1 would otherwise have to close. Fuzz-driven fixes that landed here: - V2 now skips LineTable (a backtick inside a table cell is content, not an inline-code opener). Caught by the fuzz seed "0\n---\n0`". - R8 now emits empty trailing segments as LineBlank rather than LineProse so V1's paragraph collection matches the classifier's post-pass tagging — making the R8 -> V1 -> classify chain idempotent across passes. Caught by the fuzz seed "*000<Br>". - V1 now appends only when the unmatched emphasis opener is on the LAST line of the paragraph (multi-line emphasis where the opener sits on an earlier line is genuinely ambiguous, and appending to the last line could land the closer adjacent to a structural char like '#', re-arming V5 on the next pass). Caught by the fuzz seed "*0\n# ". Tests: per-pattern test file with positive / disabled-by-default / false-positive-guard / fence-safety / idempotence coverage. All property tests still hold. 60s fuzz: 85k execs, zero failures. Coverage: 94.8% on the normalizer package.
…s + surface advisory
End-to-end integration of the normalizer pipeline into the converter,
plus two new opt-in routing knobs and the always-on auto-mode
fallback-surface advisory.
Changes:
- internal/converter/options.go
- Adds Options.PreferRichText (default false). When true, auto-mode
biases toward rich_text decomposition over the single Slack
markdown block. rich_text renders identically on push
notifications, search, screen readers, and the email digest,
where the markdown block's fallback rendering may show literal
##/**/[label](url) characters. Default flip planned for the
next major release.
- Adds Options.DecodeHTMLEntities (default false). Threaded through
to the normalizer's V11 (HTML entities) repair. Default off
because the decoded chars re-escape through sanitizeBroadcasts —
safe outcome but worth explicit opt-in.
- internal/converter/markdown_block.go
- shouldUseMarkdownBlock now early-returns false when
opts.PreferRichText is set, skipping the AST walk.
- internal/converter/renderer.go
- Adds MarkdownBlockFallbackSurfacesWarning constant: the
deterministic advisory auto-mode emits whenever it picks a
markdown block.
- ConvertWithWarnings now runs the normalizer pipeline after the
Slack mrkdwn URL-form rewrite and before goldmark parses. The
fired-codes are surfaced as a single "normalized input
(LLM-mistake repairs fired: V8, C3)" warning so the caller can
audit the repair chain.
- Auto-mode markdown-block picks now prepend the surface advisory
to the warnings slice; explicit ModeMarkdownBlock callers see
no advisory (preserves the existing contract).
- internal/converter/nested_test.go
- TestNested_LooseList_AutoMode_StillUsesMarkdownBlock updated to
expect the surface advisory (the only auto-mode test that
asserted len(warnings)==0 on short input).
- Adds expectSurfaceWarningOnly helper for shared use.
- internal/converter/prefer_rich_text_test.go (new)
- TestPreferRichText_ShortProseRoutesToRichText
- TestPreferRichText_DefaultFalse_PreservesExistingBehavior
- TestSurfaceWarning_AutoMarkdownBlock_Emitted
- TestSurfaceWarning_PreferRichText_Suppressed
- TestSurfaceWarning_ExplicitMarkdownBlockMode_NoWarning
- TestNormalization_AppliedInline (end-to-end: malformed split-link
produces a typed link element AND a V8 warning)
Fuzz-driven fixes that landed here:
- V2 (unclosed inline code) now skips any line containing a backtick
run of length 3+ — those are fenced-code-block boundaries, not
inline-code openers. Fixes a regression on inputs like
"> ```go" where the line is inside a blockquote so the fence
walker can't classify the fence directly.
- V1 (unclosed emphasis) now checks CommonMark left-flanking: an
asterisk preceded by a word character (e.g. intra-word "10*x" in
a header) is NOT an emphasis opener and must not get a closer
appended. Fixes TestHandleHeading_BoldFallback_EscapesEmphasisChars
which was inadvertently rewriting "Pricing: 10*x + 5_y" into a
malformed italic span.
Validation:
- go test ./... all green.
- make lint clean.
- go test -race ./... all green.
- Converter coverage: 89.4% (the new option/advisory paths are fully
exercised; the slight dip vs the prior 90%+ is the additional
surface area that the new walk helpers introduce).
Three production-grade additions that close the loop on best-effort Slack rendering: - internal/converter/text_fallback.go (new): DeriveTextFallback walks a converted []slack.Block slice and returns a short plain-text summary suitable for chat.postMessage(text=). Slack uses that string verbatim on push notifications, search results, screen reader output, and the email digest — the surfaces where the markdown block's own rendering degrades. The summary strips Slack mrkdwn formatting (*bold*, _italic_, ~strike~, `code`, <URL|label>, :emoji:), CommonMark links ([label](url)), ATX header hashes, and leading blockquote prefixes, then collapses whitespace and clamps to TextFallbackMaxChars (150) runes with a U+2026 suffix when clipped. Header blocks dominate the candidate order (notification previews lead with the title). - internal/converter/tables.go: data rows shorter than the header are now padded with empty rich_text cells. Slack's chat.postMessage rejects table blocks with mismatched column counts, and LLM-emitted tables routinely under-fill the trailing cell — this is the layer-B half of the catalog's C7 repair. The existing column- truncation path is untouched. - block_kit/block_kit.go: re-exports the three new public symbols (MarkdownBlockFallbackSurfacesWarning, TextFallbackMaxChars, DeriveTextFallback). The Options.PreferRichText and Options.DecodeHTMLEntities fields ride the existing type alias for converter.Options automatically. Tests: - internal/converter/text_fallback_test.go: 10-row table-driven TableCases covering header dominance, mrkdwn stripping, link stripping (both Slack and CommonMark forms), image placeholders, whitespace collapse, divider yielding empty, and edge cases. Truncation test asserts a 200-char input clips to exactly 150 runes with the ellipsis suffix. RichTextWithLink test confirms the rich_text walker extracts link labels while dropping URLs. - internal/converter/table_pad_test.go: end-to-end test feeding a 3-column header with short data rows; asserts every emitted row has exactly 3 cells. - block_kit/block_kit_test.go: three additions cover the new public surface (DeriveTextFallback through the facade, the warning constant string-shape, and the cap constant value pinned at 150 for semver stability). Validation: - go test ./... all green - go test -race ./... all green - make lint clean - Converter coverage 87.5%, block_kit coverage 85.7% (both above the >=80% gate)
… format_for_slack prompt
Wires the v0.4-track best-effort surface into every user touchpoint
and lands all docs + CHANGELOG.
MCP convert tool (internal/server/convert_tool.go):
- ConvertInput gains `prefer_rich_text` and `decode_html_entities`
bool fields, threaded through convertInputToOptions into the
corresponding converter.Options.
- ConvertOutput gains `text_fallback string`, populated via
converter.DeriveTextFallback after conversion. Empty-string is
omitted via omitempty so payloads stay clean.
- The Warnings field description is updated to reflect that it
now also carries normalization repair codes and the
fallback-surface advisory.
CLI (cmd/mcp-slack-block-kit/convert.go):
- New flags: --prefer-rich-text and --decode-html-entities.
- Convert now uses ConvertWithWarnings so warnings flow through
to JSON output; the JSON payload also includes text_fallback for
shell pipelines (e.g. jq).
MCP prompt (internal/server/prompts.go):
- format_for_slack instructions rewritten into a stable
formatForSlackInstructions constant. The new body explicitly
tells the LLM caller to:
1. pass `blocks` as chat.postMessage(blocks=)
2. pass `text_fallback` as chat.postMessage(text=)
3. surface response.warnings to the user
4. set prefer_rich_text=true for accessibility-sensitive channels
The mention-sanitization guidance from the previous body is
preserved verbatim. The mentioned deprecation (default flip in
the next major release) is called out so callers can adapt early.
Docs:
- New docs/llm-input-recovery.md: public catalog of every repair
code (V*/C*/R*) with description, default state, evidence link
list, and the "deliberately not repaired" rationale. Codes are
documented as semver-stable.
- internal/server/cheatsheet.md (the block-kit-cheatsheet MCP
resource) gains "Best-effort posting recipe" and
"Troubleshooting: literal `##`/`**`/`[label](url)` appear in
Slack" sections.
- README.md gains a footnote on the modes table about the
auto-mode fallback-surface caveat, a new "LLM input repairs"
section enumerating the normalizer patterns, and a top-level
"Troubleshooting" section walking through the three diagnostic
causes.
CHANGELOG: every behavior change in the v0.4 track documented,
organized under Added / Changed / Deprecated / Docs. The default
flip of PreferRichText (planned for the next major) is called out
in Deprecated with migration guidance.
Tests:
- TestConvertTool_PreferRichText_RoutesToRichText
- TestConvertTool_TextFallback_PopulatedInResponse
- TestConvertTool_AutoMarkdownBlock_SurfaceWarningInResponse
- TestConvertTool_Normalization_RepairsSplitLink (end-to-end:
malformed split-link in input → typed link element in output AND
V8 warning code surfaced)
- TestPrompts_FormatForSlack_ListedAndRenders extended to assert
the body contains blocks:, text:, text_fallback, and
prefer_rich_text.
End-to-end verification with the actual screenshot input:
$ printf '%s\n' '- [Project Folder]' '(https://drive.google.com/drive/folders/abc)' \
| ./bin/mcp-slack-block-kit convert --mode auto --prefer-rich-text --pretty
Output (abridged): a rich_text_list with a typed
rich_text_section_link element carrying the full URL,
text_fallback="Project Folder", and warnings naming V8. The
failure mode in the user's production Slack screenshot is now
fully handled by the library out of the box.
Validation:
- go test ./... all green
- go test -race ./... all green
- make lint clean
- make vuln: no vulnerabilities
- Coverage stays above the 80% gate across every package
…ds + V11/V6 wiring) Addresses all six ultrareview findings against PR #9. ### bug_009 (nit) — emptyTableCell wire shape internal/converter/tables.go: emptyTableCell() now constructs the inner RichTextSection with a single zero-length text element so its JSON shape is `[{"type":"text","text":""}]` instead of `"null"`. Mirrors renderRowCells' established empty-cell fallback. Dead code today (goldmark pre-pads short rows) but future-proof against any wiring change. Pinned by TestEmptyTableCell_WireShapeNeverNull. ### merged_bug_005 — C3/C4 peer guards too permissive internal/converter/normalizer/bullet_no_space.go: applyBulletNoSpace skips when the marker character appears more than once on the line (emphasis like `**bold**` or `*italic*` between two bullets is no longer mis-rewritten into `* *bold**`). `-` exempt because hyphens are common in prose; digit/whitespace regex guards still keep `-` safe for genuine lists. internal/converter/normalizer/numbered_no_space.go: - regex now requires `[^\s\d]` after the marker (was `[^\s]`), matching the bullet repair's character class. Without this the fuzz found `0.A\n0.0` non-idempotence: the `0.0` line looked like a malformed list item. - hasAdjacentNumberedPeer's catch-all `return true` replaced with a digit-class guard so `1.5 GB free\n2.3 GB used` no longer mutually validates as a numbered list. New false-positive test cases pin the regressions. ### merged_bug_001 — C5 strips inside code, only preserved exactly-2-space hard breaks internal/converter/normalizer/trailing_whitespace.go: applyTrailingWhitespace now skips LineFenceOpen/Content/Close and LineIndentedCode (matches every sibling repair; honors the package's documented "code-preserving" invariant and CommonMark §4.5). Hard-break preservation is now len-based and context-aware: preserves 2+ trailing spaces only on prose lines followed by more prose (real CommonMark §6.7 context), strips on list items / table rows / end-of-paragraph (overwhelmingly LLM sloppiness, matches original C5/C9 intent against marked PR #2201). New tests pin both fixes: InsideFenceUnchanged, InsideIndentedCodeUnchanged, and HardBreakPreserved expanded to 3- and 4-space cases. ### bug_010 — V4 idempotence violation internal/converter/normalizer/fence_state.go: classify() now treats a fenceOpen whose info string already contains a matching closing run as LineProse rather than LineFenceOpen. CommonMark §4.5 forbids the fence char inside the info string, so this is spec-alignment. Without the change, a one-liner ` ```go body``` ` followed by an unclosed-emphasis paragraph violated idempotence: pass 1 tagged the tail as LineFenceContent (V1 skipped), pass 2 re-classified post- split and V1 fired. New regression test TestNormalize_V4ThenV1_IsIdempotent and added fuzz seed. ### merged_bug_014 — R8/V7 corrupt content inside inline code spans internal/converter/normalizer/fence_state.go: new inlineCodeMask helper computes a per-byte mask of positions falling inside CommonMark §6.1 inline code spans, using the same backtick pair- matching algorithm V2 uses. internal/converter/normalizer/br_tag.go: applyBRTag skips `<br>` matches whose start position is masked; surviving matches use new replaceRangesWith / splitOnRanges helpers so positions stay stable. Example that previously corrupted: `Use \`<br>\` for HTML breaks` turned into `Use \`\\n\` for HTML breaks\`` (V2 cascaded). Now unchanged. internal/converter/normalizer/url_unicode.go: applyURLUnicode replaces ReplaceAllStringFunc with rewriteOutsideMask which honors the inline-code mask. Example that previously corrupted: `` `array[1](https://x.com/v2—doc)` `` had its em-dash silently rewritten to a hyphen. Now unchanged. New TestApplyBRTag_InsideInlineCodeUnchanged and TestApplyURLUnicode_InsideInlineCodeUnchanged pin both. ### merged_bug_002 — V11 unimplemented + V6 unreachable internal/converter/normalizer/html_entities.go: new file implements V11 entity decode. Decodes the five whitelisted XML named entities (& < > " ') plus decimal and hex numeric character references. Skips code contexts (fenced, indented, inline). Wired into the pipeline at the top (before all other repairs) so subsequent repairs see decoded characters. Broadcast-safety contract: V11 honestly decodes `<!channel>` to `<!channel>` here; the converter's sanitizeBroadcasts pass re-escapes it before rich_text emission, so the broadcast token cannot survive the round-trip to Slack unless the caller has also set AllowBroadcasts. Documented in the file header. internal/converter/options.go: adds RepairMismatchedEmphasis bool field with godoc and default in DefaultOptions(). internal/converter/renderer.go: renderer passes r.opts.RepairMismatchedEmphasis into the normalizer (was hardcoded to false, masking the existing V6 implementation). internal/server/convert_tool.go: ConvertInput gains repair_mismatched_emphasis JSON field, threaded through convertInputToOptions. cmd/mcp-slack-block-kit/convert.go: adds --repair-mismatched-emphasis CLI flag, threaded through. internal/converter/normalizer/normalizer_property_test.go: new TestProperty_V11_EntityDecodeIsLive sentinel pins that V11 actually decoded — guards against TestProperty_NoBroadcastSmuggling passing vacuously (it would always pass green if V11 ran as a no-op). ### Validation - go test ./... all green - go test -race ./... all green - make lint clean - make vuln: no vulnerabilities - 90s FuzzNormalize with extra regression seeds: 194k execs, no failures - Per-package coverage all above the 80% gate (normalizer 93.8%, converter 87.6%, server 86.8%, block_kit 85.7%, cli 83.8%) End-to-end smoke with V11 + V6 both enabled: ``` $ printf '%s\n' '**italic*' 'Tom & Jerry' \ | ./bin/mcp-slack-block-kit convert --mode rich_text \ --decode-html-entities --repair-mismatched-emphasis --pretty ``` Output: V6 collapses `**italic*` to italic span; V11 decodes `Tom & Jerry` and the result re-escapes through sanitization; warnings reports `V11, V6` fired.
Owner
Author
Ultrareview triage — all 6 findings addressed in
|
| Finding | Severity | Status | Fix |
|---|---|---|---|
bug_009 — emptyTableCell emits "elements":null |
nit | ✅ Fixed | Mirror renderRowCells' single zero-length text element + new TestEmptyTableCell_WireShapeNeverNull marshal assertion. |
merged_bug_005 — C3/C4 peer guards corrupt prose |
normal | ✅ Fixed | C3 skips when marker character repeats on line (emphasis like **bold** between bullets). C4 regex + peer-check require [^\s\d] after marker (decimal pairs like 1.5 GB\n2.3 GB no longer collapse). |
merged_bug_001 — C5 strips inside code blocks; only-exactly-2-space hard break |
normal | ✅ Fixed | C5 adds the standard LineFenceOpen/Content/Close/IndentedCode Kind switch every sibling repair has. Hard-break guard is now context-aware: preserves 2+ trailing spaces only on prose lines followed by more prose (genuine §6.7 context), strips on list items / table delimiters / end-of-paragraph (LLM sloppiness, matches original C5/C9 intent against marked PR #2201). |
bug_010 — V4 idempotence violation |
normal | ✅ Fixed | classify() now treats a fence opener whose info string contains a matching closer as LineProse (CommonMark §4.5 spec-alignment — info strings cannot contain the fence character). Kills the stale-tag cascade. New regression test + fuzz seed pin it. |
merged_bug_014 — R8/V7 corrupt inline code spans |
normal | ✅ Fixed | New inlineCodeMask helper computes per-byte mask of CommonMark §6.1 code-span positions, using the same backtick pair algorithm V2 uses. R8 and V7 skip matches whose start position is masked. New TestApplyBRTag_InsideInlineCodeUnchanged + TestApplyURLUnicode_InsideInlineCodeUnchanged pin both. |
merged_bug_002 — V11 unimplemented, V6 unreachable |
normal | ✅ Fixed | V11 (internal/converter/normalizer/html_entities.go, new) decodes the five whitelisted XML entities + numeric refs; results re-escape through sanitizeBroadcasts (broadcast-safety contract documented in the file header). V6 now reachable via new Options.RepairMismatchedEmphasis, repair_mismatched_emphasis MCP field, and --repair-mismatched-emphasis CLI flag. New TestProperty_V11_EntityDecodeIsLive sentinel guards TestProperty_NoBroadcastSmuggling against passing vacuously. |
Bonus fuzz-driven finds during fixup
The expanded FuzzNormalize corpus (seeded with one input per ultrareview bug + new opt-in flag coverage) ran 90s × 194k execs and found one additional non-idempotence case: "0.A\n0.0". Same root cause as merged_bug_005's C4 shape — fixed in the same commit by tightening the numberedNoSpace regex from [^\s] to [^\s\d], matching the bullet repair.
Validation snapshot
| Check | Result |
|---|---|
go test ./... |
all green |
go test -race ./... |
all green |
make lint |
0 issues |
make vuln |
no vulnerabilities |
| 90s fuzz with regression seeds | 194k execs, no failures |
| Coverage (all packages above 80% gate) | normalizer 93.8%, validator 95.9%, converter 87.6%, server 86.8%, reverse 87.5%, block_kit 85.7%, splitter 85.1%, cli 83.8%, preview 80.0% |
| Pre-push hooks (build-smoke, govulncheck, test-race, fuzz-smoke) | all green |
End-to-end smoke with V11 + V6 newly reachable
$ printf '%s\n' '**italic*' 'Tom & Jerry' \
| ./bin/mcp-slack-block-kit convert --mode rich_text \
--decode-html-entities --repair-mismatched-emphasis --pretty→ V6 collapses **italic* to a typed italic span; V11 decodes Tom & Jerry and the result re-escapes through broadcast sanitization (so the rendered Slack text is Tom & Jerry, broadcast-safe); warnings reports V11, V6 fired. Both opt-in repairs are now genuinely shipped, matching what CHANGELOG/README/docs already advertised.
…en regressions Addresses both findings from the second ultrareview pass against PR #9, plus two additional idempotence regressions the regression- seeded fuzz uncovered. ### bug_002 — V11 decoded C0 controls / surrogates / NUL internal/converter/normalizer/html_entities.go: new safeDecodedRune guard rejects numeric entities that would decode to C0 controls (including TAB and LF), DEL (0x7F), or UTF-16 surrogate halves (0xD800–0xDFFF). Mirrors HTML5's character-reference algorithm. Without the guard, ` ` decoded to a literal LF mid-Line.Text, which (a) violates the package's per-line invariant, (b) breaks idempotence because pass 2's classify() splits on the LF, and (c) lets LLM input smuggle paragraph/blockquote structure that wasn't in the rendered source. NUL (`�`) is worse — Slack rejects messages containing NUL outright. Pinned by new TestApplyHTMLEntities_ControlCharsRejected (10 cases: NUL/LF/CR/TAB decimal+hex, DEL, BEL, both surrogate halves) and TestApplyHTMLEntities_StructuralSmugglingBlocked. ### bug_001 — R8 whitespace-only segments tagged LineProse internal/converter/normalizer/br_tag.go: changes the post-split kind check from `seg == ""` to `strings.TrimSpace(seg) == ""`, matching classify()'s exact rule. A whitespace-only segment between two <br> tags now correctly becomes LineBlank. Without the fix, C5 (applyTrailingWhitespace) stripped a whitespace-only LineProse to empty, leaving the state {Text:"", Kind:LineProse} — a state classify() never produces. Pass 2 re-classified the empty line as LineBlank, flipping V1's paragraph boundaries (one paragraph became two) and the unclosed- emphasis appender fired on different lines than pass 1. Pinned by new TestApplyBRTag_WhitespaceOnlySegmentIsBlank and TestNormalize_BRWhitespaceMiddle_IsIdempotent regression tests. ### Bonus fuzz-driven regressions (same triage pass) **C5 vs C3/C4 ordering.** The fuzz seed `*A <br>*A` revealed that C5 ran BEFORE C3/C4 in the pipeline. C5's isHardBreakContext consults startsWithListMarker; on pass 1 the line `*A ` is not yet a list item so C5 preserves the trailing hard-break, then C3 inserts a space producing `* A `. On pass 2 the line is already a list item, isHardBreakContext returns false, and C5 strips — breaking idempotence. internal/converter/normalizer/normalizer.go: moved applyTrailingWhitespace (C5) to run AFTER V5/C3/C4 so the list-marker context is stable. None of the line-local repairs need trailing-whitespace cleanup (their regexes anchor at line start). **R8 promoted ~~~ tail to fence opener.** The fuzz seed `<Br>~~~` revealed that R8 emitted the post-split `~~~` segment as LineProse, but classify() on the same string would have tagged it LineFenceOpen. V3 (unclosed-fence repair) missed the opener on pass 1; pass 2 re-classified and V3 fired, appending a closer. internal/converter/normalizer/normalizer.go: the orchestrator now re-runs classify() on R8's output (`classify(reassemble(l))`) so post-split fence boundaries are recognized immediately. Same shape and same fix as bug_010's V4-classify recipe. **V8 false positive on bracket-less input.** The fuzz seed `]\n(]\n(#)` had no `[` anywhere but V8 still merged the trailing `]` with the following `(#)`. Pass 2 re-merged the already-merged result, producing different output. internal/converter/normalizer/split_link.go: new hasPrecedingOpenBracket guard rejects merge candidates whose `]`- ending line contains no preceding `[`. Without a preceding `[`, the `]` can't possibly close a link label. ### Validation - go test ./... all green - go test -race ./... all green - make lint clean - make vuln: no vulnerabilities - 3-minute FuzzNormalize (1.1M execs) on the refreshed pipeline: zero idempotence failures - Per-package coverage: normalizer 93.5%, validator 95.9%, converter 87.6%, server 87.2%, reverse 87.5%, block_kit 85.7%, splitter 85.1%, cli 83.8%, preview 80.0% — all above the 80% gate ### Updated fuzz seeds FuzzNormalize now seeds: bug_002 cases (` `, `
`, `�`, `paragraph > quote`), bug_001 cases (`*hello<br> <br>**unclosed`, `a<br>\t<br>b`), the fuzz-discovered shapes (`*A <br>*A`, `<Br>~~~`, `]\n(]\n(#)`).
Owner
Author
Second-pass ultrareview triage — both findings + 3 fuzz-driven bonus fixes in
|
| Finding | Severity | Status | Fix |
|---|---|---|---|
bug_002 — V11 decoded C0 controls / surrogates into raw bytes |
normal | ✅ Fixed | New safeDecodedRune guard rejects C0 controls (incl. TAB/LF), DEL (0x7F), and UTF-16 surrogate halves. Mirrors HTML5 character-reference algorithm. Pinned by 10-case TestApplyHTMLEntities_ControlCharsRejected + structural-smuggling sentinel. |
bug_001 — R8 whitespace-only segments tagged LineProse |
normal | ✅ Fixed | strings.TrimSpace(seg) == "" matches classify()'s exact LineBlank rule. New regression tests TestApplyBRTag_WhitespaceOnlySegmentIsBlank + TestNormalize_BRWhitespaceMiddle_IsIdempotent. |
Bonus fuzz-driven regressions found in the same triage pass
The regression-seeded FuzzNormalize (1.1M execs) surfaced three additional idempotence violations introduced/exposed by the first fixup. All fixed in the same commit:
| Shape | Root cause | Fix |
|---|---|---|
*A <br>*A |
C5 ran BEFORE C3/C4; C3 inserted a space mid-pipeline, changing startsWithListMarker's answer between passes |
Moved applyTrailingWhitespace to run AFTER applyBulletNoSpace/applyNumberedNoSpace so the list-marker context is stable |
<Br>~~~ |
R8 split tagged the post-split ~~~ tail as LineProse, but classify() would have tagged it LineFenceOpen — V3 missed it pass 1, fired pass 2 |
Re-run classify(reassemble(l)) after R8 fires (same recipe as bug_010's V4 fix) |
]\n(]\n(#) |
V8 merged a trailing ] with (#) even though no [ opener existed; pass 2 re-merged the result |
New hasPrecedingOpenBracket guard requires the line ending in ] to contain a preceding [ |
Validation snapshot
| Check | Result |
|---|---|
go test ./... |
all green |
go test -race ./... |
all green |
make lint |
0 issues |
make vuln |
no vulnerabilities |
| 3-min FuzzNormalize with refreshed regression seeds | 1.1M execs, zero idempotence failures |
| Per-package coverage (all above 80% gate) | normalizer 93.5%, validator 95.9%, converter 87.6%, server 87.2%, reverse 87.5%, block_kit 85.7%, splitter 85.1%, cli 83.8%, preview 80.0% |
| Pre-push hooks (build-smoke, govulncheck, test-race, fuzz-smoke) | all green |
Updated fuzz corpus
FuzzNormalize now seeds: bug_002 ( , 
, �, paragraph > quote), bug_001 (*hello<br> <br>**unclosed, a<br>\t<br>b), and the in-triage discoveries (*A <br>*A, <Br>~~~, ]\n(]\n(#)). The expanded corpus exercises every pattern that has historically broken idempotence on this branch.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
A production Slack screenshot from a downstream bot showed the
familiar failure mode: every
## Header, every**Bold:**, every[label](url)rendered as literal markdown text. Root-causeinvestigation found three reinforcing causes — malformed LLM input
(links split across lines, ATX headers without spaces, etc.),
surface-degraded rendering of the
markdownblock on pushnotifications / search / screen readers / email digest, and the
ubiquitous "caller wired the result into
text=instead ofblocks=" footgun.This PR makes the library do its best-effort conversion regardless of
LLM input quality or caller wiring choices, so the bot's output looks
right on every Slack surface out of the box.
What changed
Eight atomic commits (each independently revertable, all
Conventional Commits, all hooks green):
f2ae535feat(normalizer): new package skeleton with fence walkere464dcffeat(normalizer): repair V8 split-link, V5 ATX header, C3 bullet, C4 numbered9f71094feat(normalizer): repair V7 URL unicode, C5/C9 trailing whitespace, C1 single-tilde, C6 borderless table, R8 br tag2176ce1feat(normalizer): repair V4 one-line fence, V3 unclosed fence, V2 unclosed inline code, V1 unclosed emphasis09ca610feat(normalizer): repair V6 mismatched asterisks (opt-in)7d182f0feat(converter): wire normalizer + PreferRichText + DecodeHTMLEntities + surface advisoryd7e80c1feat(converter): DeriveTextFallback, table pad-up, public re-exports1a3826efeat(server): expose normalizer + best-effort knobs; CLI flags, docs, format_for_slack prompt1. Pre-parse normalizer (new
internal/converter/normalizer/)13 always-on repairs + 2 opt-in, each with cited evidence in
docs/llm-input-recovery.md:**bold)`code)```,~~~)```go code```split to canonical 3 lines#Title→# Title**italic*→*italic*)[label]\n(url)→[label](url)~between word chars escaped<br>→ newline (or space inside table cells)Every repair: idempotent, code-fence-aware, O(n), broadcast-safe
(no repair can introduce literal
<!channel>etc. that wasn't inthe input). All proven by per-pattern unit tests + four property
tests + a
FuzzNormalizetarget.2. New
OptionsknobsPreferRichText(defaultfalse) — biases auto-mode towardrich_textdecomposition.rich_textrenders identically onevery Slack surface; the single
markdownblock can show literalcharacters on push notifications, search, screen readers, and
email digest. Planned to flip to
truein the next majorrelease (documented in
CHANGELOGDeprecated).DecodeHTMLEntities(defaultfalse) — gates V11'swhitelisted entity decoder.
3. End-to-end fallback closure
MarkdownBlockFallbackSurfacesWarningconstant — auto-modeemits this advisory whenever it picks a
markdownblock.DeriveTextFallback([]slack.Block) string+ newConvertOutput.text_fallbackfield — gives the caller a cleanplain-text summary suitable for
chat.postMessage(text=).format_for_slackMCP prompt body rewritten to teach theLLM the recipe: pass
blocksasblocks=, passtext_fallbackas
text=, surfacewarnings.4. User-facing surfaces
prefer_rich_text/decode_html_entitiesinputs andtext_fallbackoutput../bin/mcp-slack-block-kit convert) gains--prefer-rich-textand--decode-html-entitiesflags; JSONoutput now includes
text_fallbackandwarnings.block_kit/public surface re-exports the new constants andhelper (
Options.PreferRichText/DecodeHTMLEntitiesride theexisting type alias).
block-kit-cheatsheetMCP resource) andREADME gain a "Best-effort posting recipe" + "Troubleshooting:
literal
##/**/[label](url)appear in Slack" section walkingthrough the three diagnostic causes.
End-to-end against the original screenshot
Now produces (abridged):
{ "blocks": [{ "type": "rich_text", "elements": [{ "type": "rich_text_list", "elements": [{ "type": "rich_text_section", "elements": [{ "type": "link", "url": "https://drive.google.com/drive/folders/abc", "text": "Project Folder" }] }], "style": "bullet" }] }], "text_fallback": "Project Folder", "warnings": [ "normalized input (LLM-mistake repairs fired: V8). See docs/llm-input-recovery.md for codes." ] }The malformed split-link is now repaired automatically AND surfaces
in
warningsfor caller-side visibility, AND thetext_fallbackisready for
chat.postMessage(text=).Validation
go test ./...go test -race ./...make lintmake vulnPer-package coverage:
internal/validatorinternal/converter/normalizerinternal/converterinternal/reverseinternal/serverblock_kit/internal/splittercmd/mcp-slack-block-kitinternal/previewAll above the ≥80% project gate.
Migration
This release is fully backward-compatible. Every new field
defaults to its existing behavior;
Options.PreferRichTextandOptions.DecodeHTMLEntitiesare explicit opt-ins. Two future-facingnotes:
MarkdownBlockFallbackSurfacesWarningin
warningswhenever it picks a markdown block. Callers thatasserted
len(warnings) == 0on short auto-mode inputs need toupdate (the only in-tree test that did so has already been
adjusted).
Options.PreferRichTextwill default totruein the nextmajor release. To pin current behavior across the flip, set it
explicitly to
false.Compatibility
existing
convert_markdown_to_block_kittool).format_for_slackprompt body rewritten — the prompt name andargument shape are unchanged, only the instruction text changed.
🤖 Generated with Claude Code