Skip to content

fix(markdown): restore extracted blocks verbatim so $& and $$ survive#5768

Open
husamemadH wants to merge 1 commit into
odysseus-dev:devfrom
husamemadH:fix/5663-markdown-dollar-restore
Open

fix(markdown): restore extracted blocks verbatim so $& and $$ survive#5768
husamemadH wants to merge 1 commit into
odysseus-dev:devfrom
husamemadH:fix/5663-markdown-dollar-restore

Conversation

@husamemadH

@husamemadH husamemadH commented Jul 26, 2026

Copy link
Copy Markdown

Summary

mdToHtml lifts fenced code, math, mermaid and allowed-HTML blocks out of the source and parks a placeholder (___CODE_BLOCK_0___) in their place so the markdown rules can't mangle them, then pastes each block back at the end. The paste used a string replacement:

s = s.replace(`___CODE_BLOCK_${index}___`, block);

With a string replacement, String.replace reads $&, $`, $' and $$ in the replacement as substitution patterns. So a restored block containing any of them is corrupted:

  • $& re-inserts the matched text — i.e. the placeholder itself
  • $` and $' splice in everything before / after the placeholder
  • $$ collapses to a single $

These are ordinary content in shell and perl snippets, so real code samples render wrong in chat, notes and documents. Reported in #5663.

This passes a function replacer at all four sites, matching the inline-code restore immediately below them which was already fixed this way. A function's return value is inserted verbatim, with no $ interpretation.

Target branch

  • This PR targets dev, not main.

Linked Issue

Fixes #5663

Type of Change

  • Bug fix (non-breaking — fixes a confirmed issue)

Checklist

  • I searched open issues and open PRs — this is not a duplicate.
  • This PR targets dev
  • My changes are limited to the scope described above — no unrelated refactors or whitespace changes mixed in.
  • I actually ran the app (docker compose up -d --build) and verified the change works end-to-end. Screenshots below.

How to Test

  1. Ask any model: "Write a one-line bash script using $& and put it in a fenced sh code block."

  2. Before the fix, the rendered block shows the placeholder and an orphaned amp;:

    echo "hello world" | perl -pe 's/world/___CODE_BLOCK_0___amp; again/'
    

    ($& is HTML-escaped to $& before the paste, so $& consumes the placeholder and leaves amp; behind.)

  3. After the fix the same prompt renders the snippet verbatim, $& intact.

  4. Deterministic check without a model — DevTools console on the app:

    const { mdToHtml } = await import('/static/js/markdown.js');
    console.log(mdToHtml('```sh\necho "match: $&"\n```'));       // no ___CODE_BLOCK_
    console.log(mdToHtml('```sh\nsed \'s/$`/x/\'\n```'));        // $` no longer vanishes
    console.log(mdToHtml('```sh\necho "$$USD"\n```'));           // $$ no longer collapses
  5. Tests:

    python -m pytest tests/test_markdown_rendering_js.py -q          # 17 passed
    python -m pytest tests/ -k "markdown or emoji or streaming_segmenter" -q   # 51 passed
    node --input-type=module --check < static/js/markdown.js
    node tests/markdown_codefence_placeholder_regression.mjs        # ok

    The four new tests were confirmed to fail against the unpatched markdown.js and pass with it.

Note on the issue description

#5663 lists $1 among the corrupting sequences, and an existing comment on the inline-code restore says echo $1 would be treated as a back-reference. Neither is the case: with a string search value there are no capture groups, so JS leaves $1..$99 literal. Verified both $1 and $10 pass through untouched. The four sequences that actually corrupt are $&, $`, $' and $$. The comment has been reworded accordingly; the fix itself is unchanged by this.

Visual / UI changes

  • Screenshot of the change in the running app, attached below.
  • Style match: no CSS, markup, class or icon changes — this only affects the text content inside already-existing <pre><code> output.
  • No new component patterns.

Screenshots

Before$& replaced by the placeholder, with the orphaned amp;:

image

After — the snippet renders verbatim:

image

Files changed

  • static/js/markdown.js — function replacer at the allowed-HTML, math, mermaid and code restore sites; the shared explanation moved above the group since it now covers all five.
  • tests/test_markdown_rendering_js.py — four regression tests: $& (the reported perl case), $` / $' silent deletion, $$ collapsing, and the mermaid restore path.

Disclosure

The patch was drafted with Claude Code. I reproduced the bug in my own instance, reviewed the change, and verified before/after in the running app. Flagging it per CONTRIBUTING.md's note on agent-generated PRs — this is a single targeted fix against an existing issue, not a bulk submission.

The placeholder-restore pass in mdToHtml put code, math, mermaid and
allowed-HTML blocks back with a string replacement, so String.replace read
`$&`, `` $` ``, `$'` and `$$` in the *replacement* as substitution patterns.
A fenced block containing them rendered corrupted: `$&` re-inserted the
placeholder (`perl -pe 's/world/$& again/'` became
`s/world/___CODE_BLOCK_0___amp; again/`), `` $` `` and `$'` spliced in the
surrounding document, and `$$` collapsed to a single `$`.

Pass a function replacer at all four sites, matching the inline-code site
below them, which was already fixed this way. A function's return value is
inserted verbatim with no `$` interpretation.

The inline-code comment claimed `echo $1` would be read as a back-reference;
with a string search value there are no capture groups, so `$1` is already
literal. Reworded to name the four sequences that do corrupt.

Fixes odysseus-dev#5663
@github-actions github-actions Bot added the ready for review Description complete — ready for maintainer review label Jul 26, 2026
@husamemadH

Copy link
Copy Markdown
Author

Previously attempted in #5664 by @wbaxterh, which was closed when dev was force-pushed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready for review Description complete — ready for maintainer review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(markdown): code/HTML/math blocks containing $&, $`, $', $$, or $1 render corrupted

1 participant