Rebuild the Markdown preview: GFM, inline HTML, syntax highlighting, and light/dark theming - #1155
Merged
Conversation
The rendered Markdown preview was a hand-rolled `marked` token walker that
escaped all inline HTML, gave headings a flat scale, and had cramped spacing
— so a real README rendered `<p align="center">`/`<img>` as literal text.
Replace it with a two-stage pipeline in @kolu/solid-markdown:
- render.ts — `marked` (GFM) → HTML, DOM-free + unit-tested, with a
`safeHref` link allowlist and a per-slot links toggle.
- sanitize.ts — DOMPurify sanitizes the HTML, then a small DOM pass severs
targeted links from their opener and degrades un-loadable (repo-relative)
images — markdown- and inline-HTML-sourced alike — to a labelled chip.
- markdown.css — a themed `.kolu-md` stylesheet that paints with
`currentColor` + `color-mix` and the app's `--color-accent`, so it follows
the light/dark preference with no theme prop.
Now renders full GFM (tables, task lists, strikethrough, autolinks) plus the
inline HTML a README leans on (`<details>`, `<kbd>`, alignment wrappers,
images), while stripping anything script-capable. Applies to all three
surfaces (document preview + compact/inline intent body).
Tests: 17 DOM-free unit tests for the parse contract (GFM structure, link
safety, inline-HTML passthrough); two e2e scenarios in code-tab.feature cover
the rendered GFM/inline-HTML structure and sanitization in a real browser.
CODEX's verdict was substantively correct on all five findings; I agreed with and fixed every one. The core fix is a tightened sanitizer: I replaced DOMPurify's broad defaults with an explicit Markdown-only allowlist, threaded the link policy through the sanitize pass, and scoped the raw-HTML/image surface per variant (document only). Verified the sanitizer behavior live against real DOMPurify 3.4.8 + jsdom (9/9 behavior assertions pass), typechecked the package clean (tsc exit 0), and ran the existing unit suite (17/17 pass). Added 3 e2e scenarios covering the new guarantees and updated README to match.
One important correction to CODEX's suggested approach: I deliberately did NOT use `USE_PROFILES:{html:true}` to "disable SVG/MathML". In DOMPurify 3.4.8's config parser the USE_PROFILES block runs AFTER and OVERWRITES any explicit ALLOWED_TAGS/ALLOWED_ATTR with the full html profile — which would have silently re-admitted style/class/input and defeated the whole allowlist. An explicit ALLOWED_TAGS array alone already replaces the default html+svg+mathml base, so SVG/MathML are excluded just by absence. I documented that trap in a code comment.
Files changed are under /home/srid/code/kolu/.worktrees/md-preview-improv/.worktrees/be-review-1780504728986-codex.
codex (round 1) findings:
- [F1 · blocking] The sanitizer relies on DOMPurify's broad default HTML/SVG/MathML tag and attribute allowlist while only forbidding a few tags. DOMPurify still allows attributes such as `style` and `class`, plus interactive/media/SVG elements by default; because the sanitized result is inserted into Kolu's live DOM, an untrusted README can apply global Tailwind classes or inline CSS, render focusable controls/media, or otherwise visually take over the app. This also makes README.md's claim that styles are stripped false. (packages/solid-markdown/src/sanitize.ts:20)
- [F2 · major] Inline-HTML anchors bypass the renderer's link policy. `render.ts` only applies `safeHref`, `target="_blank"`, and `links=false` handling to Markdown link tokens; raw `<a>` tags that survive DOMPurify are only patched when they already have a `target`. That means inline slots with `links={false}` can still render real anchors, and document-preview HTML anchors without `target` can navigate the Kolu tab itself. (packages/solid-markdown/src/sanitize.ts:63)
- [F3 · major] The raw-HTML/image pipeline is now used for every variant, including compact and inline intent renderers. Those surfaces previously escaped HTML and rendered images as inert text; now user/agent intent text or fallback labels can inject README-style HTML/images into dock rows, title bars, switcher cards, and intent bodies, which are compact clickable UI surfaces rather than document previews. (packages/solid-markdown/src/Markdown.tsx:53)
- [F4 · minor] Inline code in the `inline` variant loses the intended chip styling. `parseInline` emits `<code>` as a direct child of the `.kolu-md-inline` root, but selectors like `.kolu-md :not(pre) > code` and `.kolu-md-inline :not(pre) > code` require the code's parent to be a descendant of the root, so direct-child inline code only gets the base monospace rule. (packages/solid-markdown/src/markdown.css:152)
- [F5 · minor] The package exports `./render`, which is explicitly the raw, unsanitized HTML renderer. That creates a public footgun: consumers can import the package subpath and bypass DOMPurify, while the safe sanitizer is not exported as the paired API. (packages/solid-markdown/package.json:10)
claude:
- F1 fixed: Agreed. Rewrote packages/solid-markdown/src/sanitize.ts to use an explicit, tight Markdown-only allowlist (DOCUMENT_TAGS/DOCUMENT_ATTR and INTENT_TAGS/INTENT_ATTR) instead of DOMPurify defaults. Confirmed against the installed DOMPurify 3.4.8 source that the defaults admit `style`/`class`/`id` attributes, SVG+MathML profiles, and interactive/media elements (button/input/select/textarea/video/audio) — the old config only FORBID a handful of tags. New config: ALLOWED_ATTR omits style/class/id entirely; ALLOWED_TAGS contains no SVG/MathML/media; a post-pass removes any non-checkbox <input> and disables kept task checkboxes. Critical subtlety: I did NOT add USE_PROFILES — DOMPurify 3.4.8's _parseConfig runs the USE_PROFILES block (src ~37899) AFTER reading the explicit cfg.ALLOWED_TAGS (~33164) and reassigns ALLOWED_TAGS=addToSet({},text) then addToSet(html$1), clobbering the tight list and silently re-admitting style/class/input. An explicit array already replaces the default html+svg+mathml base, so SVG/MathML drop out by absence. Verified live with jsdom: <p style class id> -> <p>, <svg> -> removed, <button> -> unwrapped, <input type=text> -> removed, checkbox -> kept+disabled. Added e2e scenario 'drops style, class, SVG, and form controls'. Also corrected/strengthened the README claim (styles/SVG/form controls now genuinely stripped).
- F2 fixed: Agreed on behavior; one framing correction. Added applyLinkPolicy() in sanitize.ts applied to EVERY anchor via querySelectorAll('a'), not just a[target]. links=false -> anchor unwrapped to its child nodes (text survives, anchor gone); links=true -> href re-validated through the SAME safeHref allowlist imported from ./render, invalid href unwrapped, valid href forced to target=_blank rel='noopener noreferrer'. This closes both gaps CODEX named: raw <a href='relative.md'> previously survived with no target and could navigate the Kolu tab; raw <a> previously rendered live in links={false} inline slots. Framing note for the record: a raw <a href='javascript:..'> was NOT an XSS vector — DOMPurify's default IS_ALLOWED_URI already strips such hrefs (confirmed in the 3.4.8 source regex), so my applyLinkPolicy then unwraps the now-href-less anchor to text. The fix is real on the navigation/policy axis CODEX cared about. Added e2e scenario 'applies the link policy to raw inline anchors' asserting a[target=_blank], a[rel~=noopener], and absence of a[href^=javascript].
- F3 fixed: Agreed. Confirmed via the base commit that the OLD renderer escaped html tokens as text and never rendered inline-HTML images, across ALL variants — so the new uniform raw-HTML+image pipeline was a real behavior change for the compact/inline intent surfaces (dock rows, title bars, switcher cards, intent body), which IntentMarkdownBlock/IntentMarkdownInline feed with user/agent text. Added a richHtml axis to SanitizeOptions; Markdown.tsx sets richHtml = (variant === 'document'). Only the full-pane document preview now gets the README inline-HTML + image surface; compact/inline use INTENT_TAGS (no raw block HTML containers, no img, no input) and INTENT_ATTR (href/title only), restoring their prior no-injection posture. Verified with jsdom that intent scope drops <img> and <details> to their text. Updated the README architecture row to document the per-variant scoping.
- F4 fixed: Agreed — real CSS bug. Confirmed with marked 18.0.2 that parseInline('`code`') emits '<code>code</code>' as a DIRECT child of the inline root (no <p> wrapper), while block parse wraps in <p>. The selectors '.kolu-md :not(pre) > code' and '.kolu-md-inline :not(pre) > code' use a descendant combinator that requires the code's :not(pre) parent to be a DESCENDANT of the root — but here the parent IS the root, so direct-child inline code matched only the base '.kolu-md code' monospace rule and lost the chip padding/background. Fixed packages/solid-markdown/src/markdown.css by adding the direct-child arm to both rules: '.kolu-md > code' and '.kolu-md-inline > code', with an explanatory comment.
- F5 fixed: Agreed. Removed the public './render' subpath export from packages/solid-markdown/package.json, leaving only the '.' entry. Confirmed there are no external importers of '@kolu/solid-markdown/render' anywhere in the repo, and render.test.ts imports the package-relative './render' path (unaffected by removing the public subpath). This closes the footgun of a consumer importing the raw, pre-sanitization HTML renderer and bypassing DOMPurify; safe parsing helpers stay internal, and the only public surface is the sanitized <Markdown> component.
Committed by the codex<->claude debate (round 1); not pushed or merged.
CODEX's F1 was technically correct and I fixed it; F2-F5 were already marked resolved by CODEX and I confirmed each in the diff. Verified F1 against the bundled DOMPurify 3.4.8 source: purify.cjs.js:546-548 default ALLOW_ARIA_ATTR/ALLOW_DATA_ATTR to true, and :1314 short-circuits the ALLOWED_ATTR check for any data-*/aria-* name — so the explicit ALLOWED_ATTR list alone did NOT exclude them. The README claims a "tight Markdown-only allowlist" twice (README.md:79, :202), so leaving these permissive contradicts the documented policy. Fixed in configFor() by adding ALLOW_DATA_ATTR:false and ALLOW_ARIA_ATTR:false plus an explanatory comment. Markdown emits neither attribute, so no rendering regression; the change just makes the attribute set exactly DOCUMENT_ATTR/INTENT_ATTR as claimed. Ran prettier on the touched file (unchanged/clean). codex (round 2) findings: - [F1 · major] The sanitizer now supplies explicit `ALLOWED_ATTR`, but DOMPurify 3.4.x still permits `data-*` and `aria-*` attributes unless `ALLOW_DATA_ATTR` / `ALLOW_ARIA_ATTR` are set false. That leaves arbitrary attributes outside `DOCUMENT_ATTR` / `INTENT_ATTR` surviving on untrusted Markdown HTML, which contradicts the claimed tight Markdown-only attribute policy and can interact with app-level data-attribute hooks. (packages/solid-markdown/src/sanitize.ts:152) - [F2 · major] Previously, raw HTML anchors bypassed the Markdown renderer's link policy. (packages/solid-markdown/src/sanitize.ts:179) - [F3 · major] Previously, compact/inline intent surfaces inherited the document preview's raw HTML and image surface. (packages/solid-markdown/src/Markdown.tsx:58) - [F4 · minor] Inline code emitted as a direct child of the inline root missed the intended chip styling. (packages/solid-markdown/src/markdown.css:156) - [F5 · minor] The package exported the raw pre-sanitization render module as a public subpath. (packages/solid-markdown/package.json:10) claude: - F1 fixed: Agreed and fixed. Confirmed via bundled dompurify@3.4.8 source: purify.cjs.js:546 `ALLOW_ARIA_ATTR = true` and :548 `ALLOW_DATA_ATTR = true` are the defaults, and _isValidAttribute at :1314 admits any name matching the data-*/aria-* regex BEFORE consulting ALLOWED_ATTR — so the explicit ALLOWED_ATTR allowlist alone left arbitrary data-*/aria-* attributes surviving on untrusted Markdown HTML. This contradicts the 'tight Markdown-only allowlist' the README states at README.md:79 and :202. Fix: added `ALLOW_DATA_ATTR: false` and `ALLOW_ARIA_ATTR: false` to the DOMPurify config in configFor() at packages/solid-markdown/src/sanitize.ts:162-163, with a comment explaining the short-circuit. Markdown produces no data-*/aria-* attributes, so there is no rendering regression; the attribute set is now exactly DOCUMENT_ATTR/INTENT_ATTR. - F2 fixed: Already resolved (CODEX marked status=resolved) and confirmed in the working tree: applyLinkPolicy() at packages/solid-markdown/src/sanitize.ts:179-191 runs over every sanitized anchor (loop at :206-208), unwraps anchors when links are disabled or when safeHref(href) is undefined, and stamps kept anchors with target="_blank" + rel="noopener noreferrer". This covers raw inline <a>, not just renderer-minted anchors. No further change needed. - F3 fixed: Already resolved (CODEX marked status=resolved) and confirmed: richHtml is only true for the document variant, INTENT_TAGS (packages/solid-markdown/src/sanitize.ts:98-129) excludes img/input/table/details/div and INTENT_ATTR is just href/title — so compact/inline intent surfaces no longer inherit the document preview's raw HTML/image surface. No further change needed. - F4 fixed: Already resolved (CODEX marked status=resolved). The minor inline-code chip styling was a CSS issue in markdown.css and CODEX confirmed the direct-child `> code` arms are present for both base and inline roots. Not part of the F1 change; no further action needed. - F5 fixed: Already resolved (CODEX marked status=resolved). The public ./render subpath export was removed from package.json; only the sanitized component entrypoint is exported, so the raw pre-sanitization render module is no longer publicly reachable. No further action needed. Committed by the codex<->claude debate (round 2); not pushed or merged.
…— consumer must reach into package source for `markdown.css` Routed `@kolu/solid-markdown`'s stylesheet through a real package export, replacing the reach into the package's src tree with a stable `@import "@kolu/solid-markdown/markdown.css"`. Agreed by the lowy ⇄ hickey lens debate (finding lowy-2, raised by lowy). Not pushed or merged.
…cale is keyed off a different attribute axis (`.kolu-md-inline` class) Unified the markdown styling-scale axis: all three variants (document/compact/inline) now key off data-md-variant; the inline span emits data-md-variant and the .kolu-md-inline class is dropped. Agreed by the lowy ⇄ hickey lens debate (finding lowy-3, raised by lowy). Not pushed or merged.
…escape is the canonical leaf Replaced solid-markdown's hand-rolled escapeHtml with the canonical @kolu/html-escape leaf package. Agreed by the lowy ⇄ hickey lens debate (finding hickey-1, raised by hickey). Not pushed or merged.
…ckage no longer ships Tailwind classes Deleted the stale `@source "../../solid-markdown/src"` directive and its misleading comment block from index.css; markdown.css @import is now the sole solid-markdown styling wiring, and the solid-fileview @source is preserved. Agreed by the lowy ⇄ hickey lens debate (finding hickey-3, raised by hickey). Not pushed or merged.
Member
Author
🛡️ Review gauntlet — codex ∥ lens ∥ policeRan the three reviewers in parallel (each in its own worktree off HEAD, debating to consensus). codex and lens consolidated cleanly onto the branch (6 commits, no overlap conflicts); police ran to consensus but its commits were not auto-consolidated because the orchestrator crashed in its Report phase on a runtime codex ⇄ claude — consensus after 2 rounds (security hardening) ✅The strongest track: it found the rebuild leaned on DOMPurify's broad defaults and tightened the whole sanitize posture.
codex also added 3 e2e scenarios (tight-allowlist drop, raw-anchor link policy) and corrected the README claims. lowy ⇄ hickey — consensus (structural) ✅
code-police — ran to consensus, adjudicated manually ⚖️Not auto-consolidated (the Consolidation ledger
|
Member
Author
EvidenceThe rebuilt preview rendering a real-world README (headings, GFM table with per-column alignment, nested task lists, fenced code, blockquote, Dark Light The same fixture also confirms sanitization in the live DOM:
|
…e repo images
A feature audit (and user reports) surfaced gaps the first pass missed — the
e2e even went green while lists rendered with no bullets:
- Lists had no markers. Tailwind v4's preflight resets `list-style:none`
app-wide; markdown.css restored padding but never re-declared the marker.
Now `ul`→disc / `ol`→decimal (nested circle→square), with `ol start`
preserved.
- Footnotes (`[^1]`) and GitHub alerts (`> [!NOTE]`) were unsupported and
leaked as literal text. Registered marked-footnote, marked-alert, and
marked-gfm-heading-id (stable heading/footnote ids). Alert class markup is
rewritten to an allowlist-safe `data-md-alert` attribute (class stays
forbidden); the icon comes from CSS, not the stripped octicon SVG.
- Repo-relative images degraded to a chip even when the file exists. A new
`resolveImageSrc` seam resolves a relative src against the document's
directory (GitHub-style) and points it at the per-terminal file route
(`buildTerminalFileUrl`, now shared in kolu-common), so README images load.
- Leading YAML front-matter rendered as a stray `<hr>` + heading — now
stripped before parse.
- Allowlist completeness: `dl`/`dt`/`dd`, `figure`/`figcaption`, `caption`,
`section`, plus `start`/`width`/`height`/`colspan`/`rowspan`/`open`/`id`.
Every id is namespaced (`md-`) post-sanitize so a document's anchors can't
collide with the app's; in-page anchors scroll within the preview.
Security posture is unchanged: no `class`/`style`/SVG/script survives (verified
in-browser: 0 content class/style attrs). New coverage: render unit tests for
footnotes/alerts/front-matter/heading-ids, the image-resolver unit test, and an
e2e asserting list markers, footnotes, alerts, and route-resolved images.
Member
Author
Round 2 — feature audit + fixesGood catch on the lists. A parallel feature-audit workflow (6 auditors per category — lists, blocks, tables, inline, GFM-extras, allowlist — each running
Security is unchanged — verified in-browser that 0 Why the first e2e didn't catch it: it asserted structure ( Evidence (rebuilt, both themes)A document exercising lists (disc/circle/square + Lists + resolved image (light) Alerts + definition list + footnotes (light) Dark
|
…age resolution Round 2 added repo-relative image resolution, so the inline <img src=docs/logo.png> in this scenario now renders as a real <img> at the per-terminal file route instead of degrading to a fallback chip. Update the stale 'should not render a img' / 'should contain brand-logo' (chip-era) assertions to assert the resolved route src. (1 of 404 e2e scenarios; everything else was green.)
Closed
srid
added a commit
that referenced
this pull request
Jun 3, 2026
…rkflow runtime (#1156) ## The bug A `/be-review` run on #1155 crashed in its **Report** phase with `ReferenceError: TextEncoder is not defined`, so no per-track comments were posted and the report had to be reconstructed by hand — **with no commit links** ([example](#1155 (comment))). Root cause: the Report phase base64-encodes each comment body so the mechanical commenter agent treats it as **opaque data**, not prompt instructions (the F3 prompt-injection fix from #1153). `toBase64` fell back to `new TextEncoder()` when `Buffer` was undefined — but the **Workflow runtime has none of `Buffer`, `TextEncoder`, or `btoa`** (the same class of restriction as `Date.now()`/`Math.random()`). So the fallback threw and took down the whole Report phase. ## The fix Replace the fallback with a **self-contained pure-JS UTF-8→base64 encoder** that uses only `encodeURIComponent` (always present) + a hand-rolled base64 alphabet — no runtime globals. The `Buffer` fast-path stays behind a safe `typeof` guard for runtimes that have it. ## Verification - Byte-for-byte **identical to `Buffer.from(s,'utf8').toString('base64')`** across ASCII, multibyte (`café`, `résumé`), emoji (`🤖🔴`), markdown special chars (`| \n \` $`), and empty string. - **Round-trips** cleanly through `base64 -d` (the decode side `postComment` runs). - Full-file syntax check passes; `.apm`→generated in sync; `just fmt` clean. This is the bug that blocks reporter-authored comments (incl. the `remapCommits` SHA-resolution from #1153) from ever reaching the PR — once Report stops crashing, those land with resolving commit links. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
…ive task lists
Three more from the feature-audit list:
- **Syntax highlighting** — fenced code blocks render through Shiki (lazy
`import("shiki")`, dual github-light/dark theme via CSS variables so it
follows the app theme with no re-highlight) plus a copy button. The fence
language rides `data-lang` (the allowlist forbids `class`); Shiki escapes the
code, so its themed output is injected past the allowlist as trusted markup.
- **GitHub-faithful soft breaks** — the document preview now parses with
`breaks:false` (a single newline folds to a space, like github.com); the
chat/dock intent scale keeps message-style hard breaks.
- **Interactive task lists** — a checkbox click in the document preview writes
the flipped marker back to the file via a new path-guarded `fs.writeFile`
mutation; the working-tree watcher re-yields the content and the preview
re-renders the toggled box. (`disabled` had to be allowlisted so the
marked-task signal survives DOMPurify.)
Also documents, in `solid-markdown/LIMITATIONS.md` (linked from render.ts),
what's still unsupported (math, mermaid, emoji shortcodes, @/# autolinks, and
the non-GitHub ecosystem syntaxes) so the gaps are recorded in the code.
Verified in-browser (both themes): highlighted TS/Python with theme swap, a
3-line paragraph folded to one, and a task toggle writing `[ ]`→`[x]` to disk
(git diff confirms). Tests: render unit tests for data-lang + breaks, a
fence-aware `toggleTaskInSource` unit test, and an e2e asserting `.shiki`, the
copy button, folded breaks, and a task toggle round-tripped through the source
view.
Member
Author
Round 3 — syntax highlighting, GitHub soft breaks, interactive task listsClosing the three highest-value gaps from the audit (the "do 1, 2 and task-lists" list):
And — per the ask — what's still unimplemented is now documented in the code: Verified in-browser, both themes — TS + Python highlighted with the theme swap, a 3-line paragraph folded to one line, and a task toggle that wrote EvidenceLight — highlighted TypeScript + Python, folded paragraph, two now-checked interactive task boxes: Dark — the same code, Shiki's |
… checkbox The source-view-switch assertion was flaky (a write/switch race). Assert the re-rendered checkbox's checked state instead — the box only flips after the write → watcher → re-render round-trip, so it's the same proof in one surface.
…of plugging into resolveExistingUnder router.ts fs.writeFile now uses resolveExistingUnder (one combined path guard) instead of hand-wiring resolveUnder + assertRealpathUnder Agreed by the lowy ⇄ hickey lens debate (finding lowy-1, raised by lowy). Not pushed or merged.
…URL-scheme volatility — sanitize imports it from render across the DOM-free boundary Extracted safeHref + isLoadableImage into new url-policy.ts; render.ts and sanitize.ts now import from it, removing the cross-layer back-import into the DOM-free parser. Agreed by the lowy ⇄ hickey lens debate (finding lowy-2, raised by lowy). Not pushed or merged.
…apsulation of "languages we highlight" Dropped redundant "bash" from SUPPORTED (now new Set<string>(LANGS)), exported LANGS/ALIAS, and added highlight.test.ts asserting every ALIAS value is in LANGS so alias→grammar drift fails loudly. Agreed by the lowy ⇄ hickey lens debate (finding lowy-5, raised by lowy). Not pushed or merged.
…duplicates the sanitize pass Collapsed the markdown link policy to a single site: deleted render.ts's custom link renderer (marked now emits default anchors) so applyLinkPolicy in sanitize.ts is the only link-policy place, and updated tests/docs accordingly. Agreed by the lowy ⇄ hickey lens debate (finding hickey-1, raised by hickey). Not pushed or merged.
…pendently re-derive "is this a script-capable / non-local URL" Factored the "carries its own origin/scheme" decision into shared hasOwnScheme() in url-policy.ts and routed resolveMarkdownImageSrc's early-bail through it (hickey-3). Agreed by the lowy ⇄ hickey lens debate (finding hickey-3, raised by hickey). Not pushed or merged.
…, container-wrapping, and copy-button minting in one function Split enhanceCodeBlock into highlightInto (trusted-injection seam) and wrapWithCopyButton (decoration), composing them under the early-return guard. Agreed by the lowy ⇄ hickey lens debate (finding hickey-4, raised by hickey). Not pushed or merged.
…nsion-plugin assembly into one builder Split buildMarked's braided concerns: lifted the constant GFM extension stack and the per-slot code-fence renderer override into separate useGfmExtensions/useCodeFenceRenderer setups (hickey-5; links axis already removed by hickey-1). Agreed by the lowy ⇄ hickey lens debate (finding hickey-5, raised by hickey). Not pushed or merged.
Made the task-toggle regex CRLF-tolerant by capturing an optional trailing \r and re-emitting it, fixing silent no-op toggles in CRLF markdown files; added a CRLF test. code-police rules finding police-r1-rules-1 [minor]. Applied by the /be parallel gauntlet; not pushed or merged.
Added a best-effort comment and console.warn to the Shiki highlightCode catch block so highlighting failures degrade gracefully but remain diagnosable. code-police rules finding police-r1-rules-2 [minor]. Applied by the /be parallel gauntlet; not pushed or merged.
Added a .catch handler to copyCodeBlock's clipboard writeText so a rejected copy is logged via console.warn instead of surfacing as an unhandled rejection. code-police rules finding police-r1-rules-3 [nit]. Applied by the /be parallel gauntlet; not pushed or merged.
…ly truncates the file to its first 1 MB Make Markdown task checkboxes presentational when the source is truncated (omit onToggleTask), preventing a toggle from writing the 1 MB prefix back over the full file; added e2e assertions. code-police fact-check finding police-r1-fact-check-1 [blocking]. Applied by the /be parallel gauntlet; not pushed or merged.
…dered+indexed but invisible to the source scan, so a click toggles the wrong line Allow an optional blockquote prefix in taskToggle's TASK regex so the source scan indexes blockquoted GFM checkboxes in the same order the renderer does, fixing the task-index desync that toggled the wrong line. code-police fact-check finding police-r1-fact-check-2 [major]. Applied by the /be parallel gauntlet; not pushed or merged.
…amic> Collapsed the duplicated <Show> span/div branches in Markdown.tsx to a single <Dynamic> element, so the four-attribute quad lives in one place. code-police elegance finding police-r1-elegance-1 [minor]. Applied by the /be parallel gauntlet; not pushed or merged.
Inlined the richHtml alias: deleted `const richHtml = () => isDocument()` and pass `richHtml: isDocument()` directly at the use site, moving the explanatory comment there. code-police elegance finding police-r1-elegance-2 [nit]. Applied by the /be parallel gauntlet; not pushed or merged.
…ode block → wrong line toggled Fixed FENCE regex to tolerate blockquote prefix so blockquoted fenced code blocks are skipped, preventing task-toggle count drift; added a covering test. code-police rules finding police-r2-rules-1 [major]. Applied by the /be parallel gauntlet; not pushed or merged.
… preview instead of degrading to plain code Catch shiki highlighter load failure in the createResource fetcher (resolve to null + warn) so an errored resource never re-throws inside the html memo and blanks the preview; code degrades to plain instead. code-police rules finding police-r2-rules-2 [major]. Applied by the /be parallel gauntlet; not pushed or merged.
…ew when a leading YAML front-matter block contains a task-marker-shaped line Aligned the task-toggle source scanner with the rendered preview's index space: strip leading YAML front-matter before scanning, and only treat first-child-of-li checkboxes as interactive tasks so a raw body <input disabled> no longer shifts indices. code-police fact-check finding police-r2-fact-check-1 [major]. Applied by the /be parallel gauntlet; not pushed or merged.
…separated) task lists: not clickable, and they drift the data-md-task index so a click toggles the WRONG line Fixed isMarkedTaskCheckbox to detect loose (blank-line-separated) task lists by accepting both <li><input> (tight) and <li><p><input> (loose) shapes, restoring interactive toggling and keeping data-md-task indices congruent with the source scanner; added unit + e2e coverage. code-police rules finding police-r3-rules-1 [blocking]. Applied by the /be parallel gauntlet; not pushed or merged.
…ranch permits a write-escape via symlink Replaced the fail-open read guard on the fs.writeFile RPC with a write-side parent-realpath guard plus O_NOFOLLOW open, closing the symlink write-escape. code-police fact-check finding police-r3-fact-check-1 [major]. Applied by the /be parallel gauntlet; not pushed or merged.
…identical expression Extracted a local unwrap() in applyLinkPolicy so the anchor-unwrap operation is defined once and called from both early-return branches. code-police elegance finding police-r3-elegance-1 [nit]. Applied by the /be parallel gauntlet; not pushed or merged.
Removed the unreachable `?? src` fallback on String.split in basename (split with a positive limit always yields at least one element) code-police elegance finding police-r3-elegance-2 [nit]. Applied by the /be parallel gauntlet; not pushed or merged.
…ts every failure as "path escapes repo root" writeFile open() catch now binds and logs the errno at error level and surfaces a faithful message — only ELOOP is reported as a path escape, all other failures (EACCES/EISDIR/EROFS/ENOSPC/etc.) report their real cause. code-police rules finding police-r4-rules-1 [major]. Applied by the /be parallel gauntlet; not pushed or merged.
…(log arg omitted, generic rethrow message) Pass module `log` into resolveForWriteUnder and add a log.warn in the rejection branch so write-endpoint path-escape attempts leave server-side log evidence. code-police rules finding police-r4-rules-2 [minor]. Applied by the /be parallel gauntlet; not pushed or merged.
…ing a checkbox toggles the WRONG source line (or silently no-ops) Tightened the task-toggle scanner to mirror marked: TASK regex now requires space+non-empty text after `]` and FENCE handling enforces the CommonMark fence-length rule, fixing wrong-line/no-op checkbox toggles. code-police fact-check finding police-r4-fact-check-1 [major]. Applied by the /be parallel gauntlet; not pushed or merged.
…ated gauntlet The /be-review gauntlet consolidated lens + police (24 commits: truncation data-loss, symlink write-escape, task-scanner robustness, dedups) but crashed in its Report phase before codex's track landed. Applying codex's genuinely- uncovered findings: - F4 (raw-HTML boundary): the documented document-only raw-HTML scope wasn't enforced — compact/inline intent slots rendered raw `<h1>`/`<pre>`/`<a>` (still DOMPurify-sanitized, but past the boundary). Gated at marked's `html` token hook: escape raw HTML when `rawHtml` is off (intent), pass through for the document preview. - F5: don't load the Shiki chunk for a code-less README (gate the resource on a fenced-code predicate). - F7: decode URL-escaped image-path segments before re-encoding, so `my%20images/logo.png` isn't double-encoded to `my%2520…` and 404'd; reject a separator/traversal smuggled through an escape. Also fixes two issues in the consolidated state that `just check`/unit (deferred by the gauntlet) surfaced: police's `basename` `?? src` removal broke `noUncheckedIndexedAccess`, and lens's root-import of `hasOwnScheme` pulled the DOM component into the node-tested `markdownImageSrc` — re-pointed to the DOM-free `/url-policy` subpath. (codex F2 terminal-scoped-write was disputed by its own round-1 author as inconsistent with the fs surface and is covered by police's markdown-only narrowing; noted, not applied.)
Member
Author
🛡️ Review gauntlet #2 (full PR diff) — codex ∥ lens ∥ policeA second parallel gauntlet over the whole 2,254-line diff. It found real bugs the feature work missed — this is the high-value pass. The orchestrator again crashed in its Report phase on the engine's code-police — 17 commits consolidated ✅The standout track. Caught two blocking bugs plus a thorough task-scanner correctness sweep:
lowy ⇄ hickey — 7 commits consolidated ✅Structural dedup: extracted the URL-scheme check to a DOM-free codex ⇄ claude — preserved (crash), applied by hand ⚖️codex found 7 issues; F1/F3 overlapped police (truncation, symlink — already landed). The genuinely-uncovered ones I applied:
Adjudicated, not applied: F2 (make the write RPC terminal-scoped) — codex's own round-1 author disputed it as inconsistent with the fs surface (every Consolidation ledger
Plus two fixes the gauntlet's own deferred |
…s e2e selectors The gauntlet's police track added loose-list / raw-body / congruence scenarios asserting `input[data-md-task=N]` — a bare numeric attribute value is not a valid CSS selector, so querySelector threw. Single-quote it (`='N'`), which is valid CSS and survives gherkin's double-quoted step string.
srid
marked this pull request as ready for review
June 3, 2026 22:22
Drop the interactive task-list write-back: the preview no longer modifies files. GFM task checkboxes still render with their [x]/[ ] state but are presentational (disabled), the way GitHub renders a README's task list. Removes the fs.writeFile RPC and its safe-path write authority (resolveForWriteUnder), the client-side source scanner (taskToggle), the onToggleTask wiring through solid-fileview/solid-markdown, and the related e2e/police interactivity scenarios. Highlighting, copy buttons, soft-break folding, and in-page anchors are unchanged.
srid
changed the title
srid
added a commit
that referenced
this pull request
Jun 11, 2026
**Every Atlas note re-audited against current master and GitHub state; 93 confirmed staleness items fixed across 22 notes.** Driven by a two-stage agent workflow: one auditor per note checked every factual claim (status pills, PR states via `gh`, code cites against the working tree), then an adversarial verifier independently re-checked each finding before any edit — 13 suggested fixes were corrected or rejected at that stage. ### The load-bearing corrections - **`remote-terminals` / `pty-daemon-tui`** — the R-4 row now credits kolu-tui Phases 0–2 (#1073 / #1084 / #1255, the last merged today); `list --json` dropped from the Phase 3 row (it shipped in Phase 1); the attach loop, `requirePty` NOT_FOUND nicety, and package-size figure recast from plan tense to shipped history. *Next in remote-terminals remains pty-daemon **Phase B** — both notes already said so correctly.* - **`anyforge`** — un-parented from `remote-terminals` (multi-forge is not part of that feature — it was misfiled at birth); phase 0b (#1257) marked shipped; the pre-extraction code claims (`startGitHubPrProvider`, the kolu-common→kolu-github wire coupling, the schemas-header promotion note) recast to past tense with cites re-pointed. - **Everything else** — stale "todo/next" pills for work that shipped (#1093, #1155, #1162, #1190, #1191, #1199, #1212, #1216, #1219, #1231 …), dead cites to moved/deleted files (`iframePreviewNav.ts`, `.claude/rules/workflow.md`, drifted line pins), and internal contradictions left by partial past updates. `herdr-vs-kolu` alone had 14. > **Bug found along the way:** three notes' frontmatter `description:` contained ` #NNNN` as an unquoted YAML scalar — YAML treats whitespace+`#` as a comment start, so the rendered meta descriptions were silently truncated mid-sentence. Those descriptions are now quoted (`mini-ci-vs-justci`, `nix-typecheck-gate`, `pty-daemon`). _Eight notes audited clean with zero findings (`pty-daemon`, `surface-connection`, `surface-mcp`, `correctness-review`, `ghostex-vs-remote-terminals`, `md-preview-relative-links`, `md-preview-wikilinks`, `pty-daemon-chrome-bar`)._ `dist/` regenerated via `just atlas::build`; `check-sync` green locally. _Generated by an ultracode audit workflow on Claude Code (model `claude-fable-5`)._
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Code-tab's rendered Markdown preview was a hand-rolled
markedtoken walker that escaped every piece of inline HTML, flattened headings, dropped list markers, and rendered with cramped spacing — so a normal README showed<p align="center">/<img>as literal text, no tables, unmarked lists, and leaked[^1]/> [!NOTE]/code as plain text. This rebuilds the renderer in@kolu/solid-markdownon a real pipeline: full GitHub-Flavored Markdown, sanitized inline HTML, syntax-highlighted code, repo-image resolution, read-only task lists, and styling that follows the app's light/dark preference.Pipeline
What renders now
hr> [!NOTE]alerts, leading YAML front-matter stripped, GitHub-faithful soft breaks (the document preview folds a single newline to a space; chat/dock keep message-style breaks)[x]/[ ]state but are presentational (disabled), the way GitHub renders a README's task list; the preview never writes back to the file<details>,<kbd>,<sub>/<sup>,<p align>, definition lists, figures, images;<script>/<style>/<iframe>/classstripped,javascript:links inert<img>/![]()resolves against the doc's directory and loads from the per-terminal file route, with a labelled-chip fallbackcurrentColor+ the app accent, adapting to either palette (see Evidence)Security model (hardened in review)
The parallel review gauntlet (codex ∥ lowy⇄hickey ∥ police) tightened the sanitizer to an explicit Markdown-only allowlist — no
style/class/id-as-written, no SVG/MathML, no media;ALLOW_DATA_ATTR/ARIA_ATTRoff; everyidnamespaced (md-). Newly-allowed attributes are all inert/structural. Verified in-browser: 0class/style/script/svgsurvive from document content. Shiki output is injected past the allowlist only because it's generated from escaped code (trusted by construction); external links are forced totarget=_blank rel=noopener, and in-page anchors scroll within the preview. The preview is read-only — it has no write path back to the repo.Read-only by design
An earlier revision wired interactive task-list checkboxes that wrote the toggle back to the file through a path-guarded
fs.writeFileRPC. That write surface (and its source-scanner/index-congruence machinery) has been removed: the preview is now purely a reading view. Checkboxes still render their state — just presentational, like GitHub.What's NOT supported
Catalogued in
packages/solid-markdown/LIMITATIONS.md(linked fromrender.ts): math/LaTeX, mermaid, emoji shortcodes,@mention/#issue/ SHA autolinks, and the non-GitHub ecosystem syntaxes (==mark==,^sup^, deflist:syntax, …).Known edge — repo-relative links (e.g.
[doc](./OTHER.md)) aren't yet resolved to repo files the way images are; like any other link they open in a new tab, so a relative path lands on the app origin rather than the target file. Opening such a link in the Code tab is deferred to a follow-up — tracked in #1161.Tests
data-lang, soft-break policy; 25 inrender.test.ts), the image-path resolver (9), and a Shiki highlighter smoke test.code-tab.feature) — rendered structure, the tight-allowlist drop, raw-anchor link policy, sanitization, computed list markers, footnotes/alerts, route-resolved images,.shikihighlighting + copy button, folded soft breaks, and read-only (disabled) task checkboxes.Try it locally
Generated by
/beon Claude Code (modelclaude-opus-4-8).