Skip to content

Commit ad18adc

Browse files
cscheidclaude
andcommitted
fix(render): route destructive writes through validated OutputSink (bd-cfl67)
`q2 render` was truncating source images referenced from qmd documents to 0 bytes. ResourceCollectorTransform stored the absolute source path as an artifact destination with empty content; on_disk_path_for's `scope_root.join(absolute_path)` silently returned the absolute path unchanged; write_artifacts then opened the source file with truncating write semantics. Introduce OutputSink as the single audited surface for every destructive disk op during render. Producers enqueue Write / Copy intents; the sink validates against declared allowed_roots (lexical at enqueue, canonical at flush with /var↔/private/var aliasing handled) and silently skips Copy ops whose canonical src and dest match. Threaded through both the native render_document_to_file (one sink per render) and the WASM Pass-2 renderers. Defense in depth: Artifact::from_path / with_path debug-assert relative paths; on_disk_path_for debug-asserts relative artifact_path; sink's allowed_roots is the release-mode net. Rewrite ResourceCollectorTransform to emit (src, dest) pairs on RenderContext::resource_copies (no artifact-store route). In VFS-root mode the producer skips emission — the hub-client's parent-side asset walker reads bytes from VFS source paths directly. Tests: - R1 / F4: end-to-end renders preserve source image bytes and copy the image to _site/ at the position the rendered HTML references. - R2: on_disk_path_for #[should_panic] on root-prefixed paths. - R3/R4 + 7 more: OutputSink unit tests. - R5 + collector unit tests: emits resource_copies, dedupes URLs, skips external/data URLs. - F5: CLI-level test spawning the real q2 binary. - Existing q2-preview wiring test updated to reflect the new "walker reads from VFS source directly" contract. Manual reproduction against docs/authoring/markdown/index.qmd confirms source preserved (126124 bytes, byte-identical) and image copied into _site/authoring/markdown/elephant.png. Plan: claude-notes/plans/2026-05-20-render-truncates-source-images.md Phase 3 migration of remaining destructive writers (copy_resources_to_output_dir, copy_favicon/robots, engine intermediates) is tracked as follow-up beads issues, not blocking bd-cfl67. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 5aeedd1 commit ad18adc

17 files changed

Lines changed: 1754 additions & 129 deletions

