fix(cli): don't skip image and link substitution after a literal < in markdown - #17315
Conversation
… markdown Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
| if (content[i] === "`" && content[i - 1] !== "\\") { | ||
| inInlineCode = !inInlineCode; | ||
| i++; | ||
| continue; | ||
| } | ||
|
|
||
| if (inInlineCode) { | ||
| i++; | ||
| continue; | ||
| } |
There was a problem hiding this comment.
🔴 A single unmatched backtick on a page stops every later image and link from being rewritten
The rewriting pass now treats everything after an unmatched backtick as inline code (inInlineCode = !inInlineCode at packages/cli/docs-markdown-utils/src/parseImagePaths.ts:820-824) with no reset at line or paragraph boundaries, so images and links later on the page keep their build-machine file paths.
Impact: Pages containing an odd number of backticks (for example an indented code block whose contents include a stray backtick) publish local filesystem paths and show broken images — the same failure this change is meant to fix, just triggered by a backtick instead of <.
How the inline-code state leaks across the whole document
Pass 2 (replaceImagePathsAndUrls) now toggles inInlineCode on every unescaped backtick and, while it is set, skips all image/link/JSX handling (packages/cli/docs-markdown-utils/src/parseImagePaths.ts:826-829). The state is never reset at a newline or blank line, and the fence detector at packages/cli/docs-markdown-utils/src/parseImagePaths.ts:809-813 only recognizes at the very start of a line, so fences indented inside list items are not recognized as fences and their backtick contents feed the inline-code toggle instead. Any document whose "inline" backtick count is odd (unmatched backtick in prose, a lone backtick inside an indented fence, or a `~~~`-delimited block containing) ends up permanently in the inline-code state.
Because pass 1 for normal-sized files uses the mdast path, it correctly rewrites image sources to absolute paths; pass 2 then silently skips the substitution, and the absolute path is written into the published markdown. Per CommonMark, code spans cannot contain a blank line and unmatched backticks are literal text, so the scanner should at minimum clear inInlineCode at blank lines (and ideally at end of line).
Prompt for agents
In replaceImagePathsAndUrls (packages/cli/docs-markdown-utils/src/parseImagePaths.ts), the newly added inline-code tracking toggles inInlineCode on every unescaped backtick and never resets it. A document with an odd number of such backticks (unmatched backtick in prose, or backticks inside a fence indented within a list item, which the line-start-only ``` detector does not recognize as a fence) leaves the scanner permanently in the inline-code state, so every subsequent image and link substitution is skipped and the absolute filesystem path inserted by pass 1 is published verbatim. Per CommonMark, a code span cannot contain a blank line and unmatched backticks are literal text, so the state must be bounded: clear inInlineCode when a blank line (paragraph boundary) is reached, and consider clearing it at end of line, or match backtick runs (open with N backticks, close only with the same run length). The same unbounded state exists in streamingScanForImages (packages/cli/docs-markdown-utils/src/parseImagePaths.ts:94-136) and should be fixed consistently so both passes agree.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Valid — fixed in 10d466b. Both passes no longer carry inInlineCode/inCodeFence state at all:
findInlineCodeEndmatches backtick runs (open with N, close only on a run of exactly N) and is bounded at the next blank line; an unmatched backtick is treated as literal text and the scan continues from the next character, so it can't leak across the document.findCodeFenceEndfinds the actual closing fence line for a ``` or~~~run, allows leading indentation (so fences inside list items are recognized), and returns null for an unterminated fence rather than swallowing the rest of the page.
New tests: unmatched backtick in prose, backtick inside an indented fence, unterminated fence, plus one asserting an image inside a real fence is still left alone.
| function findJsxTagScanLimit(content: string, start: number): number { | ||
| const blankLine = content.indexOf("\n\n", start); | ||
| return blankLine === -1 ? content.length : blankLine; | ||
| } |
There was a problem hiding this comment.
🟡 Windows-style line endings defeat the new safety bound, so a stray angle bracket can again skip images
The new bound that stops a tag scan at the next empty line looks only for two consecutive Unix newlines (content.indexOf("\n\n", start) at packages/cli/docs-markdown-utils/src/parseImagePaths.ts:82), so on pages saved with Windows line endings the scan runs to the end of the page and the images and links it passes over keep their build-machine paths.
Impact: Docs pages authored with Windows line endings can still publish local filesystem paths and broken images when a tag-like < is left unterminated.
Mechanism
findJsxTagScanLimit is the only guard preventing an unterminated tag-like < from consuming the rest of the document, and it is used by both passes (packages/cli/docs-markdown-utils/src/parseImagePaths.ts:305 and :897). With CRLF content, blank lines are \r\n\r\n, which does not contain \n\n, so indexOf returns -1 and the limit becomes content.length. Nothing in this package normalizes line endings (no CRLF handling exists in packages/cli/docs-markdown-utils/src). Matching /\n[ \t]*\r?\n/ (or normalizing \r) would restore the bound.
| function findJsxTagScanLimit(content: string, start: number): number { | |
| const blankLine = content.indexOf("\n\n", start); | |
| return blankLine === -1 ? content.length : blankLine; | |
| } | |
| function findJsxTagScanLimit(content: string, start: number): number { | |
| const blankLine = /\n[ \t]*\r?\n/.exec(content.slice(start)); | |
| return blankLine == null ? content.length : start + blankLine.index; | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed in 10d466b — the bound (now findScanLimit, shared by the tag scan and the inline-code scan) matches / [ \t]*\r? /, so CRLF and whitespace-only blank lines are handled. Added a CRLF regression case.
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Description
Local images and links were published as build-machine filesystem paths (e.g.
<img src="/home/runner/work/sigma-docs/sigma-docs/fern/assets/docs-images/....png">, or/Users/<name>/...infern docs dev), producing 404s on the live site — 23 broken images across 11 pages on one customer site.parseImagePaths(pass 1) resolves and uploads the images fine; the second passreplaceImagePathsAndUrls, which swaps each absolute path forfile:<fileId>, is where it breaks. Since #16881 that pass always uses the hand-rolled streaming scanner, whose<branch assumes any<starts a JSX tag and fast-forwards to the next>. A literal<in prose or inline code (`is < Q1 - 1.5*IQR`,(filter is `<=`)) therefore swallows everything up to the next>anywhere later in the page, so every image and link in that window never gets substituted — and a failed substitution is silent, so the raw absolute path is written into the published markdown.Changes Made
isJsxTagStart: only treat<as a tag when a tag name or/follows and it isn't escaped, soa < b,<=, and\<are text.findJsxTagScanLimit: bound the tag scan at the next blank line, so an unterminated<can't consume the rest of the page.replaceImagePathsAndUrls(pass 2), matching what pass 1 already did.fixchangelog entry underpackages/cli/cli/changes/unreleased/.Testing
parseImagePaths.test.tscovering the exact shapes seen in the report (`is < Q1 - 1.5*IQR`,(filter is `<=`), plain-text< 5,\<, fenced code), plus assertions that real tags and links still get rewritten and that no local filesystem path survives to the published markdown.143/143tests pass in@fern-api/docs-markdown-utils; the new "does not leave a local filesystem path in the published markdown" test fails onmainand passes here.Worth a human eye
The tag scan is bounded at the next blank line, so a JSX opening tag split across a blank line (
<Card\n\nhref="…">) would no longer be rewritten. That pattern isn't in our fixtures and MDX generally rejects it, but it is the one behavior change beyond the bug fix.Follow-up from review (commit
10d466b): the inline-code/code-fence tracking added in pass 2 no longer uses sticky flags —findInlineCodeEndmatches backtick runs (close only on a run of the same length, bounded at the next blank line) andfindCodeFenceEndlocates the real closing fence (indentation-aware,nullwhen unterminated), so a stray backtick or an unterminated fence can't suppress later substitutions. The blank-line bound is CRLF-aware.Link to Devin session: https://app.devin.ai/sessions/97c2b82e7d744160963019aeaba0a41b
Requested by: @fern-support