From 3e3994dabb0e4a0992a40c89972ed09df92cb56e Mon Sep 17 00:00:00 2001 From: Gordon Woodhull Date: Thu, 3 Sep 2026 15:50:28 -0400 Subject: [PATCH 1/4] Close the julia oneShot worker on failed runs (GH #649) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Running the julia tests stranded a QuartoNotebookRunner worker process per failed render, accumulating indefinitely. `executeJulia` in the fixture's engine sent the oneShot post-run close only on the SUCCESS path, so a run that threw — a cell raising under the default `error: false` — skipped the close and left that file's worker open on the control server forever, since nothing ever runs that (temp-dir) file again to reclaim it. Open workers also hold the server past its `serve(; timeout = 300)` idle exit, so the server never exited either: it accumulated children and was reparented to init. Measured before the fix: one orphaned control server (ppid=1, up 14h29m) holding 35 workers and 6.02 GB RSS. `j4` alone reproduced +1 leaked worker per invocation, deterministically over three runs; `j1` (success path) leaked none. Note this refines the diagnosis in #649: the defect is in the engine's TypeScript, not in QuartoNotebookRunner. QNR behaves correctly — it was never told to close the worker. The fix (taken from the upstream-bound `q2-close-busy-fix` branch of quarto-julia-engine, not yet in PumasAI main) wraps the run so the error path closes the worker best-effort via a new `errorRunClose`: plain close first, busy -> forceclose, and any remaining failure warns rather than throwing, so a cleanup failure cannot mask the run error already in flight. New regression row `j7_failed_run_does_not_leak_worker` runs against an ISOLATED control server under a temp HOME — seeded from the ambient runtime dir so the Julia environment is already instantiated — making the assertion an exact `workers == 0` on a server the test owns rather than a racy delta on the shared server. A guard kills that server's process group on scope exit. Verified RED before the fix ("leaked 1 QNR worker process") and GREEN after; the full julia suite now strands zero processes where it previously added one. --- .../_extensions/julia-engine/julia-engine.js | 67 ++++++-- .../julia-engine/src/julia-engine.ts | 52 ++++-- .../julia-engine/src/worker-close.ts | 38 ++++ .../tests/integration/julia_engine_e2e.rs | 162 ++++++++++++++++++ 4 files changed, 285 insertions(+), 34 deletions(-) diff --git a/crates/quarto-core/tests/fixtures/extensions/julia-engine/_extensions/julia-engine/julia-engine.js b/crates/quarto-core/tests/fixtures/extensions/julia-engine/_extensions/julia-engine/julia-engine.js index c6970dea7..ab4dbef45 100644 --- a/crates/quarto-core/tests/fixtures/extensions/julia-engine/_extensions/julia-engine/julia-engine.js +++ b/crates/quarto-core/tests/fixtures/extensions/julia-engine/_extensions/julia-engine/julia-engine.js @@ -668,6 +668,33 @@ async function postRunClose(writeCommand, file, warn) { ${message}`); } } +async function errorRunClose(writeCommand, file, warn) { + try { + try { + await writeCommand({ + type: "close", + content: { + file + } + }); + } catch (e) { + if (isWorkerBusyError(e)) { + await writeCommand({ + type: "forceclose", + content: { + file + } + }); + } else { + throw e; + } + } + } catch (e) { + const message = e instanceof Error ? e.message : String(e); + warn(`Julia worker close after a failed run also failed; the worker may be leaked. +${message}`); + } +} // src/constants.ts var kJuliaEngine = "julia"; @@ -1138,23 +1165,31 @@ async function executeJulia(options) { await preRunClose(writeCloseCommand, file); } const sourceRanges = buildSourceRanges(options.target.markdown); - const response = await writeJuliaCommand(conn, { - type: "run", - content: { - file, - options, - sourceRanges + let response; + try { + response = await writeJuliaCommand(conn, { + type: "run", + content: { + file, + options, + sourceRanges + } + }, transportOptions.key, options, (update) => { + const n = update.nChunks.toString(); + const i = update.chunkIndex.toString(); + const i_padded = `${" ".repeat(n.length - i.length)}${i}`; + const ncols = getConsoleColumns() ?? 80; + const firstPart = `Running [${i_padded}/${n}] at line ${update.line}: `; + const firstPartLength = firstPart.length; + const sigLine = firstSignificantLine(update.source, Math.max(0, ncols - firstPartLength)); + quarto.console.info(`${firstPart}${sigLine}`); + }); + } catch (e) { + if (options.oneShot) { + await errorRunClose(writeCloseCommand, file, (message) => quarto.console.warning(message)); } - }, transportOptions.key, options, (update) => { - const n = update.nChunks.toString(); - const i = update.chunkIndex.toString(); - const i_padded = `${" ".repeat(n.length - i.length)}${i}`; - const ncols = getConsoleColumns() ?? 80; - const firstPart = `Running [${i_padded}/${n}] at line ${update.line}: `; - const firstPartLength = firstPart.length; - const sigLine = firstSignificantLine(update.source, Math.max(0, ncols - firstPartLength)); - quarto.console.info(`${firstPart}${sigLine}`); - }); + throw e; + } if (options.oneShot) { await postRunClose(writeCloseCommand, file, (message) => quarto.console.warning(message)); } diff --git a/crates/quarto-core/tests/fixtures/extensions/julia-engine/src/julia-engine.ts b/crates/quarto-core/tests/fixtures/extensions/julia-engine/src/julia-engine.ts index 57fca4c61..9d053b001 100644 --- a/crates/quarto-core/tests/fixtures/extensions/julia-engine/src/julia-engine.ts +++ b/crates/quarto-core/tests/fixtures/extensions/julia-engine/src/julia-engine.ts @@ -30,6 +30,7 @@ import type { // unit-testable independent of the socket) import { type CloseCommandWriter, + errorRunClose, postRunClose, preRunClose, } from "./worker-close.ts"; @@ -722,25 +723,40 @@ async function executeJulia( const sourceRanges = buildSourceRanges(options.target.markdown); - const response = await writeJuliaCommand( - conn, - { type: "run", content: { file, options, sourceRanges } }, - transportOptions.key, - options, - (update: ProgressUpdate) => { - const n = update.nChunks.toString(); - const i = update.chunkIndex.toString(); - const i_padded = `${" ".repeat(n.length - i.length)}${i}`; - const ncols = getConsoleColumns() ?? 80; - const firstPart = `Running [${i_padded}/${n}] at line ${update.line}: `; - const firstPartLength = firstPart.length; - const sigLine = firstSignificantLine( - update.source, - Math.max(0, ncols - firstPartLength), + let response; + try { + response = await writeJuliaCommand( + conn, + { type: "run", content: { file, options, sourceRanges } }, + transportOptions.key, + options, + (update: ProgressUpdate) => { + const n = update.nChunks.toString(); + const i = update.chunkIndex.toString(); + const i_padded = `${" ".repeat(n.length - i.length)}${i}`; + const ncols = getConsoleColumns() ?? 80; + const firstPart = `Running [${i_padded}/${n}] at line ${update.line}: `; + const firstPartLength = firstPart.length; + const sigLine = firstSignificantLine( + update.source, + Math.max(0, ncols - firstPartLength), + ); + quarto.console.info(`${firstPart}${sigLine}`); + }, + ); + } catch (e) { + // A failed run must still close the oneShot worker, or it leaks on the + // shared control server (and blocks the server's idle timeout). Best + // effort — the run error rethrown below is the diagnostic that matters. + if (options.oneShot) { + await errorRunClose( + writeCloseCommand, + file, + (message) => quarto.console.warning(message), ); - quarto.console.info(`${firstPart}${sigLine}`); - }, - ); + } + throw e; + } if (options.oneShot) { await postRunClose( diff --git a/crates/quarto-core/tests/fixtures/extensions/julia-engine/src/worker-close.ts b/crates/quarto-core/tests/fixtures/extensions/julia-engine/src/worker-close.ts index 34cc7cd3b..4103b207a 100644 --- a/crates/quarto-core/tests/fixtures/extensions/julia-engine/src/worker-close.ts +++ b/crates/quarto-core/tests/fixtures/extensions/julia-engine/src/worker-close.ts @@ -69,3 +69,41 @@ export async function postRunClose( ); } } + +// Error-path close (julia-engine.ts oneShot cleanup when the run FAILED). +// Without this, the throw skips the post-run close and the file's worker +// leaks on the shared control server — forever, since nothing ever runs that +// file again (and open workers also block the server's idle timeout). +// +// Semantics: best-effort, never throws. +// - Plain close first: the common failure is QNR reporting a cell error +// in-band, after which the worker is idle and healthy. +// - Busy → forceclose: a transport-level failure can leave the worker still +// running; we are abandoning it either way, and reclaiming it now prevents +// creating the abandoned-busy state preRunClose has to recover from later. +// - Any remaining failure warns instead of throwing — deliberate asymmetry +// with preRunClose's forceclose-propagates contract, because here a real +// diagnostic (the run error) is already in flight and a cleanup failure +// must not mask it. +export async function errorRunClose( + writeCommand: CloseCommandWriter, + file: string, + warn: (message: string) => void, +): Promise { + try { + try { + await writeCommand({ type: "close", content: { file } }); + } catch (e) { + if (isWorkerBusyError(e)) { + await writeCommand({ type: "forceclose", content: { file } }); + } else { + throw e; + } + } + } catch (e) { + const message = e instanceof Error ? e.message : String(e); + warn( + `Julia worker close after a failed run also failed; the worker may be leaked.\n${message}`, + ); + } +} diff --git a/crates/quarto-core/tests/integration/julia_engine_e2e.rs b/crates/quarto-core/tests/integration/julia_engine_e2e.rs index cfc023c4e..a1521c8aa 100644 --- a/crates/quarto-core/tests/integration/julia_engine_e2e.rs +++ b/crates/quarto-core/tests/integration/julia_engine_e2e.rs @@ -1263,3 +1263,165 @@ fn list_dir_recursive(dir: &Path) -> String { out.sort(); out.join("\n") } + +// ── J7: a FAILED run must not leak the file's QNR worker ────────────────────── + +/// Seed an isolated temp `HOME`'s julia runtime dir from the ambient one, so the +/// isolated control server starts against an ALREADY-INSTANTIATED environment +/// (only the small `Project.toml`/`Manifest.toml` are copied; the packages +/// themselves live in the shared `JULIA_DEPOT_PATH`, which this never touches). +/// Without this the isolated server would re-resolve the environment from +/// scratch on every run. +/// +/// Returns `false` when the ambient runtime dir is not instantiated (no +/// `Manifest.toml`) — the caller should skip. +fn seed_isolated_julia_runtime(real_home: &Path, tmp_home: &Path) -> bool { + let src = real_home.join("Library/Caches/quarto/julia"); + if !src.join("Manifest.toml").exists() { + return false; + } + let dst = tmp_home.join("Library/Caches/quarto/julia"); + std::fs::create_dir_all(&dst).unwrap(); + for name in ["Project.toml", "Manifest.toml"] { + std::fs::copy(src.join(name), dst.join(name)).unwrap(); + } + true +} + +/// PIDs of a process's direct children. Against the QNR control server's pid +/// these are exactly its per-notebook worker processes. +fn child_pids(pid: i32) -> Vec { + Command::new("pgrep") + .arg("-P") + .arg(pid.to_string()) + .output() + .ok() + .map(|o| { + String::from_utf8_lossy(&o.stdout) + .lines() + .filter_map(|l| l.trim().parse::().ok()) + .collect() + }) + .unwrap_or_default() +} + +// Regression row for the oneShot worker-process leak. `executeJulia` sent the +// post-run close only on the SUCCESS path, so a render whose Julia cell raised +// (the J4 error doc) threw out of `writeJuliaCommand` and skipped the close, +// leaving that file's worker open on the control server FOREVER — nothing ever +// runs that (temp-dir) file again to reclaim it, and an open worker also holds +// the server past its `serve(; timeout = 300)` idle exit, so the server never +// exits either. Observed live on a dev machine: 30 leaked workers under a single +// orphaned server after a day of runs, with `j4` alone reproducing +1 worker per +// invocation (deterministic). +// +// This row runs against an ISOLATED control server under a temp `HOME` (seeded +// above so the Julia environment is already instantiated), so the assertion is +// an exact `workers == 0` on a server this test owns — not a racy delta on the +// user's shared server. The guard kills that server's process group on scope +// exit, so the row cannot itself strand anything. +// +// Named revert: drop the `catch` that calls `errorRunClose` around the `run` +// command in the fixture's `julia-engine.ts`/`julia-engine.js` (restore the bare +// `await writeJuliaCommand(...)`) → the failed render leaks its worker → the +// `workers.is_empty()` assertion reddens. +#[test] +fn j7_failed_run_does_not_leak_worker() { + if !cfg!(unix) { + eprintln!("SKIP: j7 worker-count cleanup requires unix process groups"); + return; + } + if !deno_available() { + eprintln!("SKIP: deno not on PATH — j7_failed_run_does_not_leak_worker"); + return; + } + if !julia_available() { + eprintln!("SKIP: julia not on PATH — j7_failed_run_does_not_leak_worker"); + return; + } + let Some(real_home) = std::env::var("HOME").ok().map(PathBuf::from) else { + eprintln!("SKIP: HOME unset — j7_failed_run_does_not_leak_worker"); + return; + }; + + // Proves at the end that the isolation never touched the user's real server. + let shared_sentinel = SharedTransportSentinel::capture(&real_home); + + let home = TempDir::new().unwrap(); + if !seed_isolated_julia_runtime(&real_home, home.path()) { + eprintln!( + "SKIP: ambient julia runtime dir has no Manifest.toml (not instantiated) — \ + j7_failed_run_does_not_leak_worker" + ); + return; + } + // SAFETY/scope: process-local under nextest (one process per test). + unsafe { + std::env::set_var("HOME", home.path()); + } + + let transport = home + .path() + .join("Library/Caches/quarto/julia/julia_transport.txt"); + // Drops LAST (after `home`/`tmp`): kills the isolated server + its worker + // group even if an assertion below panics. + let _server_guard = IsolatedJuliaServerGuard { + transport: transport.clone(), + }; + + let tmp = setup_julia_project(); + let error_input = tmp.path().join("error.qmd"); + write_file(&error_input, ERROR_DOC); + + let runtime: Arc = Arc::new(NativeRuntime::new()); + let options = RenderToFileOptions::default(); + let project = ProjectContext::discover(&error_input, runtime.as_ref()) + .expect("project discovery for the j7 error project"); + + let result = render_document_to_file( + &error_input, + "html", + &options, + Some(&project), + runtime.clone(), + None, + None, + None, + ); + let err = result.expect_err("error doc must fail the render, not succeed"); + assert!( + err.to_string().contains("this should fail gracefully"), + "j7 precondition: the render must fail with the Julia error text \ + (otherwise this row is not exercising the error path); got: {err}" + ); + + // The failed render still started the isolated control server. + let server_pid = transport_pid(&transport).unwrap_or_else(|| { + panic!( + "j7 precondition: no isolated julia control server pid in {transport:?} \ + — the render never started a server, so there is no worker to assert on" + ) + }); + + // The close is sent before the render error surfaces, but the worker + // process exits asynchronously — poll rather than guess a sleep. + let mut workers = child_pids(server_pid); + let mut waited = 0; + while !workers.is_empty() && waited < 30 { + std::thread::sleep(std::time::Duration::from_secs(1)); + waited += 1; + workers = child_pids(server_pid); + } + + assert!( + workers.is_empty(), + "failed render leaked {} QNR worker process(es) {:?} on control server {} \ + — the oneShot close must also run on the error path, or every failed \ + render strands a worker (and keeps the server alive past its idle timeout)", + workers.len(), + workers, + server_pid + ); + + shared_sentinel.assert_untouched(); +} From f35ef6b91da19814382dd2812cea17ab6edc0e0e Mon Sep 17 00:00:00 2001 From: Gordon Woodhull Date: Thu, 3 Sep 2026 15:52:31 -0400 Subject: [PATCH 2/4] Refresh the julia-engine fixture to upstream v0.2.1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-on to the #649 leak fix, which took only the error-path close. This brings the rest of the fixture up to upstream main, so the fixture stops straddling two upstream states. All of it comes from PumasAI main at the v0.2.1 tag: - QuartoNotebookRunner 0.17.4 -> 0.18.2 - writeAll for the socket payload (landed upstream as PR #12), fixing large documents failing with a bare "Internal Error" (quarto-cli#14834). `conn.write` is not guaranteed to write the whole buffer: a payload exceeding the socket send buffer short-writes, and the old code turned that into the bare error. The fixture carried that buggy form. Unrelated to the worker leak, and never observed here — our julia fixtures are a few hundred bytes. - keep-ipynb support - extension version 0.1.0 -> 0.2.1 Two q2-local deviations are preserved deliberately: - `_extension.yml` keeps its `author` and the `name`/`claims`/ `file-extensions` engine block. Those static declarations are q2-only schema with no upstream equivalent — Quarto 1's `external-engine` schema is `closed: true` and rejects them. - `start_quartonotebookrunner_detached.jl` keeps the q2-specific comment wording; its code lines are byte-identical to upstream. Separated from the leak fix so #649 can be reviewed — or reverted — on its own: this commit is droppable without affecting the fix. No assertion churn from the QNR bump; the julia rows also got roughly 3x faster, as 0.18.x caches worker environments across runs. --- .../_extensions/julia-engine/Project.toml | 2 +- .../_extensions/julia-engine/_extension.yml | 2 +- .../_extensions/julia-engine/julia-engine.js | 19 +++++++++++++++---- .../extensions/julia-engine/src/constants.ts | 1 + .../julia-engine/src/julia-engine.ts | 15 +++++++++++---- .../julia-engine/src/worker-close.ts | 15 ++++++++------- 6 files changed, 37 insertions(+), 17 deletions(-) diff --git a/crates/quarto-core/tests/fixtures/extensions/julia-engine/_extensions/julia-engine/Project.toml b/crates/quarto-core/tests/fixtures/extensions/julia-engine/_extensions/julia-engine/Project.toml index 94def5bfc..dbb2263da 100644 --- a/crates/quarto-core/tests/fixtures/extensions/julia-engine/_extensions/julia-engine/Project.toml +++ b/crates/quarto-core/tests/fixtures/extensions/julia-engine/_extensions/julia-engine/Project.toml @@ -2,4 +2,4 @@ QuartoNotebookRunner = "4c0109c6-14e9-4c88-93f0-2b974d3468f4" [compat] -QuartoNotebookRunner = "=0.17.4" +QuartoNotebookRunner = "=0.18.2" diff --git a/crates/quarto-core/tests/fixtures/extensions/julia-engine/_extensions/julia-engine/_extension.yml b/crates/quarto-core/tests/fixtures/extensions/julia-engine/_extensions/julia-engine/_extension.yml index 3e73aa600..0cc7a219c 100644 --- a/crates/quarto-core/tests/fixtures/extensions/julia-engine/_extensions/julia-engine/_extension.yml +++ b/crates/quarto-core/tests/fixtures/extensions/julia-engine/_extensions/julia-engine/_extension.yml @@ -1,6 +1,6 @@ title: Quarto Julia Engine Extension author: Quarto Julia Engine -version: 0.1.0 +version: 0.2.1 quarto-required: ">=1.9.0" contributes: engines: diff --git a/crates/quarto-core/tests/fixtures/extensions/julia-engine/_extensions/julia-engine/julia-engine.js b/crates/quarto-core/tests/fixtures/extensions/julia-engine/_extensions/julia-engine/julia-engine.js index ab4dbef45..61efd09bd 100644 --- a/crates/quarto-core/tests/fixtures/extensions/julia-engine/_extensions/julia-engine/julia-engine.js +++ b/crates/quarto-core/tests/fixtures/extensions/julia-engine/_extensions/julia-engine/julia-engine.js @@ -621,6 +621,14 @@ function encodeBase64(data) { return new TextDecoder().decode(output); } +// deno:https://jsr.io/@std/io/0.224.0/write_all.ts +async function writeAll(writer, data) { + let nwritten = 0; + while (nwritten < data.length) { + nwritten += await writer.write(data.subarray(nwritten)); + } +} + // src/worker-close.ts function isWorkerBusyError(e) { return e instanceof Error && /worker is busy/i.test(e.message); @@ -706,6 +714,7 @@ var kFigFormat = "fig-format"; var kFigPos = "fig-pos"; var kIpynbProduceSourceNotebook = "produce-source-notebook"; var kKeepHidden = "keep-hidden"; +var kKeepIpynb = "keep-ipynb"; // src/julia-engine.ts var isWindows2 = Deno.build.os === "windows"; @@ -824,6 +833,11 @@ var juliaEngineDiscovery = { language: "julia" }; const assets = quarto.jupyter.assets(options.target.input, options.format.pandoc.to); + if (options.format.execute[kKeepIpynb]) { + const stem = options.target.source.replace(/\.[^.]+$/, ""); + const ipynbPath = stem + ".ipynb"; + Deno.writeTextFileSync(ipynbPath, JSON.stringify(nb, null, 2)); + } const result = await quarto.jupyter.toMarkdown(nb, { executeOptions: options, language: nb.metadata.kernelspec.language.toLowerCase(), @@ -1219,10 +1233,7 @@ async function writeJuliaCommand(conn, command, secret, options, onProgressUpdat }) + "\n"; const messageBytes = new TextEncoder().encode(message); trace(options, `write command "${command.type}" to socket server`); - const bytesWritten = await conn.write(messageBytes); - if (bytesWritten !== messageBytes.length) { - throw new Error("Internal Error"); - } + await writeAll(conn, messageBytes); let restOfPreviousResponse = new Uint8Array(512); let restLength = 0; while (true) { diff --git a/crates/quarto-core/tests/fixtures/extensions/julia-engine/src/constants.ts b/crates/quarto-core/tests/fixtures/extensions/julia-engine/src/constants.ts index 1664bf2c2..fe57bb665 100644 --- a/crates/quarto-core/tests/fixtures/extensions/julia-engine/src/constants.ts +++ b/crates/quarto-core/tests/fixtures/extensions/julia-engine/src/constants.ts @@ -13,3 +13,4 @@ export const kFigFormat = "fig-format"; export const kFigPos = "fig-pos"; export const kIpynbProduceSourceNotebook = "produce-source-notebook"; export const kKeepHidden = "keep-hidden"; +export const kKeepIpynb = "keep-ipynb"; diff --git a/crates/quarto-core/tests/fixtures/extensions/julia-engine/src/julia-engine.ts b/crates/quarto-core/tests/fixtures/extensions/julia-engine/src/julia-engine.ts index 9d053b001..b217468cc 100644 --- a/crates/quarto-core/tests/fixtures/extensions/julia-engine/src/julia-engine.ts +++ b/crates/quarto-core/tests/fixtures/extensions/julia-engine/src/julia-engine.ts @@ -8,6 +8,7 @@ import { dirname, fromFileUrl, join, resolve } from "path"; import { existsSync } from "fs/exists"; import { encodeBase64 } from "encoding/base64"; +import { writeAll } from "jsr:@std/io@0.224.0/write-all"; // Type imports from Quarto via import map import type { @@ -46,6 +47,7 @@ import { kIpynbProduceSourceNotebook, kJuliaEngine, kKeepHidden, + kKeepIpynb, } from "./constants.ts"; // Platform detection @@ -229,6 +231,14 @@ export const juliaEngineDiscovery: ExecutionEngineDiscovery = { options.format.pandoc.to, ); + // Write notebook to file if keep-ipynb is set (must happen before + // toMarkdown which mutates nb in place) + if (options.format.execute[kKeepIpynb]) { + const stem = options.target.source.replace(/\.[^.]+$/, ""); + const ipynbPath = stem + ".ipynb"; + Deno.writeTextFileSync(ipynbPath, JSON.stringify(nb, null, 2)); + } + // NOTE: for perforance reasons the 'nb' is mutated in place // by jupyterToMarkdown (we don't want to make a copy of a // potentially very large notebook) so should not be relied @@ -850,10 +860,7 @@ async function writeJuliaCommand( const messageBytes = new TextEncoder().encode(message); trace(options, `write command "${command.type}" to socket server`); - const bytesWritten = await conn.write(messageBytes); - if (bytesWritten !== messageBytes.length) { - throw new Error("Internal Error"); - } + await writeAll(conn, messageBytes); // a string of bytes received from the server could start with a // partial message, contain multiple complete messages (separated by newlines) after that diff --git a/crates/quarto-core/tests/fixtures/extensions/julia-engine/src/worker-close.ts b/crates/quarto-core/tests/fixtures/extensions/julia-engine/src/worker-close.ts index 4103b207a..289744e6f 100644 --- a/crates/quarto-core/tests/fixtures/extensions/julia-engine/src/worker-close.ts +++ b/crates/quarto-core/tests/fixtures/extensions/julia-engine/src/worker-close.ts @@ -16,7 +16,7 @@ export type CloseCommandWriter = ( ) => Promise; // A close fails with "worker is busy" when the file's worker is still running -// (QNR does not interrupt a running task on a plain close). See task-p0-report.md. +// (QNR does not interrupt a running task on a plain close). export function isWorkerBusyError(e: unknown): boolean { return e instanceof Error && /worker is busy/i.test(e.message); } @@ -24,13 +24,14 @@ export function isWorkerBusyError(e: unknown): boolean { // Pre-run close (julia-engine.ts oneShot / daemon-restart path). If the file's // worker is busy we recover with a forceclose rather than surfacing the bare // protocol error: the busy worker is an ABANDONED one (a prior client vanished -// mid-run and left the shared server's worker orphaned — the Bug A scenario), -// and forceclose reclaims the file so this fresh render can proceed. +// mid-run and left the shared server's worker orphaned), and forceclose +// reclaims the file so this fresh render can proceed. // -// CAVEAT (documented for the upstream PR, deliberately not special-cased here): -// a worker busy serving a *live concurrent* render on a shared server would also -// be force-closed, killing legitimate work. Distinguishing abandoned-vs-live is -// the deeper oneShot-server-reuse design question — see the q2 compat log. +// CAVEAT (deliberately documented rather than special-cased): a worker busy +// serving a *live concurrent* render on a shared server would also be +// force-closed, killing legitimate work. Distinguishing abandoned-vs-live +// workers is part of the wider question of whether a oneShot render should +// reuse a daemon-started server at all. export async function preRunClose( writeCommand: CloseCommandWriter, file: string, From d2fe5ddd41b8b3c2061fd05d9da905feda4dd42f Mon Sep 17 00:00:00 2001 From: Gordon Woodhull Date: Thu, 3 Sep 2026 15:30:22 -0400 Subject: [PATCH 3/4] Draft the julia-engine static-declarations epic (preliminary) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Research write-up off the back of the worker-leak fix. Records the findings that shape the sequencing — chiefly that Quarto 1's external-engine schema is closed:true and rejects all four static keys (verified empirically against the dev build), so the declarations cannot land upstream without a quarto-cli schema change first. Also records that q2 deliberately inverts Q1's jupyter/julia default, so the schema change should be accept-and-ignore rather than implemented in Q1; that file-extensions is not a transcription of validExtensions(); and a confirmed q2 bug where static claim lookup is case-sensitive while dynamic claiming is not. Draft only: not scoped into tasks, not started, six open questions. --- ...3-julia-engine-static-declarations-epic.md | 240 ++++++++++++++++++ 1 file changed, 240 insertions(+) create mode 100644 claude-notes/plans/2026-09-03-julia-engine-static-declarations-epic.md diff --git a/claude-notes/plans/2026-09-03-julia-engine-static-declarations-epic.md b/claude-notes/plans/2026-09-03-julia-engine-static-declarations-epic.md new file mode 100644 index 000000000..0fb082565 --- /dev/null +++ b/claude-notes/plans/2026-09-03-julia-engine-static-declarations-epic.md @@ -0,0 +1,240 @@ +# Julia engine: upstream fixes + static engine declarations (epic, DRAFT) + +**Status:** preliminary draft — research done, not scoped into tasks, not started. +**Author context:** written 2026-09-03 off the back of the worker-leak +investigation (`0f243f64c` on `julia-orphan-triage`). Nothing here is committed +to upstream yet. + +## Overview + +Two things want to go upstream to `PumasAI/quarto-julia-engine`, and they have +very different risk profiles: + +1. **The worker-leak bug fix** (plus two sibling fixes) — pure bug fixing, + Q1-compatible, no schema implications. +2. **The static engine declarations** in `_extension.yml` (`name:`, `claims:`, + `file-extensions:`) that let Quarto 2 resolve the engine in **pass 1 without + loading it**. These are *rejected by Quarto 1 today* and cannot land alone. + +The second is blocked on a `quarto-cli` schema change, which is why this is an +epic rather than a PR. + +## Key research findings + +### F1 — Q1's `external-engine` schema is closed; all four keys are rejected + +`src/resources/schema/definitions.yml:295-304` (both `external-sources/quarto-cli` +and the dev checkout `~/src/quarto-cli`): + +```yaml +- id: external-engine + schema: + object: + closed: true # <-- no additional properties + properties: + path: { path: { description: ... } } + required: [path] +``` + +Verified empirically against the dev build (`quarto` 99.9.9), one key at a time: + +| key | result | +|---|---| +| `path` only | ACCEPTED | +| `name` | REJECTED | +| `claims` | REJECTED | +| `file-extensions` | REJECTED | +| `claims-files` | REJECTED | + +Failure is at `readExtension` → `readAndValidateYamlFromFile` — the extension +fails to **load**, so it is a hard render error, not a warning. Shipping the +declarations upstream without the schema change would break the extension for +every Q1 user. + +### F2 — q2 deliberately inverts the Q1 jupyter/julia default + +Q1 (`execute/jupyter/jupyter.ts`): jupyter's `claimsLanguage` returns true for +`julia`, with the comment *"jupyter has to claim julia so that julia may also +claim it without changing the old behavior of preferring jupyter over julia +engine by default."* So **in Q1, jupyter wins `{julia}` by default.** + +q2 (`engine/jupyter/mod.rs:168-173`): jupyter is a universal `Fallback(0)`, and +the comment states the Julia extension's `Primary(1)` wins when installed (kind +dominates priority), with `{julia}` still reaching jupyter via the T4 fallback +tier when the extension is absent. + +**Consequence for the schema PR:** if Q1 ever *honors* `claims:`, it would flip +its own jupyter/julia default. The schema change should therefore be +**accept-and-ignore** (pure forward-compatibility for Quarto 2), explicitly +*not* new Q1 behavior. This needs to be stated in the PR description or a +reviewer will reasonably read it as a behavior change. + +### F3 — `file-extensions: [.jl]` is not a transcription of `validExtensions()` + +The engine's `validExtensions: () => []`. In Q1 that is correct because +`validExtensions` is a **global admission gate** (`execute/engine.ts:311-317`): +if *no* engine lists the extension the file is rejected before any `claimsFile` +runs — and **jupyter** lists `.jl` via `kJupyterPercentScriptExtensions`, so the +gate passes and julia's `claimsFile` is still consulted. + +In q2, `file-extensions` is a **per-engine can-handle pre-filter**, so `.jl` +must be declared or q2 never asks julia about a `.jl` file. This matches +`claude-notes/designs/engine-resolution.md:131-134`, but it means the same key +name carries different semantics in the two systems. Needs an explicit note in +the PR, and ideally a description in the schema itself. + +### F4 — `claims-files` correctly omitted; `.jl` input is still not zero-load + +julia's `claimsFile` is `isPercentScript(file, [".jl"])` — a content sniff. q2's +`FileClaim` is `{ extension }` only; `content-pattern` is **not implemented** +(referenced in a `types.rs` comment; the plan exists at +`claude-notes/plans/2026-07-07-plan7a-static-content-pattern-claims.md`). + +So the pass-1 zero-load win currently covers **`{julia}` cells in `.qmd`**, not +`.jl` percent-script input — julia still loads to answer the file claim. +Finishing that half is Plan 7a, not this epic, but the epic should say so. + +### F5 — static claims lookup is case-sensitive; dynamic claiming is not (CONFIRMED) + +- Dynamic: `claimsLanguage: (language) => language.toLowerCase() === "julia"`. +- Static: `parse_claims_map` inserts `entry.key.clone()` with **no** lowercasing + (unlike `file-extensions`, which normalizes to undotted lowercase at parse), + and `lookup_static_claim` does an exact `claims.get(language)`. +- The language token is **never** normalized on the way in: `engine_cell_lang` + (`capture_splice.rs:86-100`) returns the `{lang}` class body verbatim, and + `walk_block_for_langs` stores it verbatim. No `to_lowercase` anywhere in + `resolution.rs`. + +So a `{Julia}` cell claims **dynamically** but not **statically** — the static +declaration is not the "complete replacement" §3.3 promises. This is a q2 bug, +independent of anything upstream. + +### F6 — upstream CI pins quarto-cli by full commit hash + +`.github/workflows/ci.yml`: + +```yaml +QUARTO_CLI_REPO: quarto-dev/quarto-cli +QUARTO_CLI_REV: 97e7649bf14607cf39cda13f013185a4146e047b # v1.9.35 +``` + +CI checks out that exact quarto-cli rev and runs `./configure.sh`. So the +static-declarations PR must **also bump `QUARTO_CLI_REV`** to a commit +containing the schema change, or its own CI fails. Easy to miss. + +Upstream CI also **verifies the bundled JS is up to date** with `src/`, and +`EnforceChangelog.yml` requires a CHANGELOG entry on every PR. + +### F7 — `quarto-required` is inert in q2 + +q2 parses `quarto-required` into `Extension` but never compares it +(`ts_engine.rs:2901` — "carrier (inert in 1c)"). So bumping it constrains **Q1 +users only**; q2 is unaffected either way. + +## Proposed sequence + +### Step 1 — quarto-cli: loosen the `external-engine` schema + +Add `name`, `claims`, `file-extensions`, `claims-files` to the `external-engine` +definition (or drop `closed: true`), documented as **accepted and ignored by +Quarto 1**, reserved for Quarto 2 resolution. + +Notes: +- `src/resources/schema/json-schemas.json` is a **generated** artifact + (`src/core/schema/json-schema-from-schema.ts:168`); the PR must include the + regenerated file. *Open: exact regeneration command — not yet confirmed.* +- Decide whether to type the keys properly (better errors, more review surface) + or accept them loosely. +- Land it, then get it into a **prerelease** so downstream can depend on it. + +### Step 2 — quarto-julia-engine: two PRs + +**(a) The bug fix.** Three commits currently on `q2-close-busy-fix` (rebased +onto upstream v0.2.1, bundle verified in sync): + +- `b881e69` redirect the detached server's stdio to devnull +- `f7c9bfc` recover from a busy/failed oneShot worker close +- `4e6bc27` close the oneShot worker when the run fails (the leak) + +Fully Q1-compatible; no schema dependency; can go **immediately**, independent of +everything else. Needs a CHANGELOG entry (enforced). + +*Open: one PR or three?* They are independently reviewable and the leak fix is +the one with hard evidence. + +**(b) The static declarations.** Adds the `name:`/`claims:`/`file-extensions:` +block, and must additionally: +- bump `quarto-required:` to the step-1 prerelease, +- bump `QUARTO_CLI_REV` in `ci.yml` to the schema-change commit, +- add a CHANGELOG entry, +- explain F2 (accept-and-ignore, not a Q1 behavior change) and F3 + (`file-extensions` ≠ `validExtensions`) in the PR body. + +**Risk:** requiring a *prerelease* in a published extension's +`quarto-required` is user-hostile — it would make the extension refuse to +install on stable Quarto. See open question Q2. + +### Step 3 — q2: fix the static-claim case sensitivity (F5) + +Independent of upstream; can land any time. Options: + +1. Lowercase claim keys at parse in `parse_claims_map` **and** lowercase the + language at lookup in `lookup_static_claim` (mirrors how `file-extensions` + already normalizes at parse). Preferred — normalizes both sides at the + boundary, consistent with existing precedent. +2. Normalize the language once, further upstream (at `engine_cell_lang` / + `walk_block_for_langs`). Wider blast radius: that token feeds more than claim + lookup. + +TDD: a failing test with a `{Julia}` cell against a static `claims: {julia: ...}` +registry, RED before the fix. + +### Step 4 — what else belongs (candidates, not yet decided) + +- **Fixture drift management.** The q2 fixture is a hand-maintained fork + (`claude-notes/plans/2026-04-16-julia-validation.md`). After this epic it + carries **two permanent deliberate deviations**: the `claims:` block (q2-only + schema) and the Bug C comments in + `start_quartonotebookrunner_detached.jl` (code byte-identical to upstream). + Nothing checks fixture-bundle ≡ fixture-TS (upstream CI does; q2 has no + equivalent), and nothing tracks fixture-vs-upstream drift. Candidate: a + documented refresh procedure, optionally an `xtask lint` rule. +- **Plan 7a (`content-pattern`)** to make `.jl` percent-script input zero-load + too (F4) — the other half of "pass-1 happy". +- **Long-term home for the declarations if upstream declines.** q2's fixture + stays a fork indefinitely; alternatively the design doc's author-side + document-level `engines: [{julia: {claims: ...}}]` table could carry them + without touching `_extension.yml`. Worth deciding *before* asking upstream. +- **`author:` field drift.** Fixture says `Quarto Julia Engine`; upstream has + none; a local stash adds `PumasAI`. Trivial, but pick one. +- **Housekeeping:** `~/src/quarto-julia-engine` has `stash@{0}` holding + `.gitignore` + a `q2-test-unknown-key: hello` probe (that probe's question is + now answered by F1 — unknown keys are rejected). Drop or apply. + +## Open questions + +- **Q1.** Should the quarto-cli schema change *type* the new keys (full + property schemas) or just stop being `closed`? Typing them documents the + contract and gives good errors, but invites the question "what does Quarto 1 + do with these?" — to which the answer is "nothing" (F2). +- **Q2.** Is requiring a prerelease in upstream's `quarto-required` acceptable + to PumasAI at all? If not, PR 2b probably has to **wait for a stable Quarto + release** carrying the schema change — which changes the epic's timeline from + weeks to a release cycle. This is the single biggest scheduling risk. +- **Q3.** Has any of this been discussed with PumasAI / jkrumbiegel? Adding + Quarto-2-only keys to *their* extension needs buy-in, and the answer changes + how PR 2b should be pitched. +- **Q4.** PR 2a: one PR or three? +- **Q5.** Does q2 want the fixture to track upstream mechanically (refresh + script + drift lint) or stay a hand-maintained fork? +- **Q6.** Is `{Julia}` (non-lowercase language) actually reachable in practice, + or is the F5 fix purely defensive? It is a real divergence either way, but it + affects priority. + +## Not doing / out of scope + +- Implementing `claims:` semantics **in Quarto 1** (F2 — it would flip Q1's + jupyter/julia default). +- Plan 7a itself (tracked separately). +- Any braid strands: this is plan-scoped work, so items live in this + checklist when the epic is actually scheduled. From 2da32b8c1d72968b865f90228f6c924fd782224f Mon Sep 17 00:00:00 2001 From: Gordon Woodhull Date: Thu, 3 Sep 2026 15:45:10 -0400 Subject: [PATCH 4/4] Revise the julia-engine epic: release-gated, q2 branch, bundling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three changes from the first draft. Target a full stable Quarto release rather than a prerelease: a published extension whose quarto-required names a prerelease would refuse to install for ordinary users on stable Quarto, which is not something to ask PumasAI to ship. This makes the critical path a release cycle rather than weeks. Add Step 0 (discuss with Julius before anything else — the q2-only keys and the quarto-required bump are changes to their extension, and the conversation may reshape the plan) and Step 2c (offer a q2 branch on PumasAI/quarto-julia-engine so people can try Julia in q2 before the schema change ships, and so q2 has a stable remote to subtree from). Add Step 4: bundle the julia engine in q2 as a vendored subtree. Research (F8) finds q2 already has the discovery and embedding machinery — builtin_extensions_path() embeds resources/extensions via include_dir! and discover_extensions already scans a builtin dir first — so the port is mostly the maintenance command, Q1's dev-call pull-git-subtree, rehomed as cargo xtask pull-extension-subtree. Payload is 68K but the full repo is 14M, so embed only the _extensions subdir. Strengthen the provisional banner: nothing here is agreed or started. --- ...3-julia-engine-static-declarations-epic.md | 208 +++++++++++++++--- 1 file changed, 180 insertions(+), 28 deletions(-) diff --git a/claude-notes/plans/2026-09-03-julia-engine-static-declarations-epic.md b/claude-notes/plans/2026-09-03-julia-engine-static-declarations-epic.md index 0fb082565..e2db1a8fa 100644 --- a/claude-notes/plans/2026-09-03-julia-engine-static-declarations-epic.md +++ b/claude-notes/plans/2026-09-03-julia-engine-static-declarations-epic.md @@ -1,4 +1,13 @@ -# Julia engine: upstream fixes + static engine declarations (epic, DRAFT) +# Julia engine: upstream fixes, static declarations, and bundling in q2 (epic, DRAFT) + +> ## ⚠️ PROVISIONAL — NEEDS REVIEW +> +> This is a **first-pass draft written from research, not from a decision**. The +> sequencing, the scope, and the assignment of work to PRs are all proposals and +> have **not been agreed with anyone** — including PumasAI, who own the +> extension. Several load-bearing steps depend on external parties and external +> release timelines. Nothing here should be treated as settled, and no part of +> it has been started. **Review before acting on any of it.** **Status:** preliminary draft — research done, not scoped into tasks, not started. **Author context:** written 2026-09-03 off the back of the worker-leak @@ -7,17 +16,21 @@ to upstream yet. ## Overview -Two things want to go upstream to `PumasAI/quarto-julia-engine`, and they have -very different risk profiles: +Three workstreams, in increasing order of risk and dependency: 1. **The worker-leak bug fix** (plus two sibling fixes) — pure bug fixing, Q1-compatible, no schema implications. 2. **The static engine declarations** in `_extension.yml` (`name:`, `claims:`, `file-extensions:`) that let Quarto 2 resolve the engine in **pass 1 without loading it**. These are *rejected by Quarto 1 today* and cannot land alone. +3. **Bundling the julia engine into q2** as a vendored subtree, so `q2` ships + with Julia support instead of requiring a separate extension install. This + needs the engine's q2-flavoured declarations to exist somewhere stable + first — i.e. it follows (2). -The second is blocked on a `quarto-cli` schema change, which is why this is an -epic rather than a PR. +(2) is blocked on a `quarto-cli` schema change **and on that change reaching a +stable Quarto release**, which is why this is an epic rather than a PR. (1) is +independent and can go immediately. ## Key research findings @@ -131,8 +144,83 @@ q2 parses `quarto-required` into `Extension` but never compares it (`ts_engine.rs:2901` — "carrier (inert in 1c)"). So bumping it constrains **Q1 users only**; q2 is unaffected either way. +### F8 — bundling: q2 already has the runtime hook; the gap is maintenance + +Quarto 1 vendors whole extension repos as **git subtrees** under +`src/resources/extension-subtrees//`, kept in sync by a hidden dev +command (`src/command/dev-call/pull-git-subtree/cmd.ts`). That command is a thin +wrapper over `git subtree add/pull --squash`, with a hard-coded `SUBTREES` table: + +```ts +{ name: "julia-engine", + prefix: "src/resources/extension-subtrees/julia-engine", + remoteUrl: "https://github.com/PumasAI/quarto-julia-engine.git", + remoteBranch: "main" } +``` + +It finds the last split via `git log --grep="git-subtree-dir: $"`, falls +back to `subtree add` when the prefix is new, and no-ops when there are no new +commits. Requires `QUARTO_ROOT`. Discovery side (`extension/extension.ts:680-695`): +extension lookup falls back to scanning +`resourcePath("extension-subtrees")/*/_extensions/`, a **separate root** +from `resourcePath("extensions")`. + +**q2 already has the equivalent runtime machinery.** `builtin_extensions_path()` +(`extension/mod.rs:43-80`) embeds `resources/extensions/` via `include_dir!`, +lazily extracts it to a temp dir through `ResourceBundle`, and has a WASM VFS +variant; `discover_extensions` takes a `builtin_extensions_dir` that is +**scanned first**, and it is already wired up from `project/mod.rs:2206` and +`stage/context.rs:298`. So there is **no discovery work to port** — an extension +dropped into the embedded bundle is found automatically. + +**Sizes matter for the layout decision:** + +| | size | +|---|---| +| `resources/extensions/` today (7 bundled extensions) | 712K | +| julia-engine `_extensions/` payload (what must ship) | **68K** | +| julia-engine **whole repo** (tests, `.github`, `src`, docs) | **14M** | + +Q1's subtree vendors the *entire* repo. Embedding that wholesale via +`include_dir!` would put 14M of tests and CI config into every `q2` binary. So +q2 should subtree the full repo into `resources/extension-subtrees/julia-engine/` +(git cost only, mirroring Q1) but point `include_dir!` at just its +`_extensions/` subdirectory — 68K in the binary. **Open: is 14M in q2's git +history acceptable, or should we vendor a curated copy instead and give up +`git subtree`'s merge tracking?** + +The genuinely new work is therefore the **maintenance command**, not the +runtime: port `pull-git-subtree` as `cargo xtask pull-extension-subtree`. xtask +is the right home — it already hosts every comparable maintenance/build task +(`build_agents_docs`, `build_hub_mcp_bundle`, `braid_snapshot`, …). + +**Synergy with the fixture-drift problem:** if the bundled copy is subtreed from +the **q2 branch** (which carries the static declarations), then the bundled +extension already has `claims:` — q2 gets pass-1 static resolution for Julia out +of the box, and the hand-maintained test fixture could potentially be replaced +by (or derived from) the bundled copy, retiring the fork-drift item in Step 4. + +*Not yet investigated:* `filterBundledSubtreeEngines` (`extension.ts:734`, used +at `render/pandoc.ts:446,1324`) strips bundled subtree engines out of the +metadata `engines` array handed to pandoc. Whether q2 needs an analogue depends +on how q2 surfaces `engines` in metadata — not traced. + ## Proposed sequence +### Step 0 — talk to Julius first + +Before any of the below: **discuss with Julius Krumbiegel / PumasAI.** Steps 1 +and 2b add Quarto-2-only surface to *their* extension and constrain *their* +`quarto-required`. The conversation should cover: + +- whether they are willing to carry q2-only keys at all (and if so, whether in + `main` or only on a branch), +- the `quarto-required` bump and its cost to their users (see Step 2b), +- the offer in Step 2c (a q2 branch), which may be the outcome they prefer. + +This gates 2b entirely and may change its shape. Do it early — it is cheap and +it is the step most likely to invalidate the rest of the plan. + ### Step 1 — quarto-cli: loosen the `external-engine` schema Add `name`, `claims`, `file-extensions`, `claims-files` to the `external-engine` @@ -145,34 +233,54 @@ Notes: regenerated file. *Open: exact regeneration command — not yet confirmed.* - Decide whether to type the keys properly (better errors, more review surface) or accept them loosely. -- Land it, then get it into a **prerelease** so downstream can depend on it. -### Step 2 — quarto-julia-engine: two PRs +**Then wait for a full, stable Quarto release carrying it.** *(Revised — an +earlier draft targeted a prerelease.)* A published extension whose +`quarto-required` names a prerelease would refuse to install for ordinary users +on stable Quarto, which is not something we should ask PumasAI to ship. This +makes the epic's critical path a **release cycle**, not weeks — which is exactly +why Step 2c exists. -**(a) The bug fix.** Three commits currently on `q2-close-busy-fix` (rebased -onto upstream v0.2.1, bundle verified in sync): +### Step 2 — quarto-julia-engine + +**(a) The bug fix — ready now, no dependencies.** Three commits currently on +`q2-close-busy-fix` (rebased onto upstream v0.2.1, bundle verified in sync): - `b881e69` redirect the detached server's stdio to devnull - `f7c9bfc` recover from a busy/failed oneShot worker close - `4e6bc27` close the oneShot worker when the run fails (the leak) -Fully Q1-compatible; no schema dependency; can go **immediately**, independent of -everything else. Needs a CHANGELOG entry (enforced). +Fully Q1-compatible; no schema dependency; independent of everything else above +and below. Needs a CHANGELOG entry (enforced by `EnforceChangelog.yml`). *Open: one PR or three?* They are independently reviewable and the leak fix is the one with hard evidence. -**(b) The static declarations.** Adds the `name:`/`claims:`/`file-extensions:` -block, and must additionally: -- bump `quarto-required:` to the step-1 prerelease, -- bump `QUARTO_CLI_REV` in `ci.yml` to the schema-change commit, +**(b) The static declarations — gated on Step 1 shipping in a stable release.** +Adds the `name:`/`claims:`/`file-extensions:` block, and must additionally: +- bump `quarto-required:` to the **stable** release from Step 1, +- bump `QUARTO_CLI_REV` in `ci.yml` to a commit containing the schema change, - add a CHANGELOG entry, - explain F2 (accept-and-ignore, not a Q1 behavior change) and F3 - (`file-extensions` ≠ `validExtensions`) in the PR body. - -**Risk:** requiring a *prerelease* in a published extension's -`quarto-required` is user-hostile — it would make the extension refuse to -install on stable Quarto. See open question Q2. + (`file-extensions` != `validExtensions`) in the PR body. + +**(c) Offer a q2 branch on `PumasAI/quarto-julia-engine` — the unblocker.** +Because 2b waits on a release cycle, offer to maintain a **branch** on the +upstream repo (name TBD, e.g. `q2`) carrying the static declarations, so people +who want to try **Julia in Quarto 2 before it ships** can point at it. This: + +- gives early adopters a real, upstream-hosted path with no prerelease + `quarto-required` and no fork of record, +- gives q2 a **stable remote to subtree from** for Step 5 (see F8) — the + bundled copy would track this branch, not `main`, +- keeps `main` clean and Q1-only until Step 1's schema change is stable, + which is likely what PumasAI would prefer anyway, +- lets us validate the declarations against real users before asking for them + in `main`. + +*Open: does the branch live on PumasAI (preferred — upstream-hosted, discoverable) +or on the `gordonwoodhull` fork (no permission needed)? This is part of the +Step 0 conversation.* ### Step 3 — q2: fix the static-claim case sensitivity (F5) @@ -189,7 +297,44 @@ Independent of upstream; can land any time. Options: TDD: a failing test with a `{Julia}` cell against a static `claims: {julia: ...}` registry, RED before the fix. -### Step 4 — what else belongs (candidates, not yet decided) +### Step 4 — q2: bundle the julia engine as a vendored subtree + +**Goal:** `q2` ships with Julia support built in — no separate extension +install. Gated on Step 2c (a stable branch to subtree from). See **F8** for the +research; the headline is that q2 **already has the discovery + embedding +machinery**, so this is mostly maintenance tooling, not a port. + +Work items (first pass, not scoped): + +- **Vendor the subtree.** `git subtree add --squash` the Step 2c branch into + `resources/extension-subtrees/julia-engine/`, mirroring Q1's layout. Decide + the git-history cost first (14M — see F8 open question). +- **Embed only the payload.** Point a second `include_dir!` at + `resources/extension-subtrees/julia-engine/_extensions` (68K), not at the + subtree root, and expose it through the existing `ResourceBundle` / + `builtin_extensions_path()` path. *Open: does `builtin_extensions_path` + return one dir (requiring the subtree payload to be merged into the existing + bundle) or should discovery accept a list of builtin roots, mirroring Q1's + two separate roots?* — this is the main design decision in Step 5. +- **Port the maintenance command** as `cargo xtask pull-extension-subtree` + (Q1: `src/command/dev-call/pull-git-subtree/cmd.ts`): a `SUBTREES` table, last-split + detection via `git log --grep="git-subtree-dir: $"`, `subtree add` + when the prefix is new, `subtree pull --squash` otherwise, no-op when there + are no new commits. Drop the `QUARTO_ROOT` env dependency — xtask already + knows the repo root. +- **Decide the fixture's future.** If the bundled copy carries the static + declarations, the hand-maintained fixture fork may be replaceable by (or + derivable from) the bundled copy — which would retire the drift item in + Step 5. Needs care: the julia e2e tests deliberately copy the fixture into a + temp project. +- **Runtime prerequisites.** Bundling ships the engine, not Julia itself: + QuartoNotebookRunner still instantiates on first use (network), and the + engine host still needs Deno. Worth an explicit UX decision about what + `q2` does on a machine with no Julia. +- *Not investigated:* whether q2 needs an analogue of + `filterBundledSubtreeEngines` (F8). + +### Step 5 — what else belongs (candidates, not yet decided) - **Fixture drift management.** The q2 fixture is a hand-maintained fork (`claude-notes/plans/2026-04-16-julia-validation.md`). After this epic it @@ -217,13 +362,20 @@ registry, RED before the fix. property schemas) or just stop being `closed`? Typing them documents the contract and gives good errors, but invites the question "what does Quarto 1 do with these?" — to which the answer is "nothing" (F2). -- **Q2.** Is requiring a prerelease in upstream's `quarto-required` acceptable - to PumasAI at all? If not, PR 2b probably has to **wait for a stable Quarto - release** carrying the schema change — which changes the epic's timeline from - weeks to a release cycle. This is the single biggest scheduling risk. -- **Q3.** Has any of this been discussed with PumasAI / jkrumbiegel? Adding - Quarto-2-only keys to *their* extension needs buy-in, and the answer changes - how PR 2b should be pitched. +- **Q2.** *(Resolved in this revision — target a stable release, not a + prerelease.)* Remaining: how long is that cycle, and does it change what we + do in the meantime beyond Step 2c? +- **Q3.** Step 0: what does Julius say? Everything in Step 2 is contingent on + it. Specifically: q2-only keys in `main` or only on a branch; who hosts the + Step 2c branch; and are they willing to bump `quarto-required` at all. +- **Q7.** Step 4 layout: one builtin-extensions root (merge the subtree payload + into the existing embedded bundle) or teach discovery a **list** of builtin + roots (mirroring Q1's separate `extensions` / `extension-subtrees` roots)? +- **Q8.** Is 14M of vendored repo acceptable in q2's git history for the sake of + `git subtree`'s merge tracking, or do we vendor a curated 68K copy and accept + manual syncing? +- **Q9.** Once Julia is bundled, what is the story on a machine without Julia + installed — silent fallback to jupyter, or a diagnostic? - **Q4.** PR 2a: one PR or three? - **Q5.** Does q2 want the fixture to track upstream mechanically (refresh script + drift lint) or stay a hand-maintained fork?