.beads/issues.jsonl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@
152152
{"id":"bd-c05x6","title":"Plumb SourceInfo into Q-13-4 body-link 'missing document' warnings","description":"Today the Q-13-4 'Body link references missing document' warning is emitted without a source location, so a user running 'q2 render' on a website sees only the missing target — not which file or line introduced the broken link.\n\nExample, running 'cargo run --bin q2 -- render docs/' on this repo's docs site today:\n\n Warning [Q-13-4]: Body link references missing document\n 'authoring/markdown/markdown-basics.qmd' is not in the project index.\n i Check the spelling, or confirm the target file is included in the render set.\n\nThere is no 'at file:row:col' line, even though the body-link rewriter has the full Link node in hand and Link.target_source.url already carries a SourceInfo populated by the parser (crates/pampa/src/pandoc/treesitter_utils/span_link_helpers.rs:109).\n\nbd-qor9a already plumbed source-info into the nav-surface Q-13-* diagnostics (sidebar / navbar / footer / page-nav) via the paired href_source field. bd-hjv5o lists this body-link follow-up as item #1 — 'lightweight follow-up' because resolve_doc_relative_href already takes an Option<SourceInfo> location parameter; the body-link callsite just passes None at crates/quarto-core/src/transforms/link_rewrite.rs:224.\n\nScope:\n\n1. Pass link.target_source.url.clone() to resolve_doc_relative_href in LinkRewriter::visit_inline.\n2. Extend the Q-13-4 unit test (link_rewrite_diagnostic_uses_body_link_label) to assert d.location.is_some() and that the location maps back to the URL substring.\n3. Extend the integration test in crates/quarto-core/tests/link_rewriting_pipeline.rs that exercises Q-13-4 to assert on location.\n4. End-to-end: re-run 'q2 render docs/' and confirm the warning gains an 'at file:row:col' line under each Q-13-4.\n\nOut of scope:\n- Other Q-13-* surfaces (already covered by bd-qor9a).\n- Image href diagnostics (images aren't checked against the index).\n- RawHtml links (no SourceInfo on the URL portion).\n\nPlan: claude-notes/plans/2026-05-20-bd-PLACEHOLDER-body-link-source-location.md","notes":"Plan written at claude-notes/plans/2026-05-20-bd-c05x6-body-link-source-location.md (draft — awaiting user iteration before implementation begins).","status":"closed","priority":3,"issue_type":"task","created_at":"2026-05-21T00:25:11.017118Z","created_by":"cscheid","updated_at":"2026-05-21T00:37:25.978349Z","closed_at":"2026-05-21T00:37:25.978171Z","close_reason":"Implementation complete. link_rewrite.rs:224 now passes link.target_source.url.clone() instead of None to resolve_doc_relative_href, so the Q-13-4 'Body link references missing document' warning carries the link URL's SourceInfo. End-to-end verified on docs/ — Q-13-4 warnings now render with Ariadne source excerpts pointing at the offending .qmd:line:col. Three new/updated tests: link_rewrite_diagnostic_carries_source_location (unit, new), pipeline_body_link_broken_qmd_emits_diagnostic + pipeline_body_link_unresolvable_in_website_warns (integration, extended). cargo xtask verify --skip-hub-build clean (all 12 steps green). Doc-comments on missing_document_warning, resolve_href_for_html, resolve_doc_relative_href updated to reflect post-bd-c05x6 reality. Plan: claude-notes/plans/2026-05-20-bd-c05x6-body-link-source-location.md (all phases checked). Changes uncommitted pending user review.","source_repo":".","compaction_level":0,"original_size":0,"dependencies":[{"issue_id":"bd-c05x6","depends_on_id":"bd-hjv5o","type":"discovered-from","created_at":"2026-05-21T00:25:11.017118Z","created_by":"cscheid","metadata":"{}","thread_id":""}]}
153153
{"id":"bd-c083","title":"Cargo: upgrade tree-sitter v0.25.10 → v0.26.8","description":"Major upgrade surfaced by cargo-upgrade survey 2026-05-04. Current 0.25.10 is range-pinned in workspace; latest is 0.26.8. Type: pre-1.0 minor (semver-breaking). Review changelog and bump deliberately. See claude-notes/plans/2026-05-04-cargo-upgrade-survey.md and bd-hb8h.","status":"closed","priority":3,"issue_type":"chore","created_at":"2026-05-04T18:15:55.321199Z","created_by":"cscheid","updated_at":"2026-05-04T20:30:44.759296Z","closed_at":"2026-05-04T20:30:44.759137Z","close_reason":"merged: 61b01cd4","source_repo":".","compaction_level":0,"original_size":0,"labels":["cargo","deps"],"dependencies":[{"issue_id":"bd-c083","depends_on_id":"bd-hb8h","type":"discovered-from","created_at":"2026-05-04T18:16:05.692502Z","created_by":"cscheid","metadata":"{}","thread_id":""}]}
154154
{"id":"bd-c3jh","title":"Phase 9 follow-up: GC stale VFS artifacts at session end","description":"When the hub-client preview re-renders, the WASM Pass-2 produces new artifact paths (theme-css fingerprints change when content changes) but the *old* artifacts under /.quarto/project-artifacts/... linger in VFS storage. The new HTML never references them — so they don't poison the page — but they do leak.\n\nGC pass at session-end (or periodically): walk /.quarto/project-artifacts/, drop any entry whose path doesn't appear in a 'live' set (the union of artifact paths from the most-recent project render).\n\nPhase 9 plan §Risks: 'Add a follow-up to GC /.quarto/project-artifacts/... entries with no live references at session end. Not a Phase-9 blocker.'","status":"open","priority":4,"issue_type":"task","created_at":"2026-04-29T00:32:31.194561Z","created_by":"cscheid","updated_at":"2026-04-29T00:32:31.194561Z","source_repo":".","compaction_level":0,"original_size":0,"dependencies":[{"issue_id":"bd-c3jh","depends_on_id":"bd-0tr6","type":"parent-child","created_at":"2026-04-29T00:32:31.194561Z","created_by":"cscheid","metadata":"{}","thread_id":""},{"issue_id":"bd-c3jh","depends_on_id":"bd-ayj6","type":"discovered-from","created_at":"2026-04-29T00:32:31.194561Z","created_by":"cscheid","metadata":"{}","thread_id":""}]}
155+
{"id":"bd-cfl67","title":"q2 render truncates source images referenced in qmd documents","description":"ResourceCollectorTransform stores image source paths as artifacts with empty content; write_artifacts then opens the source path for writing and truncates it to 0 bytes. Confirmed via 'q2 render docs/authoring/markdown/index.qmd' which truncates docs/authoring/markdown/elephant.png from 126124 bytes to 0. Plan doc: claude-notes/plans/2026-05-20-render-truncates-source-images.md","status":"open","priority":0,"issue_type":"bug","created_at":"2026-05-21T00:53:50.747194Z","created_by":"cscheid","updated_at":"2026-05-21T00:53:50.747194Z","source_repo":".","compaction_level":0,"original_size":0,"labels":["bug","data-loss"]}
155156
{"id":"bd-coffj","title":"q2 preview: Div with class='section' emits <div>, not <section> — breaks Quarto's :has(+ section) margin rules","description":"The native HTML writer (crates/pampa/src/writers/html.rs:1129-1142) emits <section>...</section> for Pandoc Div blocks whose class list contains 'section'; the React Div component in ts-packages/preview-renderer/src/q2-preview/blocks/Div.tsx always emits <div>. Consequence: Quarto theme CSS rules keyed on the <section> tag (e.g. 'main.content > p:has(+ section) { margin-bottom: 2rem }') don't apply to preview, causing visible spacing drift. Concrete repro: the 'This is an extremely basic website' paragraph in the fixture has 17px bottom margin in preview vs 34px in render. Fix: mirror the native writer — emit <section> when classes contains SECTION ('section').","status":"closed","priority":2,"issue_type":"bug","created_at":"2026-05-18T17:53:40.764259Z","created_by":"cscheid","updated_at":"2026-05-18T17:58:07.422417Z","closed_at":"2026-05-18T17:58:07.422276Z","close_reason":"Implementation complete: Pandoc Divs with class='section' now render as <section> in q2 preview, matching the native HTML writer. Visible result: 'This is an extremely basic website' paragraph margin-bottom now 34px (was 17px), matching q2 render exactly. Quarto theme's main.content > p:has(+ section) selector now matches in preview. 2 new SPA tests; 152/152 green; cargo xtask verify 12/12 green.","source_repo":".","compaction_level":0,"original_size":0,"dependencies":[{"issue_id":"bd-coffj","depends_on_id":"bd-kw93","type":"parent-child","created_at":"2026-05-18T17:53:40.764259Z","created_by":"cscheid","metadata":"{}","thread_id":""}]}
156157
{"id":"bd-cpzp","title":"qmd writer: implicit-figure path drops trailing newline, collapses next block (issue #180)","description":"Triage doc: claude-notes/issue-reports/180/triage.md on branch issue-180.\n\nRoot cause: write_figure (crates/pampa/src/writers/qmd.rs:759) implicit-figure branch returns directly from write_image without emitting the trailing \\n that every block writer is expected to produce. When such a Figure is followed by another block (top-level or inside a Div), the inter-block separator collapses to a single \\n instead of a blank line, and the re-parser glues the two blocks into one Para. Affects every layout/subfigure div in the docs corpus.\n\nReports covered (both same root cause):\n- Bug A: top-level Figure followed by Para -> one Para after round-trip\n- Bug B: Div with multiple Figure children + caption -> one Para inside the Div after round-trip\n\nFix (TDD-first per crates/pampa/CLAUDE.md):\n1. Add failing roundtrip tests under tests/roundtrip_tests/qmd-json-qmd/\n - figure_implicit_then_para.qmd\n - layout_div_subfigures.qmd\n - figure_implicit_then_figure.qmd (extra coverage)\n2. Verify they fail.\n3. Fix write_figure: replace 'return write_image(...)' with 'write_image(...)?; writeln!(buf)?; Ok(())'.\n4. Verify tests pass.\n5. cargo nextest run --workspace to catch downstream snapshot updates.\n\nNot a duplicate of bd-emr4 — that is about non-implicit Figure shapes hitting the fallback fenced-div path. This bug is in the implicit-figure code path; different code, different fix.","status":"open","priority":2,"issue_type":"bug","created_at":"2026-05-12T18:39:12.847875Z","created_by":"cscheid","updated_at":"2026-05-12T18:39:15.853892Z","source_repo":".","compaction_level":0,"original_size":0,"dependencies":[{"issue_id":"bd-cpzp","depends_on_id":"bd-emr4","type":"related","created_at":"2026-05-12T18:39:15.853594Z","created_by":"cscheid","metadata":"{}","thread_id":""}]}
157158
{"id":"bd-creo","title":"quarto render: fail strictly on Pass-1 failures (CI-friendly contract)","description":"Sibling of bd-rqba. Once Pass-1 failures are wired through as a dedicated pass1_failures field on the render summary surfaces (D1 in plan), give 'quarto render' a strict policy: any pass1_failures entry causes a non-zero exit. Remove the current string-matching of 'profile-pass skipped …' warning text in favor of the structured field.\n\nRationale: 'quarto render' is often used in headless CI; partial-progress leniency belongs to 'quarto preview' / hub-client, not render. The engine stays policy-free; consumers choose strict (render) vs lenient (preview).\n\nAlso: document the strict-vs-lenient contract in claude-notes/designs/document-profile-contract.md so future consumers (e.g., the planned hub-client-based 'quarto preview' binary) inherit it.\n\nPlan: claude-notes/plans/2026-05-01-hub-client-website-render-ux.md (Decision D1).","status":"open","priority":1,"issue_type":"task","created_at":"2026-05-01T14:16:43.115104Z","created_by":"cscheid","updated_at":"2026-05-01T14:16:43.115104Z","source_repo":".","compaction_level":0,"original_size":0,"dependencies":[{"issue_id":"bd-creo","depends_on_id":"bd-lk66","type":"parent-child","created_at":"2026-05-01T14:16:43.115104Z","created_by":"cscheid","metadata":"{}","thread_id":""},{"issue_id":"bd-creo","depends_on_id":"bd-rqba","type":"related","created_at":"2026-05-01T14:16:43.115104Z","created_by":"cscheid","metadata":"{}","thread_id":""}]}

0 commit comments

Comments
 (0)