diff --git a/claude-notes/plans/2026-07-23-preview-engine-supporting-files.md b/claude-notes/plans/2026-07-23-preview-engine-supporting-files.md new file mode 100644 index 000000000..e4f0ccc18 --- /dev/null +++ b/claude-notes/plans/2026-07-23-preview-engine-supporting-files.md @@ -0,0 +1,327 @@ +# Preview: engine-generated images missing (`q2 preview` + hub-client q2-preview) + +**Braid strand:** bd-qbhp2cvv +**Status:** implemented on branch +`braid/bd-qbhp2cvv-preview-knitrjupyter-engine-generated`; E2E-verified +(knitr, real browser). jupyter blocked upstream by bd-rwz8kwia. + +## Overview + +Knitr (and, by the same mechanism, jupyter) execution results that include +images appear correctly in `q2 render` output but are missing in `q2 preview` +and in the hub-client q2-preview format. The engine's figure files +(`_files/figure-html/*.png`) never travel with the engine capture that +the preview replays, so the browser has nothing to show. + +## Reproduction + +Fixture: `~/Desktop/daily-log/2026/07/23/test-knitr-images-quarto-hub/hello.qmd` +(knitr doc with a ggplot cell; copied to a scratch dir for the runs below). + +```bash +# baseline — works +cargo run --bin q2 -- render /hello.qmd +# → hello.html references hello_files/figure-html/unnamed-chunk-2-1.png, +# file exists on disk, image renders. +# → .quarto/render-manifest.json records hello_files as an +# Engine-origin resource (kind: Engine, engine: knitr). + +# broken — repro +cargo run --bin q2 -- preview /hello.qmd --no-browser +# then: +curl -s -o /dev/null -w '%{http_code} %{content_type}\n' \ + http://127.0.0.1:/hello_files/figure-html/unnamed-chunk-2-1.png +# → 200 text/html ← SPA index fallback, NOT the PNG. Broken image. +``` + +Verified against the capture cache of the running preview +(`/captures/.bin`, gzipped JSON): + +```json +{ + "supporting_files": [ + "/abs/path/to/repro/hello_files" // path only — no bytes + ], + "markdown": "... ![](hello_files/figure-html/unnamed-chunk-2-1.png) ..." +} +``` + +## Diagnosis + +The preview architecture records an **engine capture** server-side and +**replays** it in the browser WASM pipeline: + +1. **Record** — `record_capture` (`crates/quarto-core/src/engine/preview_record.rs:130`) + runs the q2-preview pipeline truncated at `EngineExecutionStage`. The stage + emits an `EngineCapture` aux event + (`crates/quarto-core/src/stage/stages/engine_execution.rs:343-355`) + containing `{engine_name, input_qmd, result}` where `result` is the + serialized `ExecuteResult`. +2. **Store** — the capture is gzipped and written as a samod **binary doc**; + the sidecar/index entry (`CaptureRef.capture_doc_id`) points at it + (`crates/quarto-preview/src/re_execute.rs:337-354`, same in + `capture_driver.rs`). This is the "JSON sidecar deposited in the automerge + document" from the issue description. +3. **Replay** — the browser-side pipeline substitutes `ReplayEngine` + (`crates/quarto-core/src/engine/replay.rs`) for the real engine; it + returns the captured `ExecuteResult` verbatim. + +The hole: `ExecuteResult.supporting_files` is `Vec` +(`crates/quarto-core/src/engine/context.rs:151`) — **paths, not bytes**. +During recording, knitr writes the real figure files to disk next to the +source doc, and the capture records only the path. On replay: + +- **hub-client**: the WASM has no access to the recording machine's disk at + all. The rendered AST references `hello_files/figure-html/….png`; the asset + walker looks it up in the WASM VFS; it isn't there; broken image. +- **`q2 preview` CLI**: same replay path in the embedded SPA. Although the + figure files *do* exist on the server's disk, the preview server only + serves *declared* `resources:` (the `RESOURCE_DISK_MAP` artifact route, + `crates/quarto-preview/src/lib.rs:389`) — engine-generated files are not in + that map, so the request falls through to the SPA index fallback + (hence `200 text/html`). + +Contrast with `q2 render`: the orchestrator drains +`ctx.resource_report.add_engine_files(...)` +(`crates/quarto-core/src/stage/stages/engine_execution.rs:383-389`, contract +bd-o8pr) and copies engine supporting files into the output directory via +`copy_resources_to_output_dir` +(`crates/quarto-core/src/project/orchestrator.rs:928-964`), which is why +render works. **That copy is the exact divergence point: it is +`#[cfg(not(target_arch = "wasm32"))]`** (line 940, "Native only — the WASM +hub-client preview doesn't write to a real output dir"). On the WASM side, +replay re-adds the recorded paths to `ctx.resource_report`, but nothing +consumes them: the VFS flush (`flush_artifacts_to_vfs`, +`crates/wasm-quarto-hub-client/src/lib.rs:1547`) flushes only +`ctx.artifacts` (theme CSS/JS/fonts), never the resource report. + +Both browser-side image resolvers therefore miss: + +- AST/q2-preview path: `buildAssetManifest` + (`ts-packages/preview-renderer/src/q2-preview/assetWalker.ts:51`) → + `vfsReadBinaryFile(resolved)` misses → `continue` → image silently + omitted. +- HTML path (hub-client default `format: html`): + `ts-packages/preview-renderer/src/utils/iframePostProcessor.ts:203-235` → + VFS miss leaves the relative src, which the sandboxed iframe cannot + fetch → broken image. (So the bug affects the plain-HTML preview too, + not just the q2-preview AST format.) + +The figures also never enter the automerge project VFS by other routes: for +hub-client, knitr runs on a remote exec server +(`crates/quarto-hub-provider/src/execute.rs:301`) that never uploads the +generated `*_files/` output; the initial project scan happened before the +capture ran. + +### Why the fix belongs in the capture, not a server route + +A disk-backed server route would fix only the CLI preview. The hub-client +replays the same capture doc on machines that never ran the engine, so the +bytes must travel **inside the capture binary doc** (which already syncs via +automerge). One fix covers both consumers — this matches the existing +design where the capture doc is the single unit of engine-result transport. + +### Existing browser-side asset mechanism (reuse, don't invent) + +`buildAssetManifest` +(`ts-packages/preview-renderer/src/q2-preview/assetWalker.ts`) already walks +the rendered AST for `Image` nodes, resolves each target against the doc's +`/project/…` VFS path, reads bytes via `vfsReadBinaryFile`, and mints blob +URLs consumed by the iframe's `` component. If the engine-generated +figures are **materialized into the WASM VFS** at the right `/project/…` +path at replay time, images light up with **zero renderer changes**, in both +hub-client and the q2-preview SPA. + +## Proposed fix (sketch — to be refined) + +1. **Capture side** (`EngineExecutionStage`, native only): when emitting the + `EngineCapture` aux event, enumerate `result.supporting_files` (files and + directories, recursively), read the bytes, and attach a + `files: [{path: , contents_base64}]` array to the capture + payload. Paths are stored doc-relative (e.g. + `hello_files/figure-html/unnamed-chunk-2-1.png`) so replay is + machine-independent. +2. **Schema** (`quarto_trace::EngineCapture`): new `#[serde(default)]` + field so old captures (no `files`) still deserialize; replay treats the + absence as "no files" (current behavior). +3. **Replay side**: implementation-scoping correction discovered during + execution — the preview does **not** consume captures via + `ReplayEngine` (that remains the bd-45yw regression tool). Both the + SPA and hub-client thread `capture_gz_json` into the q2-preview + pipeline's **`CaptureSpliceStage`** + (`crates/quarto-core/src/stage/stages/capture_splice.rs`, bd-lucp), + which splices captured output blocks into the live AST. That stage + has `ctx.runtime` — and in WASM, `SystemRuntime::file_write` *is* + the VFS write. So materialization lives in `CaptureSpliceStage::run`: + for each capture, write its embedded `files` to + `/` via the runtime before splicing. + This is a pure quarto-core change — no wasm-bindgen surface changes, + and it works identically under native tests (writes to the temp + project dir) and WASM (writes to the VFS the + assetWalker/iframePostProcessor resolvers read). +4. **No SPA/renderer changes expected** — the existing resolvers + (`assetWalker.ts` blob URLs for the AST path, + `iframePostProcessor.ts` data URIs for the HTML path) pick the files + out of the VFS once they're present. + +### Design alternative considered and rejected + +Uploading the figures as separate automerge binary docs (or into the +project file tree) keyed by path: more moving parts, pollutes the synced +project with generated artifacts, and requires the remote exec server +(hub) and local preview to each grow an upload path. Embedding in the +capture doc keeps "one capture = one self-contained engine run" and rides +the existing sync/binary-doc plumbing unchanged. + +### Decisions (reviewed with Carlos, 2026-07-23) + +- **Size policy:** v1 unbounded, with a `tracing::warn!` when the + gzipped capture doc exceeds **10 MB**. Hard caps deferred until a real + doc hurts. +- **Directories:** embed `supporting_files` directory entries wholesale + in v1 (the `*_files` dir contains only what the engine generated for + this doc). +- **jupyter:** in scope — verify with a basic matplotlib doc (see test + plan; fixture below). +- **Old captures:** fine to replay without images until re-executed; the + existing staleness/re-execute machinery refreshes them. `#[serde(default)]` + keeps deserialization working. +- **No disk-serving route in `q2 preview`:** not needed. If execution of + this plan hits an unexpected wall that would require it, stop and + discuss before building it. + +jupyter verification fixture: + +```qmd +--- +title: hello jupyter +engine: jupyter +--- + +```{python} +import matplotlib.pyplot as plt +plt.plot([1,2,3]) +``` +``` + +## Related work + +- `claude-notes/plans/2026-06-09-preview-embed-vfs-resolution.md` + (bd-kjrpya2d) names this exact gap: `copy_resources_to_output_dir` is + native-only, and the assetWalker fallback "relies on referenced static + assets being present in the VFS source" — which generated figures are + not. +- `claude-notes/plans/2026-05-13-q2-preview-phase-c.md` — capture + architecture (Phase C); risk #4 is the capture-size discussion. +- bd-eiku4ymo (related, filed from this plan's review) — uncompressed + audit/GC metadata envelope on capture binary docs (createdAt, + sourcePath, engines) for sync-server audits and orphaned-capture + garbage collection. Deliberately kept out of this bugfix's scope. +- bd-o8pr — supporting-files / resource-report contract. +- bd-45yw / bd-5yff4 — replay engine + multi-engine captures. + +## Test plan (TDD — first phase of execution) + +- [ ] Rust unit: capture recorded for an engine whose `ExecuteResult` lists a + supporting file embeds the file bytes (passthrough test engine writing + a fake PNG; assert `files` in the emitted capture payload). +- [ ] Rust unit: capture without `files` field (old shape) still + deserializes and replays (serde default). +- [ ] Rust unit: replay materializes embedded files into the runtime VFS at + the doc-relative path. +- [ ] Rust unit: capture-doc size warning fires above 10 MB gzipped (and + not below). +- [ ] hub-client/WASM test: `assetManifestProject`-style test where a + captured doc with an embedded figure renders an `` with a blob + URL (follow existing `*.wasm.test.ts` patterns). +- [ ] E2E (knitr): `q2 preview` on the repro doc; fetch the figure + URL/observe the preview in a browser; image renders (manual + verification recorded in this plan per the end-to-end policy). +- [ ] E2E (jupyter): same with the matplotlib fixture above. + +## Implementation notes (finalized at execution start, 2026-07-23) + +- **Cost gating:** file collection runs only when + `PipelineObserver::wants_engine_capture_files()` returns true (new + trait method, default `false`). Only the preview's `CaptureCollector` + (`preview_record.rs`) opts in — plain `q2 render` and trace observers + pay zero extra I/O. (`JsonTraceObserver` deliberately stays opted + out for v1 so `-v` trace files don't bloat; revisit if ReplayEngine + ever wants materialization.) +- **New module** `crates/quarto-core/src/engine/capture_files.rs`: + `collect_capture_files` (record side) + `materialize_capture_files` + (splice side) + `capture_doc_size_warning` (10 MB, pure fn) + + shared `gzip_captures` used by all three writers + (`capture_driver.rs`, `re_execute.rs`, hub-provider `execute.rs` — + currently three duplicated gzip blocks). +- **Path convention:** `CaptureFile.path` is doc-relative with forward + slashes. Recording resolves `supporting_files` entries (absolute or + doc-relative) against the doc's parent dir; entries that don't + resolve under the doc dir are warn+skipped (relative image refs + couldn't reach them anyway). hub-provider runs engines in a deleted + temp dir, so recording-time embedding is the only moment the bytes + exist — confirming the design. +- **Serialization stability:** `files` uses + `#[serde(default, skip_serializing_if = "Vec::is_empty")]` — old + captures deserialize, and captures without files serialize + byte-identically to today (existing snapshots unaffected). + +## Work items + +- [x] Phase 1: `EngineCapture.files` schema (quarto-trace) + literal-site updates +- [x] Phase 2: capture-side embedding (observer gate, collect_capture_files, + engine_execution wiring) — failing test first +- [x] Phase 3: replay-side VFS materialization in CaptureSpliceStage — + failing test first +- [x] Phase 4: shared gzip helper + 10 MB warning in the three writers +- [x] Phase 5: end-to-end verification — knitr confirmed in a real + browser; jupyter blocked upstream (see below); `cargo xtask verify` + run at the end +- [x] Phase 6: docs/changelog — no `hub-client/` files changed (the fix + is entirely Rust-side; hub-client picks it up on its next WASM + rebuild), so the file-based changelog rule doesn't trigger. No + user-facing docs impact (bug fix, no new options). + +## End-to-end verification record (2026-07-23) + +Per the repo's end-to-end policy — exact invocations, observed output, +output inspected. + +**knitr (the reported bug) — FIXED, verified in a real browser:** + +```bash +# fresh scratch copy of the repro doc (hello.qmd, ggplot cell) +target/debug/q2 preview hello.qmd --no-browser # after full WASM chain rebuild: +# cd hub-client && npm run build:wasm +# cargo xtask build-q2-preview-spa +# cargo build --bin q2 +``` + +1. Server-side capture now embeds the figure (gunzip of the session's + `captures/*.bin`): + `[{"path": "hello_files/figure-html/unnamed-chunk-2-1.png", "bytes": 29276}]` + (base64 length; ≈22 KB of PNG). +2. In Chrome (DevTools MCP) against the running preview, the rendered + iframe contains + `{"src": "blob:http://127.0.0.1:55919/…", "loaded": true, "naturalWidth": 672, "naturalHeight": 480}` + — the ggplot scatter is visibly rendered (screenshot inspected). + This is precisely the predicted mechanism: capture → VFS + materialization in `CaptureSpliceStage` → `assetWalker` blob URL. + No TS/renderer changes were needed. Console shows no errors. + +**jupyter — capture transport confirmed, but blocked upstream:** + +The matplotlib fixture produces *no image anywhere*: `q2 render` emits +only the text reprs (`[]`, `
`), `ExecuteResult.supporting_files` is empty, and +the capture consequently has nothing to transport. Render/preview +parity holds; this is a jupyter-engine display_data gap, filed as +**bd-rwz8kwia** (discovered-from bd-qbhp2cvv). Once the engine emits +figures + supporting files, this fix transports them with no further +work (the transport is engine-agnostic, pinned by the +figure-writing-engine unit test). + +**Gotcha fixed during E2E:** the first WASM build failed because +`flate2` had been added to quarto-core's *native-only* dependency +section; moved to `[dependencies]` (flate2 builds cleanly on wasm32 via +miniz_oxide — the WASM client already gunzips captures with it). diff --git a/crates/quarto-core/Cargo.toml b/crates/quarto-core/Cargo.toml index 48a4c4026..eb7565f1e 100644 --- a/crates/quarto-core/Cargo.toml +++ b/crates/quarto-core/Cargo.toml @@ -74,6 +74,10 @@ include_dir = "0.7" # or other non-CLI consumers. The `quarto` crate enables this via # `quarto-core/clap`; no other consumer needs it. clap = { workspace = true, optional = true, features = ["derive"] } +# Capture-doc wire format (engine/capture_files.rs::gzip_captures). +# Needed on wasm32 too — the module compiles on both targets, and +# flate2 (miniz_oxide backend) builds cleanly for wasm. +flate2.workspace = true [features] clap = ["dep:clap"] @@ -115,7 +119,6 @@ tokio = { version = "1", features = ["sync", "time", "rt-multi-thread", "process uuid.workspace = true [dev-dependencies] -flate2.workspace = true insta.workspace = true quarto-error-catalog = { workspace = true } tempfile = "3" diff --git a/crates/quarto-core/src/engine/capture_files.rs b/crates/quarto-core/src/engine/capture_files.rs new file mode 100644 index 000000000..c585b2fd1 --- /dev/null +++ b/crates/quarto-core/src/engine/capture_files.rs @@ -0,0 +1,435 @@ +/* + * engine/capture_files.rs + * Copyright (c) 2026 Posit, PBC + * + * Embed and materialize engine-generated supporting files in engine + * captures (bd-qbhp2cvv). + */ + +//! Supporting-file transport for engine captures (bd-qbhp2cvv). +//! +//! Engines like knitr and jupyter write figure files to disk during +//! execution (`_files/figure-html/…`) and report them in +//! [`ExecuteResult::supporting_files`](super::ExecuteResult) — as +//! *paths*. `q2 render` copies those paths into the output directory +//! (`copy_resources_to_output_dir`, native only), but preview replay +//! happens on a machine (or a browser VFS) where the paths don't +//! exist; the hub even runs engines in a temp dir that is deleted +//! right after recording. So the bytes must travel inside the +//! capture: +//! +//! - [`collect_capture_files`] runs at recording time, right where +//! the `EngineCapture` aux event is emitted +//! (`EngineExecutionStage`), while the files still exist. It reads +//! every reported file (recursing into reported directories) and +//! returns [`CaptureFile`]s keyed by doc-relative, forward-slash +//! paths. +//! - [`materialize_capture_files`] runs at splice time +//! ([`CaptureSpliceStage`](crate::stage::stages::CaptureSpliceStage)), +//! writing the embedded bytes next to the live document via the +//! context's [`SystemRuntime`] — which in WASM is the VFS that the +//! preview's image resolvers (`assetWalker.ts`, +//! `iframePostProcessor.ts`) read. +//! +//! Collection is gated by +//! [`PipelineObserver::wants_engine_capture_files`](crate::stage::PipelineObserver::wants_engine_capture_files) +//! so plain `q2 render` never pays the extra file I/O. + +use std::path::{Path, PathBuf}; + +use base64::Engine as _; +use base64::engine::general_purpose::STANDARD as BASE64; +use quarto_system_runtime::SystemRuntime; +use quarto_trace::CaptureFile; + +/// Read the engine's reported supporting files and package them for +/// embedding in an [`quarto_trace::EngineCapture`]. +/// +/// `supporting_files` entries may be absolute or doc-dir-relative +/// (the [`ExecuteResult::supporting_files`](super::ExecuteResult) +/// contract); each may be a file or a directory (knitr reports the +/// whole `_files` directory). Directories are walked +/// recursively. Files are keyed by their path relative to `doc_dir`, +/// with forward-slash separators, so a capture recorded on any +/// platform replays on any other. +/// +/// Fail-soft per entry: files that cannot be read, or that resolve +/// outside `doc_dir` (a relative image reference in the document +/// could never reach those anyway), are skipped with a warning. +/// Ordering is deterministic (directory listings are sorted) so +/// identical runs produce identical capture bytes for the +/// content-hash-keyed capture cache. +pub fn collect_capture_files( + runtime: &dyn SystemRuntime, + doc_dir: &Path, + supporting_files: &[PathBuf], +) -> Vec { + let mut out = Vec::new(); + for entry in supporting_files { + let resolved = if entry.is_absolute() { + entry.clone() + } else { + doc_dir.join(entry) + }; + collect_path(runtime, doc_dir, &resolved, &mut out); + } + out +} + +fn collect_path( + runtime: &dyn SystemRuntime, + doc_dir: &Path, + path: &Path, + out: &mut Vec, +) { + if runtime.is_dir(path).unwrap_or(false) { + let mut children = match runtime.dir_list(path) { + Ok(children) => children, + Err(e) => { + tracing::warn!( + path = %path.display(), + error = %e, + "capture-files: cannot list supporting-file directory; skipping" + ); + return; + } + }; + children.sort(); + for child in children { + collect_path(runtime, doc_dir, &child, out); + } + return; + } + + let Some(rel) = doc_relative_slash_path(doc_dir, path) else { + tracing::warn!( + path = %path.display(), + doc_dir = %doc_dir.display(), + "capture-files: supporting file is outside the document directory; skipping \ + (relative references in the document cannot reach it)" + ); + return; + }; + + match runtime.file_read(path) { + Ok(bytes) => out.push(CaptureFile { + path: rel, + contents_base64: BASE64.encode(&bytes), + }), + Err(e) => { + tracing::warn!( + path = %path.display(), + error = %e, + "capture-files: cannot read supporting file; skipping" + ); + } + } +} + +/// Compute `path` relative to `doc_dir` as a forward-slash string. +/// Returns `None` when `path` is not under `doc_dir`. +fn doc_relative_slash_path(doc_dir: &Path, path: &Path) -> Option { + let rel = path.strip_prefix(doc_dir).ok()?; + let parts: Vec = rel + .components() + .map(|c| c.as_os_str().to_string_lossy().into_owned()) + .collect(); + if parts.is_empty() { + return None; + } + Some(parts.join("/")) +} + +/// Write a capture's embedded files next to the live document so +/// relative references in the spliced output resolve. +/// +/// In WASM, `runtime.file_write` targets the preview VFS — exactly +/// where the SPA/hub-client image resolvers look. Natively (tests), +/// it writes real files under the document's directory. +/// +/// Fail-soft per file: a bad base64 payload or a write failure skips +/// that file with a warning; the splice proceeds (worst case is the +/// same broken image the user has today, never a broken preview). +/// Paths are validated to stay under `doc_dir` — a capture is remote +/// data, and a crafted `../…` entry must not escape the document +/// directory. +pub fn materialize_capture_files( + runtime: &dyn SystemRuntime, + doc_dir: &Path, + files: &[CaptureFile], +) { + for file in files { + if !is_safe_relative_path(&file.path) { + tracing::warn!( + path = %file.path, + "capture-files: refusing to materialize non-relative or escaping path" + ); + continue; + } + let target = doc_dir.join(&file.path); + let bytes = match BASE64.decode(&file.contents_base64) { + Ok(bytes) => bytes, + Err(e) => { + tracing::warn!( + path = %file.path, + error = %e, + "capture-files: invalid base64 in capture; skipping" + ); + continue; + } + }; + if let Some(parent) = target.parent() + && let Err(e) = runtime.dir_create(parent, true) + { + tracing::warn!( + path = %target.display(), + error = %e, + "capture-files: cannot create parent directory; skipping" + ); + continue; + } + if let Err(e) = runtime.file_write(&target, &bytes) { + tracing::warn!( + path = %target.display(), + error = %e, + "capture-files: cannot write file; skipping" + ); + } + } +} + +/// Warn threshold for a gzipped capture doc: 10 MB (decision recorded +/// in `claude-notes/plans/2026-07-23-preview-engine-supporting-files.md`). +/// Embedded figure bytes (Phase 2) can push multi-plot docs past the +/// point where syncing the capture binary doc hurts; no hard cap in +/// v1, but the operator gets a signal. +pub const CAPTURE_DOC_WARN_BYTES: usize = 10 * 1024 * 1024; + +/// If a gzipped capture doc of `gzipped_len` bytes exceeds +/// [`CAPTURE_DOC_WARN_BYTES`], return the warning message to log. +/// Pure so the threshold behaviour is directly unit-testable; the +/// callers ([`gzip_captures`] and the capture-doc writers) feed it to +/// `tracing::warn!`. +pub fn capture_doc_size_warning(gzipped_len: usize) -> Option { + (gzipped_len > CAPTURE_DOC_WARN_BYTES).then(|| { + format!( + "capture doc is {:.1} MB gzipped (> {} MB): engine output embeds large \ + supporting files; syncing and storing this capture may be slow", + gzipped_len as f64 / (1024.0 * 1024.0), + CAPTURE_DOC_WARN_BYTES / (1024 * 1024), + ) + }) +} + +/// Serialize + gzip an `EngineCapture` sequence — the shared wire +/// format of the capture binary doc (`application/x-engine-capture+gzip`). +/// Single implementation for the three capture writers +/// (`quarto-preview`'s eager driver and re-execute handler, +/// `quarto-hub-provider`'s exec server), all of which previously +/// duplicated the gzip block. Logs [`capture_doc_size_warning`] when +/// the result is oversized. +pub fn gzip_captures(captures: &[quarto_trace::EngineCapture]) -> std::io::Result> { + use std::io::Write as _; + let json = serde_json::to_vec(captures)?; + let mut enc = flate2::write::GzEncoder::new(Vec::new(), flate2::Compression::default()); + enc.write_all(&json)?; + let gzipped = enc.finish()?; + if let Some(msg) = capture_doc_size_warning(gzipped.len()) { + tracing::warn!("{msg}"); + } + Ok(gzipped) +} + +/// A capture file path is safe when it is relative and contains no +/// `..` or root components — it must resolve strictly under the +/// document directory. +fn is_safe_relative_path(path: &str) -> bool { + use std::path::Component; + let p = Path::new(path); + !path.is_empty() + && p.components() + .all(|c| matches!(c, Component::Normal(_) | Component::CurDir)) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::Arc; + + use quarto_system_runtime::NativeRuntime; + use tempfile::TempDir; + + fn write(path: &Path, bytes: &[u8]) { + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(path, bytes).unwrap(); + } + + #[test] + fn collects_directory_recursively_with_doc_relative_paths() { + let tmp = TempDir::new().unwrap(); + let doc_dir = tmp.path().canonicalize().unwrap(); + write(&doc_dir.join("doc_files/figure-html/a.png"), b"AAA"); + write(&doc_dir.join("doc_files/figure-html/b.png"), b"BBB"); + write(&doc_dir.join("doc_files/data.csv"), b"1,2"); + + let runtime: Arc = Arc::new(NativeRuntime::new()); + let files = collect_capture_files(runtime.as_ref(), &doc_dir, &[doc_dir.join("doc_files")]); + + let mut paths: Vec<&str> = files.iter().map(|f| f.path.as_str()).collect(); + paths.sort_unstable(); + assert_eq!( + paths, + vec![ + "doc_files/data.csv", + "doc_files/figure-html/a.png", + "doc_files/figure-html/b.png", + ] + ); + let a = files + .iter() + .find(|f| f.path == "doc_files/figure-html/a.png") + .unwrap(); + assert_eq!(BASE64.decode(&a.contents_base64).unwrap(), b"AAA"); + } + + #[test] + fn collects_single_file_and_relative_entry() { + let tmp = TempDir::new().unwrap(); + let doc_dir = tmp.path().canonicalize().unwrap(); + write(&doc_dir.join("fig.png"), b"PNG"); + + let runtime: Arc = Arc::new(NativeRuntime::new()); + // Doc-dir-relative entry (the ExecuteResult contract allows both). + let files = collect_capture_files(runtime.as_ref(), &doc_dir, &[PathBuf::from("fig.png")]); + assert_eq!(files.len(), 1); + assert_eq!(files[0].path, "fig.png"); + } + + #[test] + fn skips_files_outside_doc_dir() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path().canonicalize().unwrap(); + let doc_dir = root.join("proj"); + std::fs::create_dir_all(&doc_dir).unwrap(); + write(&root.join("outside.png"), b"X"); + + let runtime: Arc = Arc::new(NativeRuntime::new()); + let files = collect_capture_files(runtime.as_ref(), &doc_dir, &[root.join("outside.png")]); + assert!(files.is_empty(), "outside-doc-dir file must be skipped"); + } + + #[test] + fn missing_entry_is_skipped_not_fatal() { + let tmp = TempDir::new().unwrap(); + let doc_dir = tmp.path().canonicalize().unwrap(); + let runtime: Arc = Arc::new(NativeRuntime::new()); + let files = + collect_capture_files(runtime.as_ref(), &doc_dir, &[doc_dir.join("never-created")]); + assert!(files.is_empty()); + } + + #[test] + fn materializes_files_under_doc_dir() { + let tmp = TempDir::new().unwrap(); + let doc_dir = tmp.path().canonicalize().unwrap(); + let runtime: Arc = Arc::new(NativeRuntime::new()); + + let files = vec![CaptureFile { + path: "doc_files/figure-html/fig.png".into(), + contents_base64: BASE64.encode(b"PNGDATA"), + }]; + materialize_capture_files(runtime.as_ref(), &doc_dir, &files); + + let on_disk = std::fs::read(doc_dir.join("doc_files/figure-html/fig.png")).unwrap(); + assert_eq!(on_disk, b"PNGDATA"); + } + + #[test] + fn materialize_refuses_escaping_paths() { + let tmp = TempDir::new().unwrap(); + let root = tmp.path().canonicalize().unwrap(); + let doc_dir = root.join("proj"); + std::fs::create_dir_all(&doc_dir).unwrap(); + let runtime: Arc = Arc::new(NativeRuntime::new()); + + for evil in ["../evil.png", "/abs/evil.png", ""] { + materialize_capture_files( + runtime.as_ref(), + &doc_dir, + &[CaptureFile { + path: evil.into(), + contents_base64: BASE64.encode(b"X"), + }], + ); + } + assert!( + !root.join("evil.png").exists(), + "escaping path must not be written" + ); + assert!(!Path::new("/abs/evil.png").exists()); + } + + #[test] + fn materialize_skips_invalid_base64() { + let tmp = TempDir::new().unwrap(); + let doc_dir = tmp.path().canonicalize().unwrap(); + let runtime: Arc = Arc::new(NativeRuntime::new()); + materialize_capture_files( + runtime.as_ref(), + &doc_dir, + &[CaptureFile { + path: "fig.png".into(), + contents_base64: "!!!not-base64!!!".into(), + }], + ); + assert!(!doc_dir.join("fig.png").exists()); + } + + #[test] + fn size_warning_fires_only_above_threshold() { + assert!(capture_doc_size_warning(0).is_none()); + assert!(capture_doc_size_warning(CAPTURE_DOC_WARN_BYTES).is_none()); + let msg = capture_doc_size_warning(CAPTURE_DOC_WARN_BYTES + 1) + .expect("one byte over the threshold must warn"); + assert!(msg.contains("10 MB"), "message names the threshold: {msg}"); + } + + #[test] + fn gzip_captures_roundtrips_through_gunzip() { + use std::io::Read as _; + let capture = quarto_trace::EngineCapture { + engine_name: "knitr".into(), + input_qmd: "```{r}\n1\n```\n".into(), + result: serde_json::json!({"markdown": "out"}), + files: vec![CaptureFile { + path: "f.png".into(), + contents_base64: BASE64.encode(b"PNG"), + }], + }; + let gzipped = gzip_captures(std::slice::from_ref(&capture)).unwrap(); + let mut json = Vec::new(); + flate2::read::GzDecoder::new(&gzipped[..]) + .read_to_end(&mut json) + .unwrap(); + let back: Vec = serde_json::from_slice(&json).unwrap(); + assert_eq!(back.len(), 1); + assert_eq!(back[0].files[0].path, "f.png"); + } + + #[test] + fn round_trip_collect_then_materialize() { + let tmp = TempDir::new().unwrap(); + let src_dir = tmp.path().canonicalize().unwrap().join("src"); + let dst_dir = tmp.path().canonicalize().unwrap().join("dst"); + std::fs::create_dir_all(&src_dir).unwrap(); + std::fs::create_dir_all(&dst_dir).unwrap(); + write(&src_dir.join("d_files/figure-html/f.png"), b"\x89PNG\r\n"); + + let runtime: Arc = Arc::new(NativeRuntime::new()); + let files = collect_capture_files(runtime.as_ref(), &src_dir, &[src_dir.join("d_files")]); + materialize_capture_files(runtime.as_ref(), &dst_dir, &files); + + let out = std::fs::read(dst_dir.join("d_files/figure-html/f.png")).unwrap(); + assert_eq!(out, b"\x89PNG\r\n"); + } +} diff --git a/crates/quarto-core/src/engine/mod.rs b/crates/quarto-core/src/engine/mod.rs index dc68f28c3..f2fe26d61 100644 --- a/crates/quarto-core/src/engine/mod.rs +++ b/crates/quarto-core/src/engine/mod.rs @@ -53,6 +53,7 @@ //! let result = engine.execute(&qmd_content, &context)?; //! ``` +pub mod capture_files; pub mod capture_splice; mod context; mod detection; diff --git a/crates/quarto-core/src/engine/preview_record.rs b/crates/quarto-core/src/engine/preview_record.rs index c94c6763b..593ccc2f3 100644 --- a/crates/quarto-core/src/engine/preview_record.rs +++ b/crates/quarto-core/src/engine/preview_record.rs @@ -96,6 +96,13 @@ impl PipelineObserver for CaptureCollector { // Other methods inherit no-op defaults; spell out the few that // would otherwise spam logs to keep this observer truly silent. fn on_event(&self, _message: &str, _level: EventLevel) {} + + // bd-qbhp2cvv: preview captures replay on machines (or browser + // VFSes) that cannot see this machine's disk — ask the stage to + // embed supporting-file bytes in the capture payload. + fn wants_engine_capture_files(&self) -> bool { + true + } } /// Build the engine-capture sub-pipeline: the q2-preview HTML pipeline @@ -377,6 +384,75 @@ mod tests { assert!(result.is_empty()); } + // ────────────────────────────────────────────────────────────── + // bd-qbhp2cvv: supporting-file bytes embedded in the capture + // ────────────────────────────────────────────────────────────── + + /// Engine that writes a fake figure file next to the document — + /// the disk shape knitr produces (`_files/figure-html/…`) — + /// and reports the `_files` directory in `supporting_files`, + /// exactly like `KnitrEngine` does. + struct FigureWritingTestEngine { + doc_dir: PathBuf, + } + + impl ExecutionEngine for FigureWritingTestEngine { + fn name(&self) -> &str { + "test-figures" + } + + fn execute( + &self, + input: &str, + _ctx: &ExecutionContext, + ) -> Result { + let fig_dir = self.doc_dir.join("doc_files").join("figure-html"); + std::fs::create_dir_all(&fig_dir).expect("create figure dir"); + std::fs::write(fig_dir.join("fig.png"), b"FAKE-PNG-BYTES").expect("write figure"); + let mut out = String::from(input); + out.push_str("\n![](doc_files/figure-html/fig.png)\n"); + Ok(ExecuteResult::new(out).with_supporting_files(vec![self.doc_dir.join("doc_files")])) + } + } + + #[tokio::test] + async fn capture_embeds_engine_supporting_file_bytes() { + // The engine writes doc_files/figure-html/fig.png during + // execution (as knitr does) and reports the directory in + // `supporting_files`. The recorded capture must embed the + // file's *bytes* at its doc-relative path — paths alone are + // useless on the replaying machine (bd-qbhp2cvv). + let (_tmp, path, project, runtime) = + fixture("---\nengine: test-figures\n---\n\n```{test-figures}\nplot(1)\n```\n"); + + let mut registry = EngineRegistry::new(); + registry.register(Arc::new(FigureWritingTestEngine { + doc_dir: path.parent().unwrap().to_path_buf(), + })); + + let captures = record_capture(&path, &project, runtime, Some(registry)) + .await + .expect("pipeline runs"); + let capture = captures.first().expect("capture present"); + + assert_eq!( + capture.files.len(), + 1, + "capture should embed exactly the one engine-generated file; got {:?}", + capture.files.iter().map(|f| &f.path).collect::>() + ); + let file = &capture.files[0]; + assert_eq!( + file.path, "doc_files/figure-html/fig.png", + "embedded path must be doc-relative with forward slashes" + ); + use base64::Engine as _; + let bytes = base64::engine::general_purpose::STANDARD + .decode(&file.contents_base64) + .expect("valid base64"); + assert_eq!(bytes, b"FAKE-PNG-BYTES"); + } + // ────────────────────────────────────────────────────────────── // Phase C.2: compute_input_qmd // ────────────────────────────────────────────────────────────── diff --git a/crates/quarto-core/src/engine/replay.rs b/crates/quarto-core/src/engine/replay.rs index f48fe618f..d8ba48fd1 100644 --- a/crates/quarto-core/src/engine/replay.rs +++ b/crates/quarto-core/src/engine/replay.rs @@ -162,6 +162,7 @@ mod tests { }, "needs_postprocess": false, }), + files: Vec::new(), } } @@ -244,6 +245,7 @@ mod tests { engine_name: "jupyter".into(), input_qmd: "x".into(), result: json!({"not_an_execute_result": true}), + files: Vec::new(), }; let recorded_input = capture.input_qmd.clone(); let engine = ReplayEngine::new(capture); @@ -279,6 +281,7 @@ mod tests { engine_name: "knitr".into(), input_qmd: "input".into(), result: result_value, + files: Vec::new(), }; let engine = ReplayEngine::new(capture); let ctx = make_test_context(); diff --git a/crates/quarto-core/src/pipeline.rs b/crates/quarto-core/src/pipeline.rs index 730e2d6ed..18c64e05a 100644 --- a/crates/quarto-core/src/pipeline.rs +++ b/crates/quarto-core/src/pipeline.rs @@ -1609,6 +1609,7 @@ mod tests { result: serde_json::json!({ "markdown": "::: {.cell}\n```{.markerlang .cell-code}\n1 + 1\n```\n\n::: {.cell-output .cell-output-stdout}\n```\nSPLICEMARKER_ZX9\n```\n:::\n:::\n" }), + files: Vec::new(), }; let project = make_test_project(); @@ -1652,6 +1653,69 @@ mod tests { ); } + /// bd-qbhp2cvv: a capture carrying embedded supporting-file bytes + /// must have them materialized next to the document when the + /// splice runs — that is how engine-generated figures become + /// readable by the preview's VFS-based image resolvers (and, in + /// this native test, appear on disk under the doc's directory). + #[tokio::test] + async fn capture_splice_materializes_embedded_files_next_to_doc() { + use base64::Engine as _; + use quarto_trace::{CaptureFile, EngineCapture}; + + let tmp = tempfile::TempDir::new().unwrap(); + let dir = tmp.path().canonicalize().unwrap(); + let doc_path = dir.join("test.qmd"); + + let qmd = "---\ntitle: T\nengine: markerlang\n---\n\n```{markerlang}\nplot(1)\n```\n"; + let capture = EngineCapture { + engine_name: "markerlang".into(), + input_qmd: "```{markerlang}\nplot(1)\n```\n".into(), + result: serde_json::json!({ + "markdown": "::: {.cell}\n![](test_files/figure-html/fig.png)\n:::\n" + }), + files: vec![CaptureFile { + path: "test_files/figure-html/fig.png".into(), + contents_base64: base64::engine::general_purpose::STANDARD.encode(b"FAKE-PNG"), + }], + }; + + let project = ProjectContext { + dir: dir.clone(), + config: crate::project::ProjectConfig::default(), + is_single_file: true, + files: vec![DocumentInfo::from_path(&doc_path)], + output_dir: dir.clone(), + }; + let doc = DocumentInfo::from_path(&doc_path); + let format = Format::html(); + let binaries = BinaryDependencies::new(); + let mut ctx = RenderContext::new(&project, &doc, &format, &binaries); + let config = HtmlRenderConfig::default().with_captures(vec![capture]); + let out = render_qmd_to_html( + qmd.as_bytes(), + &doc_path.to_string_lossy(), + &mut ctx, + &config, + make_test_runtime(), + ) + .await + .unwrap(); + + // The spliced output references the figure... + assert!( + out.html.contains("test_files/figure-html/fig.png"), + "spliced image ref must appear in the HTML; got:\n{}", + out.html + ); + // ...and the splice materialized its bytes next to the doc. + let materialized = dir.join("test_files/figure-html/fig.png"); + assert_eq!( + std::fs::read(&materialized).expect("figure file materialized next to the doc"), + b"FAKE-PNG" + ); + } + #[test] fn test_render_with_callout() { let content = @@ -2484,6 +2548,7 @@ mod tests { }, "needs_postprocess": false, }), + files: Vec::new(), }; let replay_registry = EngineRegistry::with_replay(capture); @@ -2538,6 +2603,7 @@ mod tests { engine_name: "replay-only-engine".into(), input_qmd: String::new(), result: serde_json::json!({ "markdown": "" }), + files: Vec::new(), }; let project = make_test_project(); diff --git a/crates/quarto-core/src/stage/observer.rs b/crates/quarto-core/src/stage/observer.rs index 6c9e023c9..bc332cf2e 100644 --- a/crates/quarto-core/src/stage/observer.rs +++ b/crates/quarto-core/src/stage/observer.rs @@ -215,6 +215,21 @@ pub trait PipelineObserver: Send + Sync { _data: &serde_json::Value, ) { } + + /// Whether this observer wants engine-generated supporting-file + /// *bytes* embedded in the `EngineCapture` aux payload + /// (bd-qbhp2cvv). + /// + /// Reading and base64-encoding figure files costs real I/O on + /// every engine run, so `EngineExecutionStage` only collects them + /// when the observer opts in. The preview capture recorder + /// (`preview_record::CaptureCollector`) opts in — its captures + /// replay on machines that can't see the recorder's disk. + /// Everything else (plain renders, trace observers) keeps the + /// default `false` and captures paths only. + fn wants_engine_capture_files(&self) -> bool { + false + } } /// No-op observer implementation. diff --git a/crates/quarto-core/src/stage/stages/capture_splice.rs b/crates/quarto-core/src/stage/stages/capture_splice.rs index ddc8d6c8c..b2086ce7e 100644 --- a/crates/quarto-core/src/stage/stages/capture_splice.rs +++ b/crates/quarto-core/src/stage/stages/capture_splice.rs @@ -129,6 +129,37 @@ impl PipelineStage for CaptureSpliceStage { return Ok(PipelineData::DocumentAst(doc_ast)); } + // bd-qbhp2cvv: before splicing, materialize each capture's + // embedded supporting files (engine-generated figures) next to + // the live document, so the relative image references in the + // spliced output resolve. In WASM `ctx.runtime` writes to the + // preview VFS — exactly where the SPA/hub-client image + // resolvers (assetWalker / iframePostProcessor) read. The doc + // dir comes from the AST's path (absolute in the WASM VFS and + // in `q2 render`); a bare source name (some native test + // harnesses) falls back to the project dir. + let doc_dir = doc_ast + .path + .parent() + .filter(|p| !p.as_os_str().is_empty()) + .map_or_else(|| ctx.project.dir.clone(), std::path::Path::to_path_buf); + for capture in &self.captures { + if !capture.files.is_empty() { + trace_event!( + ctx, + EventLevel::Debug, + "capture-splice: materializing {} supporting file(s) for engine={}", + capture.files.len(), + capture.engine_name + ); + crate::engine::capture_files::materialize_capture_files( + ctx.runtime.as_ref(), + &doc_dir, + &capture.files, + ); + } + } + // bd-5yff4: fold the captures in order. Engine N+1's splice runs // on engine N's spliced output, mirroring the server-side // sequence. Each capture is fail-soft: a parse failure or an diff --git a/crates/quarto-core/src/stage/stages/engine_execution.rs b/crates/quarto-core/src/stage/stages/engine_execution.rs index 56badcfbf..e49df2235 100644 --- a/crates/quarto-core/src/stage/stages/engine_execution.rs +++ b/crates/quarto-core/src/stage/stages/engine_execution.rs @@ -342,11 +342,35 @@ impl PipelineStage for EngineExecutionStage { // engine capture(s); other observers ignore the kind. match serde_json::to_value(&result) { Ok(result_json) => { - let payload = serde_json::json!({ + // bd-qbhp2cvv: when the observer wants them (the + // preview capture recorder does; plain renders + // don't), embed the supporting files' *bytes* in + // the capture. This is the only moment they are + // guaranteed to exist — the hub records captures + // in a temp dir that is deleted right after. + let capture_files = if ctx.observer.wants_engine_capture_files() + && !result.supporting_files.is_empty() + { + path.parent().map_or_else(Vec::new, |doc_dir| { + crate::engine::capture_files::collect_capture_files( + ctx.runtime.as_ref(), + doc_dir, + &result.supporting_files, + ) + }) + } else { + Vec::new() + }; + let mut payload = serde_json::json!({ "engine_name": engine.name(), "input_qmd": qmd, "result": result_json, }); + if !capture_files.is_empty() + && let Ok(files_json) = serde_json::to_value(&capture_files) + { + payload["files"] = files_json; + } ctx.observer.on_auxiliary_data( self.name(), run_index, @@ -1629,6 +1653,7 @@ mod tests { }, "needs_postprocess": false, }), + files: Vec::new(), }; let mut registry = EngineRegistry::new(); @@ -2013,6 +2038,7 @@ mod tests { }, "needs_postprocess": false, }), + files: Vec::new(), }; let mut registry = EngineRegistry::new(); diff --git a/crates/quarto-core/tests/integration/project_resources.rs b/crates/quarto-core/tests/integration/project_resources.rs index f13b7143d..cb9b49547 100644 --- a/crates/quarto-core/tests/integration/project_resources.rs +++ b/crates/quarto-core/tests/integration/project_resources.rs @@ -656,6 +656,7 @@ mod orchestrator_engine_channel { }, "needs_postprocess": false, }), + files: Vec::new(), }; // Now run the *real* pipeline through ProjectPipeline::new diff --git a/crates/quarto-core/tests/integration/render_to_html_captures.rs b/crates/quarto-core/tests/integration/render_to_html_captures.rs index e29762fcb..cf0150ff1 100644 --- a/crates/quarto-core/tests/integration/render_to_html_captures.rs +++ b/crates/quarto-core/tests/integration/render_to_html_captures.rs @@ -87,6 +87,7 @@ fn marker_capture() -> EngineCapture { result: serde_json::json!({ "markdown": "::: {.cell}\n```{.markerlang .cell-code}\n1 + 1\n```\n\n::: {.cell-output .cell-output-stdout}\n```\nSPLICEMARKER_ZX9\n```\n:::\n:::\n" }), + files: Vec::new(), } } diff --git a/crates/quarto-core/tests/integration/replay_engine.rs b/crates/quarto-core/tests/integration/replay_engine.rs index 68898ec5a..6478a8067 100644 --- a/crates/quarto-core/tests/integration/replay_engine.rs +++ b/crates/quarto-core/tests/integration/replay_engine.rs @@ -158,6 +158,7 @@ fn replay_capture_in_options_overrides_engine_through_render_to_file() { }, "needs_postprocess": false, }), + files: Vec::new(), }; let options = RenderToFileOptions { @@ -218,6 +219,7 @@ fn replay_capture_miss_surfaces_as_render_error() { }, "needs_postprocess": false, }), + files: Vec::new(), }; let options = RenderToFileOptions { diff --git a/crates/quarto-hub-provider/Cargo.toml b/crates/quarto-hub-provider/Cargo.toml index 13b713e94..be894caa8 100644 --- a/crates/quarto-hub-provider/Cargo.toml +++ b/crates/quarto-hub-provider/Cargo.toml @@ -31,7 +31,6 @@ quarto-system-runtime = { workspace = true } quarto-trace = { workspace = true } # Gzip the capture JSON before storing it as a binary automerge doc (same wire # format q2-preview uses; see CAPTURE_MIME_TYPE in execute.rs). -flate2 = { workspace = true } # CBOR for the ephemeral exec channel. The browser's DocHandle.broadcast # CBOR-encodes payloads (cbor-x, useRecords:false → standard CBOR maps), so we # mirror it with ciborium for cross-language interop. @@ -58,6 +57,7 @@ thiserror = { workspace = true } tracing = { workspace = true } [dev-dependencies] +flate2 = { workspace = true } tokio = { version = "1", features = ["rt-multi-thread", "macros", "process", "io-util", "time", "net", "test-util"] } tokio-tungstenite = { version = "0.27.0", features = ["native-tls"] } diff --git a/crates/quarto-hub-provider/src/execute.rs b/crates/quarto-hub-provider/src/execute.rs index 210484a74..366b6df45 100644 --- a/crates/quarto-hub-provider/src/execute.rs +++ b/crates/quarto-hub-provider/src/execute.rs @@ -36,8 +36,6 @@ use std::str::FromStr as _; use std::sync::{Arc, Mutex}; use std::time::Duration; -use flate2::Compression; -use flate2::write::GzEncoder; use futures::StreamExt; use quarto_core::engine::EngineRegistry; use quarto_core::engine::preview_record::{compute_input_qmd, record_capture}; @@ -47,7 +45,6 @@ use quarto_hub::resource::create_binary_document; use quarto_system_runtime::{NativeRuntime, SystemRuntime}; use quarto_trace::EngineCapture; use samod::Repo; -use std::io::Write as _; use crate::ProviderError; use crate::consent::ConsentGate; @@ -432,19 +429,14 @@ fn is_safe_relative(rel_path: &str) -> bool { /// Gzip the captures to JSON and store them as a capture binary automerge doc. /// Mirror of `quarto-preview`'s `write_capture_doc` (kept in sync via the -/// shared [`CAPTURE_MIME_TYPE`] and JSON+gzip wire format). +/// shared [`CAPTURE_MIME_TYPE`] and the shared wire-format helper +/// `gzip_captures` — serialize + gzip + 10MB size warning, bd-qbhp2cvv). async fn write_capture_doc( repo: &Repo, captures: &[EngineCapture], ) -> Result { - let json = serde_json::to_vec(captures) - .map_err(|e| ProviderError::Protocol(format!("serialize captures: {e}")))?; - let mut enc = GzEncoder::new(Vec::new(), Compression::default()); - enc.write_all(&json) - .map_err(|e| ProviderError::Protocol(format!("gzip write: {e}")))?; - let gzipped = enc - .finish() - .map_err(|e| ProviderError::Protocol(format!("gzip finish: {e}")))?; + let gzipped = quarto_core::engine::capture_files::gzip_captures(captures) + .map_err(|e| ProviderError::Protocol(format!("serialize/gzip captures: {e}")))?; let doc = create_binary_document(&gzipped, CAPTURE_MIME_TYPE) .map_err(|e| ProviderError::Protocol(format!("binary doc: {e}")))?; let handle = repo diff --git a/crates/quarto-preview/src/cache.rs b/crates/quarto-preview/src/cache.rs index 938d27b35..93a82a6cc 100644 --- a/crates/quarto-preview/src/cache.rs +++ b/crates/quarto-preview/src/cache.rs @@ -215,6 +215,7 @@ mod tests { "includes": [], "needs_postprocess": false, }), + files: Vec::new(), } } diff --git a/crates/quarto-preview/src/capture_driver.rs b/crates/quarto-preview/src/capture_driver.rs index f4d9fb1a1..d4fcbf23c 100644 --- a/crates/quarto-preview/src/capture_driver.rs +++ b/crates/quarto-preview/src/capture_driver.rs @@ -21,7 +21,6 @@ //! so a project with five engine-bearing docs doesn't burst five //! concurrent engine invocations on startup. -use std::io::Write; use std::sync::Arc; use quarto_core::engine::EngineRegistry; @@ -327,9 +326,11 @@ async fn write_capture_doc( ctx: &Arc, captures: &[EngineCapture], ) -> Result { - let json = - serde_json::to_vec(captures).map_err(|e| RecordError::Serialize(format!("{}", e)))?; - let gzipped = gzip_bytes(&json).map_err(|e| RecordError::Gzip(format!("{}", e)))?; + // Shared wire format (serialize + gzip + 10MB size warning, + // bd-qbhp2cvv) — same helper as re_execute.rs and the + // hub-provider exec server. + let gzipped = quarto_core::engine::capture_files::gzip_captures(captures) + .map_err(|e| RecordError::Gzip(format!("{}", e)))?; let automerge_doc = create_binary_document(&gzipped, CAPTURE_MIME_TYPE) .map_err(|e| RecordError::CreateBinaryDoc(format!("{}", e)))?; @@ -343,12 +344,6 @@ async fn write_capture_doc( Ok(handle.document_id().to_string()) } -fn gzip_bytes(input: &[u8]) -> std::io::Result> { - let mut enc = flate2::write::GzEncoder::new(Vec::new(), flate2::Compression::default()); - enc.write_all(input)?; - enc.finish() -} - /// Errors the driver can surface. Each variant is a short message /// suitable for `tracing::warn!` — the calling loop catches the /// failure, logs, and moves on to the next file. We deliberately @@ -361,9 +356,7 @@ pub enum RecordError { DiscoverFailed(String), #[error("engine capture pipeline failed: {0}")] RecordFailed(String), - #[error("serializing capture to JSON failed: {0}")] - Serialize(String), - #[error("gzipping capture failed: {0}")] + #[error("serializing/gzipping capture failed: {0}")] Gzip(String), #[error("creating capture binary doc failed: {0}")] CreateBinaryDoc(String), diff --git a/crates/quarto-preview/src/re_execute.rs b/crates/quarto-preview/src/re_execute.rs index b08aa1a47..cd21e024c 100644 --- a/crates/quarto-preview/src/re_execute.rs +++ b/crates/quarto-preview/src/re_execute.rs @@ -18,7 +18,6 @@ //! server is bound to 127.0.0.1. use std::collections::HashSet; -use std::io::Write; use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex, OnceLock}; @@ -333,16 +332,15 @@ async fn perform_re_execute( /// Mirror of `capture_driver::write_capture_doc` (private there). /// Duplicated here to avoid widening that module's public surface /// for the C.5-only handler. Both call sites stay in sync via the -/// shared `CAPTURE_MIME_TYPE` constant. +/// shared `CAPTURE_MIME_TYPE` constant and the shared wire-format +/// helper `gzip_captures` (serialize + gzip + 10MB size warning, +/// bd-qbhp2cvv). async fn write_capture_doc( ctx: &Arc, captures: &[EngineCapture], ) -> Result { - let json = serde_json::to_vec(captures).map_err(|e| format!("serialize: {e}"))?; - let mut enc = flate2::write::GzEncoder::new(Vec::new(), flate2::Compression::default()); - enc.write_all(&json) - .map_err(|e| format!("gzip write: {e}"))?; - let gzipped = enc.finish().map_err(|e| format!("gzip finish: {e}"))?; + let gzipped = quarto_core::engine::capture_files::gzip_captures(captures) + .map_err(|e| format!("serialize/gzip: {e}"))?; let doc = create_binary_document(&gzipped, CAPTURE_MIME_TYPE) .map_err(|e| format!("binary doc: {e}"))?; let handle = ctx diff --git a/crates/quarto-trace/src/lib.rs b/crates/quarto-trace/src/lib.rs index dcee16aee..14d542ea9 100644 --- a/crates/quarto-trace/src/lib.rs +++ b/crates/quarto-trace/src/lib.rs @@ -201,6 +201,36 @@ pub struct EngineCapture { /// `quarto-core::engine::replay` deserializes via /// `serde_json::from_value`. pub result: serde_json::Value, + + /// Contents of engine-generated supporting files (bd-qbhp2cvv). + /// + /// `result.supporting_files` records *paths* only; those files + /// exist only on the machine (often only in the temp dir) where + /// the engine ran. For preview replay on another machine — or in + /// the browser WASM VFS — the bytes must travel with the capture. + /// Recording embeds them here; `CaptureSpliceStage` materializes + /// them next to the document before splicing. + /// + /// Empty for captures that predate this field (`serde(default)`) + /// and for engines that produced no supporting files; such + /// captures serialize without the key, byte-identical to the + /// pre-field wire format. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub files: Vec, +} + +/// One engine-generated supporting file embedded in an +/// [`EngineCapture`] (bd-qbhp2cvv). +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct CaptureFile { + /// Path relative to the source document's directory, always with + /// forward-slash separators (e.g. + /// `"doc_files/figure-html/cell-1.png"`) so captures recorded on + /// one platform replay on any other. + pub path: String, + + /// Base64-encoded (standard alphabet, padded) file contents. + pub contents_base64: String, } /// Top-level metadata about a render invocation. diff --git a/crates/quarto-trace/tests/roundtrip.rs b/crates/quarto-trace/tests/roundtrip.rs index a483759c9..e63860aa2 100644 --- a/crates/quarto-trace/tests/roundtrip.rs +++ b/crates/quarto-trace/tests/roundtrip.rs @@ -352,6 +352,7 @@ fn test_engine_capture_roundtrip_through_disk() { engine_name: "jupyter".into(), input_qmd: "---\nengine: jupyter\n---\n\n# Hello\n".into(), result: result_json.clone(), + files: Vec::new(), }]; write_trace(&doc, &path).unwrap(); @@ -784,3 +785,57 @@ fn test_v2_dedup_actually_shrinks_repeated_asts() { let _ = std::fs::remove_dir_all(&tmp); } + +// ── bd-qbhp2cvv: EngineCapture.files wire-format compatibility ───────── + +/// A capture JSON that predates the `files` field must deserialize +/// with an empty `files` vec (serde default). +#[test] +fn test_engine_capture_without_files_field_deserializes() { + let old_wire = serde_json::json!({ + "engine_name": "knitr", + "input_qmd": "```{r}\n1\n```\n", + "result": {"markdown": "output", "supporting_files": []}, + }); + let capture: EngineCapture = serde_json::from_value(old_wire).unwrap(); + assert!(capture.files.is_empty()); +} + +/// A capture with no files must serialize WITHOUT the `files` key — +/// byte-identical wire shape to the pre-field format, so existing +/// snapshots and hash-keyed caches are unaffected. +#[test] +fn test_engine_capture_empty_files_serializes_without_key() { + let capture = EngineCapture { + engine_name: "knitr".into(), + input_qmd: "```{r}\n1\n```\n".into(), + result: serde_json::json!({"markdown": "output"}), + files: Vec::new(), + }; + let value = serde_json::to_value(&capture).unwrap(); + assert!( + value.get("files").is_none(), + "empty files must not appear on the wire; got: {value}" + ); +} + +/// Files round-trip: path + base64 contents survive serialize → +/// deserialize unchanged. +#[test] +fn test_engine_capture_files_roundtrip() { + use quarto_trace::CaptureFile; + let capture = EngineCapture { + engine_name: "knitr".into(), + input_qmd: "```{r}\nplot(1)\n```\n".into(), + result: serde_json::json!({"markdown": "![](doc_files/figure-html/f.png)"}), + files: vec![CaptureFile { + path: "doc_files/figure-html/f.png".into(), + contents_base64: "iVBORw0KGgo=".into(), + }], + }; + let json = serde_json::to_vec(&capture).unwrap(); + let back: EngineCapture = serde_json::from_slice(&json).unwrap(); + assert_eq!(back.files.len(), 1); + assert_eq!(back.files[0].path, "doc_files/figure-html/f.png"); + assert_eq!(back.files[0].contents_base64, "iVBORw0KGgo="); +} diff --git a/crates/wasm-quarto-hub-client/Cargo.lock b/crates/wasm-quarto-hub-client/Cargo.lock index e2f814d70..b8a393149 100644 --- a/crates/wasm-quarto-hub-client/Cargo.lock +++ b/crates/wasm-quarto-hub-client/Cargo.lock @@ -2142,6 +2142,7 @@ dependencies = [ "anyhow", "async-trait", "base64", + "flate2", "glob", "hashlink", "hex",