Skip to content

feat(bin): add verified local Markdown previews#906

Open
gavinsomers wants to merge 16 commits into
kunchenguid:mainfrom
gavinsomers:fm/fm-agents-markdown-preview
Open

feat(bin): add verified local Markdown previews#906
gavinsomers wants to merge 16 commits into
kunchenguid:mainfrom
gavinsomers:fm/fm-agents-markdown-preview

Conversation

@gavinsomers

@gavinsomers gavinsomers commented Jul 23, 2026

Copy link
Copy Markdown

What Changed

  • Add a fail-closed Markdown preview helper with sanitized rendering, bounded resource use, current-file verification, tokenized loopback serving, and reliable cleanup.
  • Require previews for generated or updated Markdown and document the helper, its Python 3 requirement, and test-runner integration.
  • Add comprehensive preview regression coverage while hardening Pi TypeScript probing and isolating Orca bootstrap tests from host binaries.

Risk Assessment

✅ Low: The latest changes correctly preserve existing preview failures while normalizing descriptor I/O, close, and loopback server-construction errors without introducing a material source risk.

Testing

Baseline diff/status inspection, focused preview and related regression tests, and a live end-user rendering all completed successfully; the screenshot shows formatted Markdown with raw script text escaped and the remote image omitted, the helper reported success, and its loopback server was closed afterward. The in-app browser was unavailable, so the visual evidence was captured with local headless Chrome instead.

  • Evidence: Rendered Firstmate Markdown preview (local file: /tmp/no-mistakes-evidence/01KY7DACYHC79Z810G6TNF0G39/markdown-preview.png)
Evidence: Successful preview and lifecycle evidence

ok - previewed preview-fixture.md preview server closed after delivery (curl exit 7)

ok - previewed preview-fixture.md

Pipeline

Updates from git push no-mistakes

⏭️ **intent** - skipped

✅ No issues found.

🔧 **Rebase** - 1 issue found → auto-fixed ✅
  • ⚠️ AGENTS.md - merge conflict rebasing onto origin/main

🔧 Fix applied.
✅ Re-checked - no issues remain.

🔧 **Review** - 2 issues found → auto-fixed (6) ✅
  • ⚠️ AGENTS.md:49 - The mandatory security-sensitive preview workflow has no repository-owned renderer or verification helper. Each harness must independently interpret sanitization, remote-resource blocking, current-file verification, and cleanup, producing inconsistent guarantees or making delivery impossible on a clean host. Decide whether to add one pinned, fail-safe preview helper or skill and reference it here.
  • ⚠️ tests/fm-pi-primary-types.test.sh:9 - The tsc --version probe runs before checking whether the Pi package exists. A host without Pi but with a broken, wrapped, or unrelated tsc now fails instead of taking the documented Pi-not-installed skip path. Resolve and validate PI_PACKAGE_DIR before probing the compiler.

