fix(markdown): reject closing code fences with content after the marker#61300
fix(markdown): reject closing code fences with content after the marker#61300lawrence3699 wants to merge 1 commit into
Conversation
… marker parseFenceSpans() treated any line matching the fence marker pattern as a valid closing fence, even when the line had content after the marker (e.g. ```javascript inside an open ```python block). Per CommonMark §4.5, a closing code fence may only be followed by spaces or tabs. This caused the message chunking system (auto-reply/chunk.ts, pi-embedded-block-chunker.ts) to misidentify fence boundaries and split messages inside code blocks, breaking formatting on Telegram, Discord, and other channels. The fix adds a guard that rejects closing fences with non-whitespace content after the marker: !/[^ \t]/.test(match[3]). Also strips trailing \r before matching so CRLF input is handled correctly — the JS regex . metacharacter does not match \r, which previously caused the fence regex to fail entirely on CRLF documents.
Greptile SummaryFixes Confidence Score: 5/5Safe to merge — both fixes are spec-correct, narrow in scope, and covered by 18 new tests plus the 63 existing passing tests. No P0 or P1 findings. The closing-fence guard correctly implements CommonMark §4.5 (only ASCII space/tab allowed after the marker). The CRLF strip is standard line-ending normalization. The No files require special attention. Reviews (1): Last reviewed commit: "fix(markdown): reject closing code fence..." | Re-trigger Greptile |
There was a problem hiding this comment.
Pull request overview
This PR tightens Markdown fenced-code-block span parsing (parseFenceSpans) to follow CommonMark’s closing-fence rules more precisely, preventing downstream chunkers from splitting messages inside code blocks (especially when fence-like text appears within an open fence).
Changes:
- Treat a fence line as a closing fence only when the marker is followed by ASCII spaces/tabs (otherwise it’s content).
- Strip trailing
\rper line during parsing so CRLF inputs don’t break fence detection. - Add comprehensive unit tests covering closing-fence trailing content, whitespace rules (including NBSP), marker-length rules, CRLF handling, and
findFenceSpanAt/isSafeFenceBreak.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/markdown/fences.ts | Fixes fence-closing detection per CommonMark and adds CRLF robustness. |
| src/markdown/fences.test.ts | Adds targeted test coverage for fence parsing and safe-break behavior. |
What
parseFenceSpans()insrc/markdown/fences.tstreated any line matching the fence marker pattern as a valid closing fence, even when the line contained non-whitespace content after the marker — for example,```javascriptinside an already-open```pythonblock.Per CommonMark §4.5, a closing code fence may only be followed by spaces or tabs. Lines like
```javascriptare content, not closers.User-facing impact
When an AI assistant generates a response that discusses code formatting or contains nested fence-like patterns inside a code block, the message chunking system (
auto-reply/chunk.ts,pi-embedded-block-chunker.ts) mis-identifies the fence boundaries and can split the message inside the code block. Users on Telegram, Discord, WhatsApp, and other channels see broken formatting: a code block that ends too early, unformatted code spilling into plaintext, and a stray```opening a new unwanted block.Before
The parser returns two spans.
isSafeFenceBreakallows a split between the two, cutting the code block in half.After
The parser returns one span covering the entire block.
Fix
Added a guard on the closing-fence condition:
!/[^ \t]/.test(match[3] ?? ""). This ensures only ASCII spaces and tabs are allowed after the fence marker, matching the CommonMark spec exactly (rejects NBSP and other Unicode whitespace too).Added
.replace(/\r$/, "")when slicing each line, so CRLF input doesn't break the fence regex. JavaScript's.metacharacter does not match\r, which previously caused the entire fence regex to fail on CRLF documents (e.g. messages forwarded from Windows clients).Tests added
18 new tests in
src/markdown/fences.test.ts:findFenceSpanAt/isSafeFenceBreakcoverageAll 81 existing + new tests pass, including the downstream
auto-reply/chunk.test.ts(56 tests) andpi-embedded-block-chunker.test.ts(8 tests).