diff --git a/claude-notes/plans/2026-06-19-resource-copy-error-diagnostic.md b/claude-notes/plans/2026-06-19-resource-copy-error-diagnostic.md new file mode 100644 index 000000000..7110224e8 --- /dev/null +++ b/claude-notes/plans/2026-06-19-resource-copy-error-diagnostic.md @@ -0,0 +1,248 @@ +# Resource-copy failure → structured diagnostic + +**Strand:** bd-bxrkxblx +**Date:** 2026-06-19 +**Status:** assessment / awaiting go-ahead (DO NOT execute yet) + +## Overview + +`cargo run --bin q2 -- render` (against `docs/`) currently emits: + +``` +error: /…/docs/guides/authoring/figures.qmd: failed to copy /…/docs/guides/authoring/images/html-figure.png -> /…/docs/_site/guides/authoring/images/html-figure.png: No such file or directory (os error 2) +``` + +This is a **plain, unstructured error string**. It has no error code, no +explanation of what the user did wrong, no actionable hint, and — most +importantly — no source span pointing at the broken reference. It is exactly +the class of message `quarto-error-reporting` exists to replace. + +The goal of this work: turn this (and its sibling I/O failures) into a +tidyverse-shaped `DiagnosticMessage` with a `Q-5-x` error code, a catalog +entry, an explanation, and ideally a span underlining the offending +`![](images/html-figure.png)` reference in `figures.qmd`. + +## Root cause / mechanism + +The error has two distinct layers, and we need to fix the **reporting** layer +(the underlying trigger — a missing image — is legitimately an error; we just +report it badly). + +### Underlying trigger +`docs/guides/authoring/figures.qmd:19` references `images/html-figure.png`, but +`docs/guides/authoring/images/` **does not exist**. The resource collector +enqueues a copy of the (non-existent) source; the copy fails at flush with +`ENOENT`. + +### Reporting path (the actual bug to fix) +1. `ResourceCollectorTransform::collect_resource` + (`crates/quarto-core/src/transforms/resource_collector.rs:398-424`) builds + `src = input_dir.join(url)` / `dest = output_dir.join(url)` and pushes the + pair into `self.copies`. **No source span is carried** — the copy intent is + a bare `(PathBuf, PathBuf)` (`ctx.resource_copies: Vec<(PathBuf, PathBuf)>`, + `render.rs:303`). The `Inline::Image` it came from *does* have a + `SourceInfo` in scope at the call site, but it is discarded. +2. The intents are drained and flushed: + - native single-doc: `render_to_file.rs:393-403` + - WASM/preview + project pass-2: `pass2_renderer.rs:667-683` + (`flush_resource_copies`) +3. `OutputSink::flush` runs the copy and, on I/O failure, returns + `OutputSinkError::Copy { src, dest, source }` + (`crates/quarto-core/src/output_sink.rs:116-121`). Its `Display` + (`output_sink.rs:157-164`) is the `failed to copy … -> …: …` string. +4. `From for QuartoError` + (`output_sink.rs:170-174`) flattens it to + `QuartoError::other(value.to_string())` — **all structure is lost here**. +5. The render command attaches this opaque error to the failing document as a + `pass2_failure` whose `diagnostics` vec is **empty**, so it falls into the + **legacy** branch in `crates/quarto/src/commands/render.rs:910-912`: + `eprintln!("error: {}: {}", failure.input.display(), failure.error)`. + Failures *with* structured `diagnostics` already go through the pretty + coalescer (`render.rs:907-909`); ours never gets there. + +### Why this is the right framing +There is already a **resource subsystem** in the catalog (`Q-5-1..5`, +`subsystem: "project"`) and a working precedent for exactly this conversion: +`resource_error_to_parse_error` in +`crates/quarto-core/src/project_resources.rs:645-705` turns a `ResourceError` +into a `DiagnosticMessage` with `.with_code("Q-5-x")`, `.with_location(span)`, +`.problem(...)`, `.add_info(...)`, degrading gracefully to a span-less message +when no FileId resolves. We follow that pattern. + +`Q-5-1..5` are taken; the next free project-subsystem code is **`Q-5-6`** (and +`Q-5-7` if we split error classes — see Decision 2). + +## Decisions — SETTLED (2026-06-19) + +The user reviewed the three decisions. Resolution below; the original options +are kept in git history. + +### Settled design: split the two failure modes, two codes, span-aware + +**Two distinct failure classes, reported differently:** + +1. **Referenced resource missing** — `Q-5-6`, **warning**, span-aware. The + common, user-fixable case (a `![](images/x.png)` whose source doesn't + exist). Detected **before** any copy is attempted; the build **continues** + (matches Q1's tolerant behavior). Diagnostic underlines the offending + reference in the source `.qmd`. +2. **Resource copy/write failed for an environment reason** — `Q-5-7`, hard + **error**. Permission-denied, disk-full (`ENOSPC`), etc., surfaced at + `OutputSink::flush`. The render fails. The specific OS condition is carried + as an **advisory detail** inside the same diagnostic — we pattern-match + `io::ErrorKind` (`PermissionDenied`, `StorageFull`, …) to tailor the hint + where the OS gives a usable kind, falling back to the raw message + otherwise. + +### Where the existence check lives — drain site, not the transform + +The user's intent was "detect at collection time." In our architecture the +`ResourceCollectorTransform` is a **pure AST→AST transform with no `SystemRuntime`** +(`RenderContext` carries no runtime handle, `render.rs:182`). Putting the +`is_file` stat *inside* the transform would force either plumbing a runtime +into a currently-I/O-free transform or reaching for bare `std::fs` (smell in +the `?Send` pipeline shared with WASM). + +Instead the existence check lives at the **drain/flush sites** +(`render_to_file.rs:393`, `pass2_renderer::flush_resource_copies:667`), which +already hold a `runtime: &dyn SystemRuntime` exposing +`is_file` / `path_exists` (trait method, native + WASM). This is **functionally +identical** to "collection time" from the user's perspective — the check +happens *before any copy is attempted* — but sites the I/O where a runtime +already exists. It is therefore the originally-described "option C" semantics at +**~the same cost as option B** (the only real cost is threading the span, which +B needs anyway; the stat itself is one `runtime.is_file(src)` call). My earlier +"C is the most work" assessment wrongly assumed the stat had to live inside the +transform. + +WASM gets correct behavior for free: in `vfs_root` mode the collector emits +**zero** copy intents (`resource_collector.rs:99`), so the drain list is empty +and both the existence check and the flush no-op. + +### Drain-site algorithm + +For each `(src, dest, origin_span)` intent (span threaded per below): + +```text +if !runtime.is_file(src): + emit Q-5-6 WARNING with origin_span; skip this copy; continue +else: + sink.copy(src, dest); enqueue +# after the loop: +sink.flush(runtime): + on OutputSinkError::Copy/Write/... -> emit Q-5-7 ERROR, + advisory tailored from io::ErrorKind +``` + +### Where the conversion lives +A dedicated converter near the drain sites (mirroring +`resource_error_to_parse_error`), **not** the lossy `From for +QuartoError`. It takes the failure (+ the intent's `origin` span) and produces +`DiagnosticMessage` + `SourceContext`. Both drain sites call it. + +## Test plan (TDD — write first) + +1. **Catalog presence test** (in `catalog.rs` tests, mirroring the + `error_catalog_has_q_13_navigation_codes` style): assert `Q-5-6` (and + `Q-5-7` if we split) exist, are in the `project` subsystem, mention + "resource"/"copy", and have a `docs_url` ending in the code. +2. **Conversion unit test** (quarto-core): given an `OutputSinkError::Copy` + with an ENOENT source + an origin `SourceInfo`, the converter yields a + `DiagnosticMessage` with the right code, a problem mentioning the missing + path, and a resolved location. A second test: span-less degradation when no + origin is available. +3. **End-to-end render test** (route through the real render path, per the + end-to-end-verification rule): render a fixture doc referencing a missing + image and assert the structured diagnostic (code + pretty form) appears + instead of the legacy `error: …: failed to copy …` line. Use a fixture under + the crate's `tests/` tree, not `docs/`. +4. **Manual E2E**: `cargo run --bin q2 -- render ` and paste the + before/after terminal output into this plan. + +## Work items + +- [x] Settle Decisions 1–3 with the user. (settled 2026-06-19, above) +- [x] Add catalog entries `Q-5-6` (referenced resource not found, warning) and + `Q-5-7` (resource copy/write failed, error) to + `crates/quarto-error-reporting/error_catalog.json`; write the catalog + presence test (fails first). *(2026-06-19: test + `error_catalog_has_q_5_6_and_q_5_7` in `catalog.rs` — verified red then + green. Both entries in `project` subsystem.)* +- [x] Enrich the resource-copy intent to carry the originating `SourceInfo`: + new `ResourceCopyIntent { src, dest, origin: SourceInfo }` in `render.rs` + replaces the `(PathBuf, PathBuf)` tuple. `origin` populated in + `resource_collector.rs` from `img.target_source.url` (falling back to + `img.source_info`). Threaded through `render.rs` ctx, + `stage/context.rs`, `render_to_file.rs`, and + `pass2_renderer::flush_resource_copies`. *(2026-06-19, compiles.)* +- [x] Add the dedicated converter + (`crates/quarto-core/src/resource_copy_diagnostics.rs`), modeled on + `resource_error_to_parse_error`. Three unit tests (all pass): (a) missing + source + span → `Q-5-6` warning, located; (b) `OutputSinkError::Copy` + with `PermissionDenied` → `Q-5-7` error with the permission advisory; (c) + disk-full (`Other` kind) surfaces the raw OS message. Plus shared helpers + `enqueue_resource_copies` (existence-check + enqueue) and + `copy_failure_error` (wrap as `QuartoError::Parse`). +- [x] Implement the drain-site algorithm at **both** sites + (`render_to_file.rs`, `pass2_renderer::flush_resource_copies`): + `runtime.is_file(src)` pre-check → `Q-5-6` warning merged into + `render_output.diagnostics`; on flush, only `OutputSinkError::Copy` + routes to `Q-5-7` (other variants — HTML write, dir create — keep the + existing path). `Q-5-7` reaches the pretty coalescer via + `file_failure_from_error` lifting `QuartoError::Parse` diagnostics. +- [x] Confirm warning semantics: `Q-5-6` is pushed as a *warning* into the + render output's diagnostics and the copy is skipped — the render + proceeds. Only the `Q-5-7` `Copy` flush fault returns `Err` and aborts. + *(Still to verify end-to-end that exit code is 0 with only Q-5-6.)* +- [x] End-to-end render test + manual verification. *(2026-06-19)* + - Regression tests added to + `tests/integration/render_preserves_source_files.rs` (single-doc + + website config), both green. + - **Manual single-doc:** `q2 render /doc.qmd` with a missing + `images/nope.png` → printed `Warning: [Q-5-6] Referenced resource not + found`, a pretty ariadne diagnostic underlining exactly `images/nope.png` + in the reference, the problem prose, the hint, **exit code 0**, and the + HTML was still produced. Output inspected. + - **Manual project (the original repro):** `q2 render docs/` → the former + `error: …/figures.qmd: failed to copy …/images/html-figure.png … (os + error 2)` is now a `Q-5-6` warning; `Rendered 166 of 166 files … — 1 + error, 27 warnings`. The remaining "1 error" is a *pre-existing, + unrelated* `duplicate crossref identifier fig-charts`, not our copy + failure. No `Q-5-7` fired (no permission/disk faults), as expected. + - **Q-5-7** (permission/disk) is covered by the converter unit tests; + not separately driven through the binary (reliably forcing a + permission/ENOSPC copy fault is platform-specific — see the + cross-platform rule). The native render-to-file routing + (`OutputSinkError::Copy → copy_failure_error`) is simple and shared + with the unit-tested helper. *Honest status: Q-5-7's end-to-end binary + path was not exercised; its diagnostic shape and the routing helper + are unit-tested.* + - The WASM/preview drain (`pass2_renderer::flush_resource_copies`) + mirrors the native drain and shares `enqueue_resource_copies`; not + separately E2E'd on native (it's the WASM-only `WasmPassTwoOutput` + path). +- [x] `cargo nextest run --workspace` (10,284 passed) and full `cargo xtask + verify` (Rust + WASM + hub-client builds and tests) — both green + (2026-06-19). One clippy `io_other_error` lint in a test was fixed + (`std::io::Error::other`). +- [x] Authored matching docs pages `docs/errors/project/Q-5-6.qmd` and + `Q-5-7.qmd`, modeled on the sibling Q-5 pages (front-matter schema, + What/Why/How + Related sections, `status: stub`). Both render cleanly via + `q2 render docs/` (auto-discovered by the `errors/index.qmd` listing; no + manual index edit needed). No `cargo xtask error-docs` tool exists yet + and there is no automated catalog↔docs parity test to satisfy. + +## Key references +- `crates/quarto-core/src/output_sink.rs:116-174` — `OutputSinkError` + the + lossy `From … for QuartoError`. +- `crates/quarto-core/src/transforms/resource_collector.rs:398-424` — where the + copy intent (and the discarded span) originate. +- `crates/quarto-core/src/render.rs:303` — `resource_copies` ctx field type. +- `crates/quarto-core/src/render_to_file.rs:393-403`, + `crates/quarto-core/src/project/pass2_renderer.rs:667-683` — flush sites. +- `crates/quarto-core/src/project_resources.rs:645-705` — + `resource_error_to_parse_error`, the precedent to mirror. +- `crates/quarto/src/commands/render.rs:887-916` — legacy vs. structured + reporting split (the branch we want to land in). +- `crates/quarto-error-reporting/src/builder.rs` — `DiagnosticMessageBuilder` + (`with_code`, `with_location`, `problem`, `add_info`, `add_hint`). diff --git a/crates/quarto-core/src/lib.rs b/crates/quarto-core/src/lib.rs index f414c82ae..c6031cad0 100644 --- a/crates/quarto-core/src/lib.rs +++ b/crates/quarto-core/src/lib.rs @@ -55,6 +55,7 @@ pub mod project_resources; pub mod render; #[cfg(not(target_arch = "wasm32"))] pub mod render_to_file; +pub mod resource_copy_diagnostics; pub mod resource_resolver; pub mod resources; pub mod revealjs; diff --git a/crates/quarto-core/src/project/pass2_renderer.rs b/crates/quarto-core/src/project/pass2_renderer.rs index 551652bef..51b8a1de9 100644 --- a/crates/quarto-core/src/project/pass2_renderer.rs +++ b/crates/quarto-core/src/project/pass2_renderer.rs @@ -664,22 +664,30 @@ impl WasmPassTwoOutput { /// branch, so user-resource copies are committed alongside the /// rest of the render's destructive output, governed by the sink's /// `allowed_roots` and `src == dest` checks (bd-cfl67). +/// Returns the `Q-5-6` warnings for any referenced resources whose +/// source was missing (skipped, not copied); the caller merges these +/// into the page's diagnostics. A genuine copy fault surfaces as the +/// `Q-5-7` error (bd-bxrkxblx). fn flush_resource_copies( - copies: Vec<(std::path::PathBuf, std::path::PathBuf)>, + copies: Vec, resolver: &ResourceResolverContext, runtime: &dyn quarto_system_runtime::SystemRuntime, -) -> Result<()> { +) -> Result> { if copies.is_empty() { - return Ok(()); + return Ok(Vec::new()); } let mut sink = crate::output_sink::OutputSink::new(resolver.allowed_output_roots()); - for (src, dest) in copies { - sink.copy(src, dest) - .map_err(crate::error::QuartoError::from)?; - } - sink.flush(runtime) - .map_err(crate::error::QuartoError::from)?; - Ok(()) + let warnings = + crate::resource_copy_diagnostics::enqueue_resource_copies(copies, &mut sink, runtime)?; + // This sink holds only resource copies, so any flush failure is + // unambiguously a resource-copy fault → Q-5-7. + sink.flush(runtime).map_err(|e| match &e { + crate::output_sink::OutputSinkError::Copy { .. } => { + crate::resource_copy_diagnostics::copy_failure_error(&e) + } + _ => crate::error::QuartoError::from(e), + })?; + Ok(warnings) } /// In-memory Pass-2 renderer used by the WASM hub-client live @@ -814,7 +822,7 @@ impl Pass2Renderer for RenderToHtmlRenderer { let config = HtmlRenderConfig::with_resolver(resolver.clone()); let source_name = doc_info.input.to_string_lossy().to_string(); - let render_output = render_qmd_to_html( + let mut render_output = render_qmd_to_html( &input_bytes, &source_name, &mut ctx, @@ -873,11 +881,12 @@ impl Pass2Renderer for RenderToHtmlRenderer { // rendered HTML's `` URL points to (in WASM // hub-client mode that's `{vfs_root}/`; in a native // website that's `{output_dir}//`). - flush_resource_copies( + let copy_warnings = flush_resource_copies( std::mem::take(&mut ctx.resource_copies), &resolver, runtime.as_ref(), )?; + render_output.diagnostics.extend(copy_warnings); Ok(WasmPassTwoOutput { source_path: doc_info.input.clone(), @@ -1074,7 +1083,7 @@ impl Pass2Renderer for RenderToPreviewAstRenderer { let source_name = doc_info.input.to_string_lossy().to_string(); - let preview_output = render_qmd_to_preview_ast( + let mut preview_output = render_qmd_to_preview_ast( &input_bytes, &source_name, &mut ctx, @@ -1154,11 +1163,12 @@ impl Pass2Renderer for RenderToPreviewAstRenderer { } // bd-cfl67: see the matching comment in the HTML renderer. - flush_resource_copies( + let copy_warnings = flush_resource_copies( std::mem::take(&mut ctx.resource_copies), &resolver, runtime.as_ref(), )?; + preview_output.diagnostics.extend(copy_warnings); Ok(WasmPassTwoOutput { source_path: doc_info.input.clone(), diff --git a/crates/quarto-core/src/render.rs b/crates/quarto-core/src/render.rs index 73d969bc0..57877c0d6 100644 --- a/crates/quarto-core/src/render.rs +++ b/crates/quarto-core/src/render.rs @@ -168,6 +168,31 @@ impl BinaryDependencies { } } +/// A pending user-resource copy intent collected from the document AST. +/// +/// Producers (e.g. [`crate::transforms::ResourceCollectorTransform`]) +/// emit one of these per referenced on-disk resource (an image, etc.) +/// instead of copying directly. The drain sites +/// (`render_document_to_file`, `pass2_renderer::flush_resource_copies`) +/// validate existence and route the copy through the per-render +/// [`crate::output_sink::OutputSink`]. +/// +/// `origin` carries the source location of the reference that produced +/// the intent (the image's URL span, falling back to the whole-image +/// span). It lets the drain site attach a precise diagnostic — e.g. +/// `Q-5-6` underlining the offending `![](…)` — when the source file +/// is missing or the copy fails. See bd-bxrkxblx. +#[derive(Debug, Clone)] +pub struct ResourceCopyIntent { + /// Absolute source path the resource is read from. + pub src: PathBuf, + /// Absolute destination path in the output tree. + pub dest: PathBuf, + /// Source location of the document reference that requested the + /// copy, used for diagnostics when the copy can't be satisfied. + pub origin: quarto_source_map::SourceInfo, +} + /// Context for a single document render operation. /// /// This is the mutable state passed through all pipeline stages. @@ -287,11 +312,11 @@ pub struct RenderContext<'a> { /// AST that must be copied from the source tree to the output /// tree before the render is committed. /// - /// Each entry is `(src_absolute, dest_absolute)`. The destination - /// is computed by the producing transform from - /// [`ResourceResolverContext::page_dir`] joined with the URL the - /// document references — so the copied file lands at the same - /// relative location the rendered HTML expects. + /// Each entry is a [`ResourceCopyIntent`] (`src`, `dest`, + /// `origin` span). The destination is computed by the producing + /// transform from [`ResourceResolverContext::page_dir`] joined + /// with the URL the document references — so the copied file lands + /// at the same relative location the rendered HTML expects. /// /// Drained into the per-render [`crate::output_sink::OutputSink`] /// by `render_document_to_file` after the pipeline finishes. @@ -300,7 +325,7 @@ pub struct RenderContext<'a> { /// this field rather than calling `runtime.file_copy` directly, /// so every destructive disk write in the render flows through /// the one validated sink — see bd-cfl67. - pub resource_copies: Vec<(PathBuf, PathBuf)>, + pub resource_copies: Vec, /// Resolved listings produced by `ListingGenerateTransform` and /// consumed by `ListingRenderTransform`. Populated only when the diff --git a/crates/quarto-core/src/render_to_file.rs b/crates/quarto-core/src/render_to_file.rs index 5ab8e572e..e0fcaa796 100644 --- a/crates/quarto-core/src/render_to_file.rs +++ b/crates/quarto-core/src/render_to_file.rs @@ -70,7 +70,7 @@ use crate::Result; use crate::artifact::{ArtifactScope, ArtifactStore}; use crate::error::QuartoError; use crate::format::Format; -use crate::output_sink::OutputSink; +use crate::output_sink::{OutputSink, OutputSinkError}; use crate::pipeline::{HtmlRenderConfig, RenderOutput, render_qmd_to_html}; use crate::project::index::ProjectIndex; use crate::project::orchestrator::project_type_for; @@ -334,7 +334,7 @@ pub fn render_document_to_file( } // Run the render pipeline - let render_output = pollster::block_on(render_qmd_to_html( + let mut render_output = pollster::block_on(render_qmd_to_html( &input_bytes, &input_path.to_string_lossy(), &mut ctx, @@ -390,17 +390,33 @@ pub fn render_document_to_file( // skip ops whose src and dest canonicalize equal — the common // single-doc case where output_dir == input_dir and the // resource is already where the HTML expects it. + // + // bd-bxrkxblx: a referenced resource whose *source* is missing is + // a user-content problem — emit a `Q-5-6` warning (located at the + // reference) and skip it, rather than aborting the render on the + // ENOENT at flush. Genuine flush-time copy faults (permission, + // disk full) become the `Q-5-7` error below. let resource_copies = std::mem::take(&mut ctx.resource_copies); - for (src, dest) in resource_copies { - sink.copy(src, dest).map_err(QuartoError::from)?; - } + let copy_warnings = crate::resource_copy_diagnostics::enqueue_resource_copies( + resource_copies, + &mut sink, + runtime.as_ref(), + )?; + render_output.diagnostics.extend(copy_warnings); // Output HTML also goes through the sink so the whole render's // destructive output is validated and committed atomically. sink.write(output_path.clone(), render_output.html.as_bytes().to_vec()) .map_err(QuartoError::from)?; - sink.flush(runtime.as_ref()).map_err(QuartoError::from)?; + // Distinguish a resource-copy fault (the bytes the user referenced + // couldn't be placed — Q-5-7) from any other destructive write in + // the sink (HTML output, artifact dir creation), which keep their + // existing error path. + sink.flush(runtime.as_ref()).map_err(|e| match &e { + OutputSinkError::Copy { .. } => crate::resource_copy_diagnostics::copy_failure_error(&e), + _ => QuartoError::from(e), + })?; debug!("Output: {}", output_path.display()); diff --git a/crates/quarto-core/src/resource_copy_diagnostics.rs b/crates/quarto-core/src/resource_copy_diagnostics.rs new file mode 100644 index 000000000..9fbe792ad --- /dev/null +++ b/crates/quarto-core/src/resource_copy_diagnostics.rs @@ -0,0 +1,236 @@ +//! Structured diagnostics for user-resource copy failures (bd-bxrkxblx). +//! +//! A document can reference an on-disk resource (an image, etc.) that +//! Quarto must copy into the rendered output. Two things can go wrong, +//! and they deserve different treatment: +//! +//! - **The referenced source file is missing** — a user-content problem +//! the author can fix. Detected at the drain site *before* the copy is +//! attempted (`runtime.is_file(src)` is false). Reported as the +//! **`Q-5-6`** *warning*, located at the reference so the renderer can +//! underline the offending `![](…)`. The render continues without the +//! file (matching Quarto 1's tolerant behavior). +//! - **The copy/write fails at [`OutputSink::flush`](crate::output_sink) +//! for an environment reason** — permission denied, disk full, etc. +//! Reported as the **`Q-5-7`** *error*. Span-less: the fault is the +//! filesystem, not any particular reference. The specific OS condition +//! is surfaced as advisory detail. +//! +//! Both functions mirror the precedent in +//! [`crate::project_resources::resource_error_to_parse_error`]. + +use quarto_error_reporting::{DiagnosticMessage, DiagnosticMessageBuilder}; +use quarto_system_runtime::SystemRuntime; + +use crate::error::{ParseError, QuartoError}; +use crate::output_sink::{OutputSink, OutputSinkError}; +use crate::render::ResourceCopyIntent; + +/// Build the `Q-5-6` warning for a referenced resource whose source +/// file does not exist. +/// +/// The diagnostic is located at the reference's `origin` span so the +/// renderer can underline the offending reference in the source `.qmd`. +/// The caller is responsible for rendering it against a source context +/// that contains the referencing file (the per-render +/// `RenderOutput::source_context` already does). +pub fn missing_resource_diagnostic(intent: &ResourceCopyIntent) -> DiagnosticMessage { + DiagnosticMessageBuilder::warning("Referenced resource not found") + .with_code("Q-5-6") + .with_location(intent.origin.clone()) + .problem(format!( + "The referenced file `{}` does not exist, so it was not copied \ + into the output. The render continued without it.", + intent.src.display() + )) + .add_hint("Check the path in the reference, or add the missing file.") + .build() +} + +/// Build the `Q-5-7` error for a resource copy/write that failed at +/// flush. +/// +/// Span-less: a flush failure is an environment condition (permission +/// denied, disk full), not a problem with any particular reference. The +/// underlying OS condition is surfaced as advisory detail where +/// `io::ErrorKind` is informative, falling back to the raw OS message. +pub fn copy_failure_diagnostic(err: &OutputSinkError) -> DiagnosticMessage { + let (problem, advisory) = describe(err); + let mut builder = DiagnosticMessageBuilder::error("Resource copy failed") + .with_code("Q-5-7") + .problem(problem); + if let Some(advisory) = advisory { + builder = builder.add_info(advisory); + } + builder.build() +} + +/// Map an [`OutputSinkError`] to a problem statement and an optional +/// advisory detail. I/O-bearing variants get a path-aware problem plus +/// an `io::ErrorKind`-tailored advisory; the remaining (validation / +/// canonicalize) variants fall back to the error's own `Display`. +fn describe(err: &OutputSinkError) -> (String, Option) { + match err { + OutputSinkError::Copy { src, dest, source } => ( + format!( + "Could not copy `{}` to `{}`.", + src.display(), + dest.display() + ), + advisory_for_io(source), + ), + OutputSinkError::Write { dest, source } => ( + format!("Could not write `{}`.", dest.display()), + advisory_for_io(source), + ), + OutputSinkError::CreateParent { parent, source } => ( + format!( + "Could not create the output directory `{}`.", + parent.display() + ), + advisory_for_io(source), + ), + OutputSinkError::Canonicalize { path, source } => ( + format!("Could not resolve the output path `{}`.", path.display()), + advisory_for_io(source), + ), + // Enqueue-time contract violations (a producer fed a bad + // destination). These indicate an internal bug rather than an + // environment fault; surface the sink's own message verbatim. + other @ (OutputSinkError::DestOutsideAllowedRoots { .. } + | OutputSinkError::DestNotAbsolute { .. }) => (other.to_string(), None), + } +} + +/// Enqueue each copy intent whose source file exists into `sink`, +/// collecting a `Q-5-6` warning (and skipping the copy) for each intent +/// whose source is missing. +/// +/// This is the shared body of both drain sites +/// (`render_document_to_file` and `pass2_renderer::flush_resource_copies`): +/// it implements the "detect missing source before attempting the copy" +/// policy at the one place that holds a [`SystemRuntime`]. Returns the +/// collected warnings for the caller to merge into the render's +/// diagnostics. Enqueue-time validation failures (a producer fed a +/// destination outside the allowed roots) propagate as `QuartoError`. +/// +/// If the existence check itself errors, the source is *assumed present* +/// and the copy is enqueued — a genuine fault then surfaces as the +/// `Q-5-7` error at flush, rather than being masked as a spurious +/// "missing resource" warning. +pub fn enqueue_resource_copies( + intents: Vec, + sink: &mut OutputSink, + runtime: &dyn SystemRuntime, +) -> Result, QuartoError> { + let mut warnings = Vec::new(); + for intent in intents { + let exists = runtime.is_file(&intent.src).unwrap_or(true); + if !exists { + warnings.push(missing_resource_diagnostic(&intent)); + continue; + } + sink.copy(intent.src, intent.dest) + .map_err(QuartoError::from)?; + } + Ok(warnings) +} + +/// Map an [`OutputSinkError`] from a resource-copy flush into a +/// structured [`QuartoError::Parse`] carrying the `Q-5-7` diagnostic, so +/// it reaches the pretty reporting path (via `file_failure_from_error`) +/// rather than the legacy single-line fallback. Span-less, so an empty +/// source context suffices. +pub fn copy_failure_error(err: &OutputSinkError) -> QuartoError { + QuartoError::Parse(ParseError::new( + vec![copy_failure_diagnostic(err)], + quarto_source_map::SourceContext::new(), + )) +} + +/// Tailor an advisory line from the underlying I/O error. `PermissionDenied` +/// gets a writable-directory nudge; everything else carries the raw OS +/// message so distinctive conditions (e.g. "No space left on device") +/// still reach the user. +fn advisory_for_io(source: &std::io::Error) -> Option { + match source.kind() { + std::io::ErrorKind::PermissionDenied => Some( + "The filesystem denied permission for this write. Check that the \ + output directory is writable." + .to_string(), + ), + _ => Some(format!("The filesystem reported: {source}")), + } +} + +#[cfg(test)] +mod tests { + use super::*; + use quarto_error_reporting::DiagnosticKind; + use quarto_source_map::{FileId, SourceInfo}; + use std::path::PathBuf; + + fn intent(src: &str) -> ResourceCopyIntent { + ResourceCopyIntent { + src: PathBuf::from(src), + dest: PathBuf::from("/out/img.png"), + origin: SourceInfo::original(FileId(0), 10, 30), + } + } + + #[test] + fn missing_resource_is_q_5_6_warning_with_location() { + let diag = missing_resource_diagnostic(&intent("/project/images/missing.png")); + assert_eq!(diag.code.as_deref(), Some("Q-5-6")); + assert_eq!(diag.kind, DiagnosticKind::Warning); + assert!( + diag.location.is_some(), + "Q-5-6 must carry the reference's source location" + ); + let text = diag.to_text(None); + assert!( + text.contains("missing.png"), + "problem must name the missing source; got: {text}" + ); + } + + #[test] + fn copy_failure_permission_denied_is_q_5_7_error_with_advisory() { + let err = OutputSinkError::Copy { + src: PathBuf::from("/project/a.png"), + dest: PathBuf::from("/out/a.png"), + source: std::io::Error::from(std::io::ErrorKind::PermissionDenied), + }; + let diag = copy_failure_diagnostic(&err); + assert_eq!(diag.code.as_deref(), Some("Q-5-7")); + assert_eq!(diag.kind, DiagnosticKind::Error); + assert!( + diag.location.is_none(), + "Q-5-7 is span-less — the fault is the filesystem, not a reference" + ); + let text = diag.to_text(None); + assert!( + text.to_lowercase().contains("permission"), + "permission-denied advisory expected; got: {text}" + ); + } + + #[test] + fn copy_failure_disk_full_surfaces_raw_os_message() { + // ENOSPC: on stable Rust this may not map to a dedicated + // ErrorKind, so the advisory must fall through to the raw OS + // message rather than swallowing it. + let err = OutputSinkError::Copy { + src: PathBuf::from("/project/big.bin"), + dest: PathBuf::from("/out/big.bin"), + source: std::io::Error::other("No space left on device (os error 28)"), + }; + let diag = copy_failure_diagnostic(&err); + assert_eq!(diag.code.as_deref(), Some("Q-5-7")); + let text = diag.to_text(None); + assert!( + text.contains("No space left on device"), + "disk-full message must reach the user; got: {text}" + ); + } +} diff --git a/crates/quarto-core/src/stage/context.rs b/crates/quarto-core/src/stage/context.rs index 42fb7012c..b291b10f2 100644 --- a/crates/quarto-core/src/stage/context.rs +++ b/crates/quarto-core/src/stage/context.rs @@ -114,7 +114,7 @@ pub struct StageContext { /// holds only engine + filter contributions. pub resource_report: crate::project_resources::DocumentResourceReport, - /// User-resource copy intents (`(src_absolute, dest_absolute)`) + /// User-resource copy intents ([`crate::render::ResourceCopyIntent`]) /// collected by AST transforms — currently /// [`crate::transforms::ResourceCollectorTransform`]. Bridged /// to/from the inner `RenderContext::resource_copies` by @@ -125,7 +125,7 @@ pub struct StageContext { /// /// See bd-cfl67 for why this lives parallel to `artifacts` /// rather than inside it. - pub resource_copies: Vec<(std::path::PathBuf, std::path::PathBuf)>, + pub resource_copies: Vec, // === Observation & Control === /// Observer for tracing, progress reporting, and WASM callbacks diff --git a/crates/quarto-core/src/transforms/resource_collector.rs b/crates/quarto-core/src/transforms/resource_collector.rs index 445a7320e..6d790f0ee 100644 --- a/crates/quarto-core/src/transforms/resource_collector.rs +++ b/crates/quarto-core/src/transforms/resource_collector.rs @@ -31,7 +31,7 @@ //! paths, and this producer no longer touches the artifact store //! at all. -use std::path::{Path, PathBuf}; +use std::path::Path; use quarto_pandoc_types::Slot; use quarto_pandoc_types::block::Block; @@ -124,10 +124,10 @@ impl AstTransform for ResourceCollectorTransform { struct ResourceVisitor<'a> { input_dir: &'a Path, output_dir: &'a Path, - /// `(source_absolute, destination_absolute)` pairs in - /// discovery order. The producer dedupes by URL — see - /// [`Self::collect_resource`]. - copies: Vec<(PathBuf, PathBuf)>, + /// Copy intents in discovery order, each carrying the source span + /// of the reference that produced it. The producer dedupes by URL + /// — see [`Self::collect_resource`]. + copies: Vec, /// Set of URLs already added, for dedup within a single page. seen_urls: std::collections::HashSet, } @@ -274,8 +274,15 @@ impl<'a> ResourceVisitor<'a> { fn visit_inline(&mut self, inline: &Inline) { match inline { Inline::Image(img) => { - // Collect image resource - target is (url, title) tuple - self.collect_resource(&img.target.0); + // Collect image resource - target is (url, title) tuple. + // Prefer the URL's own span (underlines just the path); + // fall back to the whole-image span when absent. + let origin = img + .target_source + .url + .clone() + .unwrap_or_else(|| img.source_info.clone()); + self.collect_resource(&img.target.0, origin); } Inline::Link(link) => { // Visit link content @@ -395,7 +402,7 @@ impl<'a> ResourceVisitor<'a> { } } - fn collect_resource(&mut self, url: &str) { + fn collect_resource(&mut self, url: &str, origin: quarto_source_map::SourceInfo) { // Skip external URLs and inlined data URIs — nothing on // disk to copy. if url.starts_with("http://") @@ -420,7 +427,8 @@ impl<'a> ResourceVisitor<'a> { let src = self.input_dir.join(url); let dest = self.output_dir.join(url); - self.copies.push((src, dest)); + self.copies + .push(crate::render::ResourceCopyIntent { src, dest, origin }); } } @@ -449,13 +457,15 @@ pub fn collect_referenced_asset_urls(blocks: &[Block]) -> Vec { visitor .copies .into_iter() - .map(|(src, _dest)| src.to_string_lossy().into_owned()) + .map(|intent| intent.src.to_string_lossy().into_owned()) .collect() } #[cfg(test)] mod tests { use super::*; + use std::path::PathBuf; + use quarto_pandoc_types::attr::{AttrSourceInfo, TargetSourceInfo}; use quarto_pandoc_types::block::Paragraph; use quarto_pandoc_types::inline::{Image, Inline, Str}; @@ -596,12 +606,14 @@ mod tests { assert_eq!(ctx.resource_copies.len(), 1); assert_eq!( - ctx.resource_copies[0], - ( - PathBuf::from("/project/images/photo.png"), - PathBuf::from("/project/_site/images/photo.png"), - ), - "src under input_dir, dest under page_dir, same relative position", + ctx.resource_copies[0].src, + PathBuf::from("/project/images/photo.png"), + "src under input_dir", + ); + assert_eq!( + ctx.resource_copies[0].dest, + PathBuf::from("/project/_site/images/photo.png"), + "dest under page_dir, same relative position", ); // The artifact store is untouched — the contract is that // user resources flow through `resource_copies`, not diff --git a/crates/quarto-core/tests/integration/render_preserves_source_files.rs b/crates/quarto-core/tests/integration/render_preserves_source_files.rs index 8c1c715d9..78445a913 100644 --- a/crates/quarto-core/tests/integration/render_preserves_source_files.rs +++ b/crates/quarto-core/tests/integration/render_preserves_source_files.rs @@ -140,3 +140,77 @@ fn website_render_copies_image_to_output_and_preserves_source() { html ); } + +/// Collect the `Q-X-Y` codes attached to a render result's diagnostics. +fn diagnostic_codes(result: &quarto_core::render_to_file::RenderToFileResult) -> Vec { + result + .render_output + .diagnostics + .iter() + .filter_map(|d| d.code.clone()) + .collect() +} + +/// bd-bxrkxblx (single-doc): referencing an image whose *source* is +/// missing must NOT abort the render. The render succeeds and emits a +/// `Q-5-6` warning rather than the legacy `failed to copy … (os error +/// 2)` hard error. +#[test] +fn render_with_missing_referenced_image_warns_q_5_6_and_continues() { + let temp = TempDir::new().unwrap(); + let project_dir = temp.path(); + + let qmd_path = project_dir.join("index.qmd"); + write_text( + &qmd_path, + "---\ntitle: Test\n---\n\n![Caption](missing.png)\n", + ); + // missing.png is intentionally never created. + + let options = RenderToFileOptions::default(); + let runtime = runtime_arc(); + let result = render_to_file(&qmd_path, "html", &options, runtime) + .expect("render must succeed despite the missing image"); + + let codes = diagnostic_codes(&result); + assert!( + codes.iter().any(|c| c == "Q-5-6"), + "expected a Q-5-6 warning for the missing image; got codes: {codes:?}" + ); +} + +/// bd-bxrkxblx (website): same invariant when the output dir is +/// distinct from the input dir (`_site/`). The render completes, the +/// missing image yields a `Q-5-6` warning, and nothing is written at +/// the would-be destination. +#[test] +fn website_render_with_missing_image_warns_q_5_6_and_continues() { + let temp = TempDir::new().unwrap(); + let project_dir = temp.path(); + + write_text( + &project_dir.join("_quarto.yml"), + "project:\n type: website\n", + ); + write_text( + &project_dir.join("index.qmd"), + "---\ntitle: Test\n---\n\n![Caption](missing.png)\n", + ); + // missing.png is intentionally never created. + + let options = RenderToFileOptions::default(); + let runtime = runtime_arc(); + let result = render_to_file(&project_dir.join("index.qmd"), "html", &options, runtime) + .expect("website render must succeed despite the missing image"); + + let codes = diagnostic_codes(&result); + assert!( + codes.iter().any(|c| c == "Q-5-6"), + "expected a Q-5-6 warning for the missing image; got codes: {codes:?}" + ); + + assert!( + !project_dir.join("_site/missing.png").exists(), + "no file should be written at the missing image's destination" + ); +} diff --git a/crates/quarto-error-reporting/error_catalog.json b/crates/quarto-error-reporting/error_catalog.json index fafc987ae..7574a4e5c 100644 --- a/crates/quarto-error-reporting/error_catalog.json +++ b/crates/quarto-error-reporting/error_catalog.json @@ -776,6 +776,20 @@ "docs_url": "https://quarto.org/docs/errors/project/Q-5-5", "since_version": "99.9.9" }, + "Q-5-6": { + "subsystem": "project", + "title": "Referenced Resource Not Found", + "message_template": "A document references a resource file (for example an image) whose source does not exist on disk, so it cannot be copied into the output. Check the path in the reference, or add the missing file. The render continues without it.", + "docs_url": "https://quarto.org/docs/errors/project/Q-5-6", + "since_version": "99.9.9" + }, + "Q-5-7": { + "subsystem": "project", + "title": "Resource Copy Failed", + "message_template": "Quarto could not copy a referenced resource into the output directory because of a filesystem error, such as a permission problem or insufficient disk space. The render cannot complete until the underlying filesystem condition is resolved.", + "docs_url": "https://quarto.org/docs/errors/project/Q-5-7", + "since_version": "99.9.9" + }, "Q-7-1": { "subsystem": "cli", "title": "Missing Newline at End of File", diff --git a/crates/quarto-error-reporting/src/catalog.rs b/crates/quarto-error-reporting/src/catalog.rs index 66a7f457f..622f19ab0 100644 --- a/crates/quarto-error-reporting/src/catalog.rs +++ b/crates/quarto-error-reporting/src/catalog.rs @@ -234,4 +234,54 @@ mod tests { q16.message_template ); } + + // bd-bxrkxblx: Q-5-6 / Q-5-7 resource-copy diagnostics. + // + // Q-5-6 is the span-aware *warning* for a referenced resource + // whose source file is missing (detected before the copy is + // attempted). Q-5-7 is the hard *error* for a copy/write that + // fails at flush for an environment reason (permission denied, + // disk full). See + // `claude-notes/plans/2026-06-19-resource-copy-error-diagnostic.md`. + #[test] + fn error_catalog_has_q_5_6_and_q_5_7() { + let q6 = get_error_info("Q-5-6").expect("Q-5-6 must be in the catalog"); + assert_eq!(q6.subsystem, "project"); + assert!( + q6.title.to_lowercase().contains("resource"), + "Q-5-6 title must mention `resource`; got: {}", + q6.title + ); + assert!( + q6.message_template.to_lowercase().contains("not exist") + || q6.message_template.to_lowercase().contains("missing") + || q6.message_template.to_lowercase().contains("not found"), + "Q-5-6 message must describe the missing source; got: {}", + q6.message_template + ); + assert!( + q6.docs_url.as_deref().is_some_and(|u| u.ends_with("Q-5-6")), + "Q-5-6 docs_url must end with the code; got: {:?}", + q6.docs_url + ); + + let q7 = get_error_info("Q-5-7").expect("Q-5-7 must be in the catalog"); + assert_eq!(q7.subsystem, "project"); + assert!( + q7.title.to_lowercase().contains("copy") + || q7.title.to_lowercase().contains("resource"), + "Q-5-7 title must mention copy/resource; got: {}", + q7.title + ); + assert!( + q7.message_template.to_lowercase().contains("copy"), + "Q-5-7 message must mention copying; got: {}", + q7.message_template + ); + assert!( + q7.docs_url.as_deref().is_some_and(|u| u.ends_with("Q-5-7")), + "Q-5-7 docs_url must end with the code; got: {:?}", + q7.docs_url + ); + } } diff --git a/docs/errors/project/Q-5-6.qmd b/docs/errors/project/Q-5-6.qmd new file mode 100644 index 000000000..dca731c4d --- /dev/null +++ b/docs/errors/project/Q-5-6.qmd @@ -0,0 +1,65 @@ +--- +title: "Referenced Resource Not Found" +description: "A document references a resource file (such as an image) whose source does not exist, so it could not be copied into the output." +code: Q-5-6 +subsystem: project +status: stub +since: "99.9.9" +categories: + - project +--- + +# `Q-5-6` — Referenced Resource Not Found + +> A document references a resource file (such as an image) +> whose source does not exist, so it could not be copied into +> the output. + +## What this means + +When a document references a local file — most commonly an +image, e.g. `![](images/diagram.png)` — Quarto copies that +file into the rendered output so the link resolves in the +published site. This warning fires when the referenced source +file does not exist on disk: there is nothing to copy. + +This is a **warning**, not an error. The render continues and +produces output; the missing file is simply left out. The +diagnostic points at the reference in your source document so +you can see exactly which link is affected. + +## Why this happens + +Common causes: + +- **A typo in the path** — wrong directory, wrong extension, + or a misspelled filename. +- **A file that was moved or deleted** but whose reference was + left behind. +- **A path that is relative to the wrong directory** — a + document-relative path resolves against the directory + containing the document, not the project root. (A leading + `/` makes it project-root-relative.) +- **A generated file that was expected from a code cell or an + earlier build step** that did not actually produce it. + +## How to fix + +Check the path in the reported reference against what is on +disk: + +- Correct the path if it is a typo. +- Add the missing file if it should exist. +- If the reference is intentional but the file is produced + later (e.g. by a separate build step), make sure that step + runs before the render. + +If the file genuinely should not be referenced, remove the +reference. + +## Related + +- `Q-5-7` — the source file existed, but the copy itself + failed (for example, a permission or disk-space problem). +- `Q-5-1` — a `resources:` pattern resolves outside the + project root. diff --git a/docs/errors/project/Q-5-7.qmd b/docs/errors/project/Q-5-7.qmd new file mode 100644 index 000000000..0fb7de652 --- /dev/null +++ b/docs/errors/project/Q-5-7.qmd @@ -0,0 +1,62 @@ +--- +title: "Resource Copy Failed" +description: "A referenced resource exists but could not be copied into the output directory because of a filesystem error." +code: Q-5-7 +subsystem: project +status: stub +since: "99.9.9" +categories: + - project +--- + +# `Q-5-7` — Resource Copy Failed + +> A referenced resource exists but could not be copied into +> the output directory because of a filesystem error. + +## What this means + +When a document references a local file (such as an image), +Quarto copies that file into the rendered output. This error +fires when the source file *does* exist but the copy itself +fails partway through — the filesystem refused or could not +complete the write. + +Unlike a missing source file (`Q-5-6`, a warning the render +recovers from), a failed copy is a hard error: Quarto cannot +produce a complete, self-contained output, so the render +stops. The diagnostic includes the underlying filesystem +condition as an advisory. + +## Why this happens + +Common causes: + +- **Permission denied** — the output directory (or a file + inside it) is not writable by the user running Quarto. +- **No space left on device** — the disk or volume holding the + output directory is full. +- **A read-only filesystem** — the output location is mounted + read-only. +- **A transient I/O error** — a network mount went away, or + the underlying storage reported an error mid-write. + +## How to fix + +Read the advisory in the message — it names the specific +filesystem condition: + +- **Permission denied:** make the output directory writable, + or run the render as a user who can write there. +- **No space left on device:** free up space, or point the + output at a volume with room. +- **Read-only filesystem:** choose a writable output location. +- **Transient errors:** retry, and consider whether the output + directory should live on a more reliable filesystem. + +## Related + +- `Q-5-6` — the reference pointed at a source file that does + not exist (a warning; the render continues). +- `Q-5-3` — a `resources:` glob walk failed for a similar + class of filesystem reason.