🔧 Fix: Captain: Add fail-safe previews and package-first Pi checks
3 issues (1 error, 2 warnings) still open:

  • 🚨 bin/fm-markdown-preview.py:569 - After a verified browser fetch, the helper still requires the opener process to exit within one second and then terminates it. xdg-open can run its selected handler in the foreground, so a successfully loaded preview may be reported as failed and its newly launched browser terminated. Decide whether verified delivery should constitute success independently of opener lifetime.
  • ⚠️ bin/fm-markdown-preview.py:176 - Every unmatched [ scans the remainder of the line for ](, with similar repeated scans for unmatched emphasis delimiters. Bracket-heavy input is therefore quadratic, while rendering occurs before the timeout and accepts files up to 8 MiB. Use a single-pass or otherwise bounded delimiter scan.
  • ⚠️ bin/fm-markdown-preview.py:515 - Server verification reads at most MAX_SOURCE_BYTES + 1, but HTML escaping can expand an accepted source to several times that size. A valid source below the 8 MiB limit can therefore be truncated during verification and falsely rejected as a content mismatch. Read against the bounded rendered-body length or enforce an explicit rendered-output limit before serving.

🔧 Fix: Captain: Harden preview delivery and rendering bounds
6 issues (1 error, 5 warnings) still open:

  • 🚨 bin/fm-markdown-preview.py:538 - urllib.request.urlopen honors proxy environment variables, so local verification can send the tokenized loopback URL to an external proxy and falsely fail when loopback is not exempted. Use a proxy-disabled opener or a direct loopback connection for verification.
  • ⚠️ bin/fm-markdown-preview.py:643 - If thread.start() raises, cleanup calls server.shutdown() even though serve_forever() never started; BaseServer.shutdown() can then wait indefinitely. Track whether the thread started and call only server_close() otherwise.
  • ⚠️ bin/fm-markdown-preview.py:178 - The delimiter budget does not cover counting a backtick run. For a large unmatched run, each successive backtick recounts most of the remaining run, making rendering quadratic before the serving timeout begins. Consume the whole unmatched run at once or include this scan in the budget.
  • ⚠️ bin/fm-markdown-preview.py:73 - The fence regex combines overlapping whitespace-capable groups before an end anchor, causing quadratic backtracking for a tilde fence followed by a long whitespace tail and a final backtick. Parse the fence tail separately or make the groups non-overlapping.
  • ⚠️ bin/fm-markdown-preview.py:357 - Nested blockquotes recursively invoke render_blocks without a depth bound, so roughly 1,000 nested markers can raise an uncaught RecursionError from a small accepted file. Render iteratively or reject excessive nesting with PreviewError.
  • ⚠️ bin/fm-markdown-preview.py:432 - The 64 MiB rendered-output limit is checked only after all block and table output has been accumulated, joined, and encoded. A wide accepted table can therefore allocate hundreds of megabytes before rejection. Enforce an output or cell budget during rendering.

🔧 Fix: Harden preview verification and rendering bounds
6 warnings still open:

  • ⚠️ bin/fm-markdown-preview.py:350 - Each fenced-block line rebuilds a marker-sized closing prefix. An accepted file with a multi-megabyte opening fence and many short body lines therefore performs quadratic allocation and work. Reuse marker directly in startswith or cache the prefix once.
  • ⚠️ bin/fm-markdown-preview.py:76 - The lazy heading body followed by overlapping \s*#*\s* suffixes can backtrack quadratically on a heading containing a long whitespace run before a non-matching suffix. Parse trailing heading markers in one pass or make the groups non-overlapping.
  • ⚠️ bin/fm-markdown-preview.py:100 - RenderBuffer limits bytes but not part count. Millions of tiny accepted blocks can remain below 64 MiB of output while split() and parts retain millions of Python objects, consuming hundreds of megabytes. Add a line/block/part budget or coalesce output incrementally.
  • ⚠️ bin/fm-markdown-preview.py:166 - urlsplit raises ValueError for malformed targets such as an unmatched IPv6 bracket, so one malformed Markdown link produces an uncaught traceback instead of being rendered without an unsafe href. Catch parser errors and return None.
  • ⚠️ bin/fm-markdown-preview.py:310 - Table detection applies the column limit before confirming that the following line is a divider. Ordinary prose containing | followed by a non-table line with over 1,024 delimited fields is therefore rejected as an oversized table. Enforce the limit only after recognizing a valid divider row.
  • ⚠️ bin/fm-markdown-preview.py:414 - The 64-level quote bound still copies the nearly complete captured line at every recursive level. A valid 8 MiB line with 64 nested quote markers can retain roughly 512 MiB before output budgeting applies. Strip quote prefixes iteratively or include retained input size in the budget.

🔧 Fix: Bound Markdown parser work and retained memory
2 warnings still open:

  • ⚠️ bin/fm-markdown-preview.py:132 - read_source() performs a blocking os.open(..., O_RDONLY) before validating that the descriptor is regular. A FIFO, including a path swapped to one, can therefore hang the supposedly bounded helper indefinitely. Open nonblocking and then validate the descriptor type.
  • ⚠️ bin/fm-markdown-preview.py:194 - The new byte and part budgets apply only after render_inline() has accumulated and joined its private lists. One accepted 8 MiB line containing dense short markup can retain millions of fragments and substantial object overhead before a single RenderBuffer.append() occurs. Apply incremental byte/fragment limits inside inline rendering or coalesce bounded chunks.

🔧 Fix: Bound source opening and inline rendering
2 warnings still open:

  • ⚠️ bin/fm-markdown-preview.py:180 - Only os.open is translated to PreviewError; failures from os.fstat, os.read, or os.close escape as raw tracebacks instead of the documented controlled preview failure. Normalize descriptor I/O errors while preserving existing PreviewError exceptions.
  • ⚠️ bin/fm-markdown-preview.py:812 - Loopback socket creation or binding can raise OSError from PreviewServer(state), but main() does not catch it, so an expected serving failure produces a traceback. Wrap server construction as PreviewError and retain cleanup for partially initialized state.

🔧 Fix: Normalize preview I/O and server failures
✅ Re-checked - no issues remain.

✅ **Test** - passed

✅ No issues found.

  • Inspected git diff 68ed7ed2355619749c6ee2bb91b7298dee420df8..37b94216fea0b795e92676b45b1c85362c638d98 to establish intent and affected test scope.
  • tests/fm-markdown-preview.test.sh
  • tests/fm-bootstrap.test.sh
  • tests/fm-pi-primary-types-order.test.sh
  • tests/fm-pi-primary-types.test.sh (expected environment skip because installed TypeScript is older than version 5)
  • bin/fm-markdown-preview.py --timeout 30 --opener /tmp/no-mistakes-evidence/01KY7DACYHC79Z810G6TNF0G39/capture-url-opener.sh /tmp/no-mistakes-evidence/01KY7DACYHC79Z810G6TNF0G39/preview-fixture.md
  • Rendered the live loopback URL using google-chrome --headless --window-size=1440,1000 --screenshot=... and visually inspected the resulting PNG.
  • Verified the loopback URL was unreachable after delivery using curl --noproxy '*' --silent --show-error --max-time 1 "$preview_url".
  • Checked cleanup with git status --short.
✅ **Document** - passed

✅ No issues found.

⚠️ **Lint** - 1 warning
  • ⚠️ linter found issues (exit code 1)
✅ **Push** - passed

✅ No issues found.

@gavinsomers gavinsomers changed the title feat: require safe local Markdown previews feat: require secure local Markdown previews Jul 23, 2026
@gavinsomers gavinsomers changed the title feat: require secure local Markdown previews feat: add secure Markdown preview workflow Jul 23, 2026
@gavinsomers
gavinsomers force-pushed the fm/fm-agents-markdown-preview branch from 944cfcd to 31918aa Compare July 23, 2026 13:23
@gavinsomers gavinsomers changed the title feat: add secure Markdown preview workflow feat(bin): add verified local Markdown previews Jul 23, 2026
@gavinsomers

Copy link
Copy Markdown
Author

Hi @kunchenguid — this is ready for review. The Markdown preview helper and workflow rules passed the full automated review and test suite, with low residual risk. Thanks.

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.

1 participant