Skip to content

fix(md-exec): heredoc body containing a ``` line is mistaken for the end of the code block - #259

Open
batuhankocyigit wants to merge 1 commit into
circlefin:mainfrom
batuhankocyigit:fix/md-exec-heredoc-fence-detection
Open

fix(md-exec): heredoc body containing a ``` line is mistaken for the end of the code block#259
batuhankocyigit wants to merge 1 commit into
circlefin:mainfrom
batuhankocyigit:fix/md-exec-heredoc-fence-detection

Conversation

@batuhankocyigit

Copy link
Copy Markdown

fix(md-exec): heredoc body containing a ``` line is mistaken for the end of the code block

The bug

scripts/md-exec.py's MarkdownParser.parse() detects code-fence
boundaries by checking whether a line, stripped, starts with ```.
This check runs unconditionally on every line inside a code block —
including lines that are part of an open heredoc body being fed to a
shell command. If a heredoc body contains a line starting with ```
(for example, a heredoc that writes out a markdown file, or a snippet that
itself documents markdown syntax), the parser treats that line as the end
of the code fence, cutting the command short mid-heredoc. The shell then
receives an unterminated heredoc.

Reproduction — this file:

```sh {empty_output}
$ cat > /tmp/out.txt << 'EOF'
before
```
after
EOF
```

fails today with:

/bin/bash: line 6: warning: here-document at line 3 delimited by
end-of-file (wanted `EOF')

heredoc_delimiter is already tracked by the parser (set when a << DELIM command starts, cleared when the delimiter line is seen), but the
fence-detection check doesn't consult it.

This isn't just a contrived case: docs/running-an-arc-node.md already
uses heredocs to write out full files (cat << "EOF" > ~/.arc_env, two
sudo tee ... <<EOF systemd unit blocks), and make test-unit-contract
runs python3 scripts/md-exec.py crates/quake/tests as part of the test
suite — so this is live infrastructure, not just the tool's own
self-documentation.

The fix

One-line change: only treat a ```-prefixed line as a fence boundary
when we're not currently inside an open heredoc body.

if stripped.startswith("```") and heredoc_delimiter is None:

Testing

  • Added a regression case to scripts/md-exec.md (the tool's own
    self-test suite, run via python3 md-exec.py md-exec.md) covering a
    heredoc body with an embedded ``` line.
  • Confirmed the new case fails without the fix (git stash the .py
    change, rerun — reproduces the exact bash heredoc warning above) and
    passes with it.
  • Full self-test suite: 30/30 passing (29 pre-existing + 1 new), no
    regressions.
  • python3 -m py_compile scripts/md-exec.py — compiles cleanly.

Notes for reviewers

  • Scoped to the single fence-detection condition; no other parsing logic
    touched.
  • I also fuzz-tested the ellipsis-matching (check_match) function
    separately looking for bugs there; found one genuinely ambiguous edge
    case (4+ consecutive literal dots, e.g. foo....bar), but the docs only
    define behavior for exactly ..., so I didn't treat that as a bug or
    include a "fix" for it here — happy to open a separate discussion/PR if
    you'd like that behavior formally defined.

@osr21 osr21 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Full executable verification this time (Python is in my environment, unlike the Rust/shell gaps I've disclosed elsewhere) — every claim reproduces:

  • Bug confirmed on main: your exact repro file fails with the exact warning quoted (here-document at line 3 delimited by end-of-file (wanted 'EOF')), 0 passed 1 failed.
  • Fix confirmed on your branch: same repro passes; the parser now correctly carries the full heredoc body — including the embedded ``` line — into the command.
  • Self-test suite: 29/29 on main, 30/30 on your branch — and this time the pre-existing count in the PR body is right too.
  • Real-world exposure confirmed: docs/running-an-arc-node.md has 3 heredocs on main, and make test-unit-contract does run md-exec.py over crates/quake/tests (Makefile line 262), so the parser is live test infrastructure, not just self-documentation.
  • Scope check: the diff is exactly the one condition (and heredoc_delimiter is None) plus an explanatory comment and the regression case in md-exec.md. The state machine already clears heredoc_delimiter on the delimiter line before the next fence check, so the fix composes correctly with the existing tracking — no new state introduced.

One edge case worth a sentence in the comment, though I don't consider it blocking: if a code block contains a heredoc whose delimiter line never appears (malformed markdown), the parser now also skips the real closing fence and silently swallows the rest of the file into that command. Pre-fix, the same malformed input produced a confusing-but-visible failure. That's a strictly-broken-input scenario and the old behavior wasn't good either, so I'd merge as-is — but if you want to harden it, resetting heredoc_delimiter with a warning when EOF is reached inside an open heredoc would make the failure mode loud again.

Also appreciated the restraint in the notes: not "fixing" the 4-dot ellipsis ambiguity you found while fuzzing, because the spec only defines ..., is the right call — a behavior-defining discussion first is exactly how that should go.

LGTM. Small, correctly-scoped, honestly-documented, and verified end-to-end.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants