From c2e11508d0f6e9d2115b9ee86689d39c8d7d54a8 Mon Sep 17 00:00:00 2001
From: Christophe Dervieux <christophe.dervieux@gmail.com>
Date: Tue, 22 Sep 2026 15:13:39 +0200
Subject: [PATCH 1/5] Isolate a failing project pre-render's tests instead of
 aborting smoke-all

tests/smoke/smoke-all.test.ts pre-renders a project (for files annotated
render-project: true) in a bare, unguarded top-level await. A failing
pre-render throws uncaught at module-evaluation time. Deno reports this
loudly (exit 1, named error, real stack trace), but every Deno.test()
that would have been registered after the throw - the rest of the glob,
and everything after the loop - is silently never created and never
counted. The summary reads "N passed | 1 failed" with no indication that
M tests never ran at all. This is pre-existing and mode-independent (dev
and binary mode both throw identically for an ordinary render failure);
it is not a regression from binary-mode test support.

render-project is per-file front matter, not a project-level setting, so
a single-pass try/catch cannot isolate a failure: an unannotated sibling
file can already be registered before the annotated file triggers the
pre-render. Split the discovery loop into three passes instead - collect
every file first, pre-render each distinct project once, then register
tests - so a failed project's files can be skipped as a group (one named
synthetic failure per project, not a crash of the whole file) while every
other file, including ones after it in the file list, keeps running and
counting normally. A skipped file's would-be cleanup entry is synthesized
so its outputs are still removed at parity with a registered file.

A timeout is only safe to isolate if the render is known to have
stopped. Dev mode can never confirm this (no in-process cancellation), so
its timeouts stay fatal. Binary mode's process-tree kill previously
discarded its outcome entirely - the Windows path lost taskkill's exit
code in a local that never escaped, and the Unix path treated any
non-throwing pgrep call as an authoritative enumeration even though a
non-zero exit (pgrep's own usage/internal error codes) produces the same
empty output as a childless leaf. Made cancellation a total exit-code
partition with a fail-safe default, carried through a new
QuartoTimeoutError (renderCancelled, killDetail) instead of a bare string,
so an unconfirmed kill can no longer be mistaken for a confirmed one and
isolated anyway.

The new tests/unit/ regression test spawns a nested deno test of
smoke-all.test.ts against a generated fixture (an unannotated file
excluded from the project's render list, two annotated files so a
retry-per-annotation implementation can't pass trivially, and a trailing
healthy file) and asserts on its JUnit report: exactly one synthetic
failure, zero tests for any file of the broken project, and the trailing
file still running - the exact coverage silently lost today.
---
 tests/quarto-cmd.ts                           | 222 ++++++++++++--
 tests/smoke/smoke-all.test.ts                 | 117 +++++++-
 .../smoke-all-prerender-isolation.test.ts     | 279 ++++++++++++++++++
 3 files changed, 571 insertions(+), 47 deletions(-)
 create mode 100644 tests/unit/smoke-all-prerender-isolation.test.ts

diff --git a/tests/quarto-cmd.ts b/tests/quarto-cmd.ts
index 62e7d784344..74e80961318 100644
--- a/tests/quarto-cmd.ts
+++ b/tests/quarto-cmd.ts
@@ -183,60 +183,140 @@ export function assertTestBinary(bin: string) {
   checkedBinary = bin;
 }
 
+// Outcome of a process-tree kill attempt. `cancelled` is true only when
+// every signalled node was positively enumerated: any spawn throw, a
+// non-zero/unparseable pgrep result, a non-zero taskkill exit, or a
+// Deno.kill failure other than NotFound leaves it false. This fail-safe
+// default is the point: an implementer who forgets a branch gets "fatal",
+// not "silently continue beside a possible orphan".
+interface KillOutcome {
+  cancelled: boolean;
+  detail: string;
+}
+
 // The launcher waits on Deno, so kill the process tree deepest first.
-async function killProcessTree(pid: number) {
+async function killProcessTree(pid: number): Promise<KillOutcome> {
   if (isWindows) {
-    let killed = false;
     try {
-      // taskkill reports failure through its exit code.
+      // taskkill reports failure through its exit code; outputSync-style
+      // Deno.Command does not throw on a non-zero exit, only on a spawn
+      // failure, so the exit code is the only reliable signal here.
       const result = await new Deno.Command("taskkill", {
         args: ["/PID", String(pid), "/T", "/F"],
         stdout: "null",
-        stderr: "null",
+        stderr: "piped",
       }).output();
-      killed = result.code === 0;
-    } catch {
-      // Fall through to a direct kill.
-    }
-    if (!killed) {
-      // Ensure child.output() can resolve even if the tree kill failed.
+      if (result.code === 0) {
+        return {
+          cancelled: true,
+          detail: `taskkill /T /F pid ${pid} exited 0`,
+        };
+      }
+      // taskkill did not confirm the tree. Still attempt to unblock
+      // child.output() by killing the launcher directly, but this reaches
+      // only the launcher, not its descendants, so the outcome stays
+      // unconfirmed regardless of whether this fallback succeeds.
+      try {
+        Deno.kill(pid, "SIGKILL");
+      } catch {
+        // already exited
+      }
+      const stderrText = new TextDecoder().decode(result.stderr).trim();
+      return {
+        cancelled: false,
+        detail: `taskkill /T /F pid ${pid} exited ${result.code}: ${stderrText}`,
+      };
+    } catch (e) {
       try {
         Deno.kill(pid, "SIGKILL");
       } catch {
         // already exited
       }
+      return {
+        cancelled: false,
+        detail: `taskkill /T /F pid ${pid} failed to spawn: ${String(e)}`,
+      };
     }
-    return;
   }
+
+  // Unix: enumerate with pgrep -P (Linux and macOS/BSD), classifying each
+  // invocation by exit code rather than by whether it threw -- outputSync()
+  // does not throw on a non-zero exit. Exit 0 with parseable stdout or
+  // exit 1 (a genuine childless leaf) are the only enumerated outcomes; a
+  // spawn throw, any other exit code, or unparseable stdout forces the
+  // aggregate `cancelled` to false without aborting the walk (still signal
+  // every pid already collected).
   const pids: number[] = [];
   const stack = [pid];
+  let confirmed = true;
+  let unconfirmedDetail = "";
   while (stack.length > 0) {
     const current = stack.pop()!;
     pids.push(current);
+
+    let spawnFailed = false;
+    let code = -1;
+    let stdoutText = "";
+    let stderrText = "";
     try {
-      // pgrep -P works on Linux and macOS/BSD.
       const result = new Deno.Command("pgrep", {
         args: ["-P", String(current)],
         stdout: "piped",
-        stderr: "null",
+        stderr: "piped",
       }).outputSync();
-      const children = new TextDecoder()
-        .decode(result.stdout)
-        .split("\n")
-        .map((line) => parseInt(line.trim(), 10))
-        .filter((child) => !isNaN(child));
-      stack.push(...children);
-    } catch {
-      // pgrep unavailable; fall back to killing what we have
+      code = result.code;
+      stdoutText = new TextDecoder().decode(result.stdout);
+      stderrText = new TextDecoder().decode(result.stderr);
+    } catch (e) {
+      spawnFailed = true;
+      stderrText = String(e);
+    }
+
+    if (spawnFailed) {
+      confirmed = false;
+      unconfirmedDetail = `pgrep -P ${current} failed to spawn: ${stderrText}`;
+      continue;
+    }
+    if (code === 1) {
+      // No children matched: a genuine leaf, nothing to push.
+      continue;
+    }
+    if (code !== 0) {
+      // Usage (2) or internal (3) error: enumeration unreliable.
+      confirmed = false;
+      unconfirmedDetail = `pgrep -P ${current} exited ${code}: ${stderrText.trim()}`;
+      continue;
+    }
+    const lines = stdoutText.split("\n").map((line) => line.trim()).filter((
+      line,
+    ) => line.length > 0);
+    const children = lines.map((line) => parseInt(line, 10));
+    if (children.some((child) => isNaN(child))) {
+      confirmed = false;
+      unconfirmedDetail =
+        `pgrep -P ${current} produced unparseable output: ${stdoutText.trim()}`;
     }
+    stack.push(...children.filter((child) => !isNaN(child)));
   }
+
+  let signalled = 0;
   for (const target of pids.reverse()) {
     try {
       Deno.kill(target, "SIGKILL");
-    } catch {
-      // already exited
+      signalled++;
+    } catch (e) {
+      if (!(e instanceof Deno.errors.NotFound)) {
+        confirmed = false;
+        unconfirmedDetail = `kill pid ${target} failed: ${String(e)}`;
+      }
     }
   }
+  return {
+    cancelled: confirmed,
+    detail: confirmed
+      ? `signalled ${signalled} pid(s) from root ${pid}`
+      : (unconfirmedDetail || `could not confirm process tree from root ${pid}`),
+  };
 }
 
 export interface RunQuartoOptions {
@@ -262,6 +342,30 @@ export interface RunQuartoResult {
   stderrTail?: string;
 }
 
+// A render timeout, discriminated from a plain failure so a caller can tell
+// whether the render is known to have been stopped.
+export class QuartoTimeoutError extends Error {
+  constructor(
+    message: string,
+    readonly timeoutMs: number,
+    // True only when the render is known to have been stopped. Dev mode
+    // races a timer against an in-process render it cannot cancel, so it is
+    // always false there; binary mode sets it from the process-tree kill
+    // outcome.
+    readonly renderCancelled: boolean,
+    // Platform detail of the kill attempt, including the child pid.
+    // Undefined in dev mode, which attempts no kill.
+    readonly killDetail?: string,
+  ) {
+    super(message);
+    this.name = "QuartoTimeoutError";
+  }
+}
+
+export function isQuartoTimeoutError(e: unknown): e is QuartoTimeoutError {
+  return e instanceof QuartoTimeoutError;
+}
+
 // Dispatch to the in-process dev sources or the configured built binary.
 export async function runQuarto(
   args: string[],
@@ -281,7 +385,17 @@ async function runDevQuarto(
   const timeoutMs = options.timeoutMs ?? kDefaultRenderTimeoutMs;
   let timer: ReturnType<typeof setTimeout> | undefined;
   const timeout = new Promise<never>((_resolve, reject) => {
-    timer = setTimeout(reject, timeoutMs, `timed out after ${timeoutMs}ms`);
+    timer = setTimeout(
+      reject,
+      timeoutMs,
+      new QuartoTimeoutError(
+        `timed out after ${timeoutMs}ms`,
+        timeoutMs,
+        // Dev mode never attempts a kill: it can never cancel an
+        // in-process render, only lose the race below.
+        false,
+      ),
+    );
   });
   try {
     await Promise.race([quarto(args, undefined, options.env), timeout]);
@@ -290,9 +404,10 @@ async function runDevQuarto(
       clearTimeout(timer);
     }
   }
-  // quarto() either resolves or rejects: on CommandError or commandFailed()
-  // it calls exitWithCleanup(1), which Deno.exits the whole test process
-  // before this function could return a failure code anyway.
+  // quarto() calls exitWithCleanup(1), which Deno.exits the whole test
+  // process, only for a CommandError (Cliffy argument parsing) or
+  // commandFailed() (set only by the add/remove commands, never render).
+  // A render failure is neither, so it rejects out of quarto() instead.
   return { timedOut: false };
 }
 
@@ -335,16 +450,34 @@ async function runBinaryQuarto(
   }).spawn();
 
   let timedOut = false;
+  let killPromise: Promise<KillOutcome> | undefined;
   const timer = setTimeout(() => {
     timedOut = true;
-    // child.output() resolves after the kill; avoid an unhandled rejection.
-    killProcessTree(child.pid).catch(() => {});
+    killPromise = killProcessTree(child.pid);
   }, timeoutMs);
 
   // Drain both streams to avoid pipe-buffer deadlocks.
   const output = await child.output();
   clearTimeout(timer);
 
+  // The direct child is known dead at this point (child.output() above
+  // already resolved), but the deno/pandoc descendants it spawned are only
+  // known dead if killOutcome.cancelled is true.
+  let killOutcome: KillOutcome = {
+    cancelled: false,
+    detail: "no timeout occurred",
+  };
+  if (timedOut && killPromise !== undefined) {
+    try {
+      killOutcome = await killPromise;
+    } catch (e) {
+      killOutcome = {
+        cancelled: false,
+        detail: `killProcessTree rejected: ${String(e)}`,
+      };
+    }
+  }
+
   const stderrText = new TextDecoder().decode(output.stderr);
   const stderrTail = stderrText.split("\n").slice(-25).join("\n").trim();
   const commandLine = `quarto ${args.join(" ")}`;
@@ -359,11 +492,36 @@ async function runBinaryQuarto(
     });
   }
 
+  // throwOnFailure: false callers (e.g. testQuartoCmd) get the one
+  // diagnostic for an unconfirmed kill; throwing callers get no log here
+  // (see the QuartoTimeoutError branch below) since a confirmed kill on
+  // this path has no orphan to warn about.
+  if (timedOut && !killOutcome.cancelled && !throwOnFailure) {
+    console.error(
+      `[binary mode] process-tree kill UNCONFIRMED: ${commandLine} ` +
+        `(pid ${child.pid}) timed out after ${timeoutMs}ms; ${killOutcome.detail}. ` +
+        `Descendant quarto/pandoc processes may still be running and writing, ` +
+        `and the caller is continuing anyway, so later failures in this run ` +
+        `may be corruption from the orphan rather than genuine.`,
+    );
+  }
+
   if ((output.code !== 0 || timedOut) && throwOnFailure) {
+    if (timedOut) {
+      const base =
+        `${commandLine} (pid ${child.pid}) timed out after ${timeoutMs}ms`;
+      const message = killOutcome.cancelled
+        ? base
+        : `${base}; process-tree kill UNCONFIRMED: ${killOutcome.detail}`;
+      throw new QuartoTimeoutError(
+        message,
+        timeoutMs,
+        killOutcome.cancelled,
+        killOutcome.detail,
+      );
+    }
     throw new Error(
-      timedOut
-        ? `${commandLine} timed out after ${timeoutMs}ms`
-        : `${commandLine} exited with code ${output.code}\nstderr (tail):\n${stderrTail}`,
+      `${commandLine} exited with code ${output.code}\nstderr (tail):\n${stderrTail}`,
     );
   }
 
diff --git a/tests/smoke/smoke-all.test.ts b/tests/smoke/smoke-all.test.ts
index 2082afa844c..7121ec91a56 100644
--- a/tests/smoke/smoke-all.test.ts
+++ b/tests/smoke/smoke-all.test.ts
@@ -5,7 +5,7 @@
  */
 
 import { expandGlobSync } from "../../src/core/deno/expand-glob.ts";
-import { testQuartoCmd, Verify } from "../test.ts";
+import { testQuartoCmd, unitTest, Verify } from "../test.ts";
 import { initYamlIntelligenceResourcesFromFilesystem } from "../../src/core/schema/utils.ts";
 import {
   initState,
@@ -57,7 +57,7 @@ import { findProjectDir, findProjectOutputDir, outputForInput } from "../utils.t
 import { jupyterNotebookToMarkdown } from "../../src/command/convert/jupyter.ts";
 import { basename, dirname, join, relative } from "../../src/deno_ral/path.ts";
 import { WalkEntry } from "../../src/deno_ral/fs.ts";
-import { runQuarto } from "../quarto-cmd.ts";
+import { isQuartoTimeoutError, runQuarto } from "../quarto-cmd.ts";
 import { safeExistsSync, safeRemoveSync } from "../../src/core/path.ts";
 import { runningInCI } from "../../src/core/ci-info.ts";
 
@@ -412,9 +412,27 @@ const projectFilePromises: Map<string, Promise<void>[]> = new Map();
 // Create an array to hold all the promises for the tests of files
 let testFilesPromises = [];
 
+// Records, keyed by project path, of a project whose pre-render failed
+// (populated in the pre-render pass below).
+const failedProjectPreRenders: Map<string, Error> = new Map();
+
+interface DiscoveredFile {
+  input: string;
+  // deno-lint-ignore no-explicit-any
+  metadata: Record<string, any>;
+  testSpecs: QuartoInlineTestSpec[];
+  projectPath: string | undefined;
+}
+
+// Pass 1 (discovery, no registration). render-project is per-file front
+// matter, not a project-level setting, so a project can mix annotated and
+// unannotated files with the unannotated ones sorting first -- collecting
+// every file before registering (or pre-rendering) any of them is what
+// makes "skip every file of a failed project" achievable at all.
+const discovered: DiscoveredFile[] = [];
 for (const { path: fileName } of files) {
   const input = relative(Deno.cwd(), fileName);
-  
+
   const metadata = input.endsWith("md") // qmd or md
     ? readYamlFromMarkdown(Deno.readTextFileSync(input))
     : readYamlFromMarkdown(await jupyterNotebookToMarkdown(input, false));
@@ -444,24 +462,93 @@ for (const { path: fileName } of files) {
   const projectPath = findRootTestsProjectDir(input);
   if (projectPath) testedProjects.add(projectPath);
 
-  // Render project before testing individual document if required
-  if (
-    (metadata["_quarto"] as any)?.["render-project"] && 
-    projectPath && 
-    !renderedProjects.has(projectPath)
-  ) {
-      // fail-loudly pre-render (throwOnFailure defaults to true);
-      // dispatches to the built binary when QUARTO_TEST_BIN is set
-      await runQuarto(["render", projectPath]);
-      renderedProjects.add(projectPath);
+  discovered.push({ input, metadata, testSpecs, projectPath });
+}
+
+// Pass 1.5 (pre-render). One attempt per distinct project that any
+// collected file marks render-project, ahead of any registration below.
+const projectsNeedingPreRender = new Set<string>();
+for (const entry of discovered) {
+  if ((entry.metadata["_quarto"] as any)?.["render-project"] && entry.projectPath) {
+    projectsNeedingPreRender.add(entry.projectPath);
+  }
+}
+for (const projectPath of projectsNeedingPreRender) {
+  try {
+    // dispatches to the built binary when QUARTO_TEST_BIN is set; a
+    // failure here isolates this project's files (see
+    // failedProjectPreRenders below) rather than aborting the whole file.
+    await runQuarto(["render", projectPath]);
+    renderedProjects.add(projectPath);
+  } catch (err) {
+    // A timeout whose render was not confirmably stopped is fatal: an
+    // uncancelled dev-mode render, or a binary-mode process tree that
+    // could not be confirmed killed, could keep writing into this
+    // project's directory while the rest of the suite -- and the cleanup
+    // block below -- run alongside it.
+    if (isQuartoTimeoutError(err) && !err.renderCancelled) {
+      console.error(
+        `[smoke-all] project pre-render for ${projectPath} timed out after ` +
+          `${err.timeoutMs}ms and the render could not be confirmed stopped` +
+          (err.killDetail ? ` (${err.killDetail})` : "") +
+          `; aborting rather than let a still-running render race the rest ` +
+          `of the suite.`,
+      );
+      throw err;
     }
+    failedProjectPreRenders.set(
+      projectPath,
+      err instanceof Error ? err : new Error(String(err)),
+    );
+  }
+}
+
+// One synthetic failing test per failed project (not per skipped file):
+// one clearly named failure pointing at the cause, rather than N red lines
+// for one root cause.
+for (const [projectPath, error] of failedProjectPreRenders) {
+  unitTest(`smoke-all project pre-render failed: ${projectPath}`, async () => {
+    throw error;
+  });
+}
+
+// Pass 2 (registration).
+for (const entry of discovered) {
+  const { input, metadata, testSpecs, projectPath } = entry;
+
+  if (projectPath && failedProjectPreRenders.has(projectPath)) {
+    console.log(
+      `Skipping tests for ${input}: its project's pre-render failed (${projectPath})`,
+    );
+    // Mirror exactly what this file's own teardown would have pushed to
+    // projectCleanupEntries, so a skipped file lands at exact parity with
+    // a registered one. Only specs that would take the normal `render`
+    // branch push an entry -- the editor-support-crossref branch's
+    // teardown pushes none (it only removes its own temp file), and
+    // synthesizing one would reduce through parseFormatString to base
+    // editor and delete an unrelated html support directory.
+    for (const testSpec of testSpecs) {
+      if (testSpec.format === "editor-support-crossref") {
+        continue;
+      }
+      if (!projectCleanupEntries.has(projectPath)) {
+        projectCleanupEntries.set(projectPath, []);
+      }
+      projectCleanupEntries.get(projectPath)!.push({
+        input,
+        format: testSpec.format,
+        metadata,
+      });
+    }
+    continue;
+  }
 
   const fileTestsPromise = new Promise<void>(async (resolve, reject) => {
     try {
 
       // Create an array to hold all the promises for the testSpecs
       let testSpecPromises = [];
-      
+
       for (const testSpec of testSpecs) {
         const {
           format,
@@ -509,7 +596,7 @@ for (const { path: fileName } of files) {
             testSpecReject(error);
           }
         }));
-          
+
       }
 
       // Wait for all the promises to resolve
diff --git a/tests/unit/smoke-all-prerender-isolation.test.ts b/tests/unit/smoke-all-prerender-isolation.test.ts
new file mode 100644
index 00000000000..e9371939237
--- /dev/null
+++ b/tests/unit/smoke-all-prerender-isolation.test.ts
@@ -0,0 +1,279 @@
+/*
+ * smoke-all-prerender-isolation.test.ts
+ *
+ * Copyright (C) 2026 Posit Software, PBC
+ */
+import { assert, assertEquals } from "testing/asserts";
+import { unitTest } from "../test.ts";
+import { dirname, fromFileUrl, join } from "../../src/deno_ral/path.ts";
+import { safeExistsSync } from "../../src/core/path.ts";
+
+// tests/unit/ -> tests/ -> repo root
+function resolveQuartoRoot(): string {
+  const envRoot = Deno.env.get("QUARTO_ROOT");
+  if (envRoot && safeExistsSync(join(envRoot, "src", "import_map.json"))) {
+    return envRoot;
+  }
+  const here = dirname(fromFileUrl(import.meta.url));
+  const derived = dirname(dirname(here));
+  if (safeExistsSync(join(derived, "src", "import_map.json"))) {
+    return derived;
+  }
+  throw new Error(
+    "Could not resolve the Quarto repo root: QUARTO_ROOT is unset or invalid, " +
+      `and the derived root ${derived} has no src/import_map.json.`,
+  );
+}
+
+interface JUnitTestcase {
+  name: string;
+  failed: boolean;
+}
+
+function parseJUnit(
+  xml: string,
+): { total: number; failures: number; cases: JUnitTestcase[] } {
+  const suiteMatch = xml.match(
+    /<testsuites[^>]*\btests="(\d+)"[^>]*\bfailures="(\d+)"/,
+  );
+  const cases: JUnitTestcase[] = [];
+  for (
+    const m of xml.matchAll(/<testcase\b([^>]*)>([\s\S]*?)<\/testcase>/g)
+  ) {
+    const nameMatch = m[1].match(/name="([^"]*)"/);
+    cases.push({
+      name: nameMatch ? nameMatch[1] : "",
+      failed: /<failure\b/.test(m[2]),
+    });
+  }
+  return {
+    total: suiteMatch ? parseInt(suiteMatch[1], 10) : cases.length,
+    failures: suiteMatch
+      ? parseInt(suiteMatch[2], 10)
+      : cases.filter((c) => c.failed).length,
+    cases,
+  };
+}
+
+unitTest(
+  "smoke-all isolates a failing project pre-render instead of aborting the whole file",
+  async () => {
+    const quartoRoot = resolveQuartoRoot();
+    const testsDir = join(quartoRoot, "tests");
+
+    const tmpRoot = Deno.makeTempDirSync({
+      prefix: "quarto-smoke-all-prerender-",
+    });
+    try {
+      const smokeAllDir = join(tmpRoot, "smoke-all");
+      const projectDir = join(smokeAllDir, "_prerender-crash");
+      Deno.mkdirSync(projectDir, { recursive: true });
+
+      Deno.writeTextFileSync(
+        join(projectDir, "_quarto.yml"),
+        "project:\n  type: default\n  render:\n" +
+          "    - broken-b.qmd\n    - broken-c.qmd\n",
+      );
+
+      // Unannotated, excluded from project.render (so the pre-render never
+      // visits it), and pinned to html so the cleanup entry synthesized for
+      // a skipped file resolves to exactly broken-a.html / broken-a_files.
+      // Sorts first alphabetically, so a one-pass implementation would
+      // register it before the pre-render below ever runs.
+      Deno.writeTextFileSync(
+        join(projectDir, "broken-a.qmd"),
+        [
+          "---",
+          "title: a",
+          "_quarto:",
+          "  tests:",
+          "    html:",
+          "      ensureHtmlElements:",
+          '        - ["body"]',
+          "---",
+          "",
+          "# a",
+          "",
+        ].join("\n"),
+      );
+      // Sentinels, pre-created so their post-run absence is attributable
+      // only to the cleanup entry synthesized for the skipped file, never
+      // to render ordering: the pre-render's project.render list excludes
+      // broken-a.qmd, so nothing else in the run can touch these paths.
+      Deno.writeTextFileSync(
+        join(projectDir, "broken-a.html"),
+        "<html><body>sentinel</body></html>\n",
+      );
+      const supportDir = join(projectDir, "broken-a_files");
+      Deno.mkdirSync(supportDir, { recursive: true });
+      Deno.writeTextFileSync(
+        join(supportDir, "sentinel.txt"),
+        "sentinel\n",
+      );
+
+      // Annotated: the file whose render fails the project pre-render.
+      Deno.writeTextFileSync(
+        join(projectDir, "broken-b.qmd"),
+        [
+          "---",
+          "title: b",
+          "_quarto:",
+          "  render-project: true",
+          "filters:",
+          "  - does-not-exist.lua",
+          "---",
+          "",
+          "# b",
+          "",
+        ].join("\n"),
+      );
+      // A second annotated file: with only one, "exactly one synthetic
+      // failure" would be trivially satisfied by an implementation that
+      // retries the pre-render once per annotation.
+      Deno.writeTextFileSync(
+        join(projectDir, "broken-c.qmd"),
+        [
+          "---",
+          "title: c",
+          "_quarto:",
+          "  render-project: true",
+          "---",
+          "",
+          "# c",
+          "",
+        ].join("\n"),
+      );
+
+      // Coverage-loss canary: must still be registered and run despite
+      // sorting after the broken project in the explicit file-argument list.
+      Deno.writeTextFileSync(
+        join(smokeAllDir, "healthy-canary.qmd"),
+        ["---", "title: healthy", "---", "", "# healthy", ""].join("\n"),
+      );
+
+      assert(
+        safeExistsSync(join(projectDir, "broken-a.html")),
+        "sentinel broken-a.html must exist before the child run",
+      );
+      assert(
+        safeExistsSync(join(supportDir, "sentinel.txt")),
+        "sentinel broken-a_files/sentinel.txt must exist before the child run",
+      );
+
+      const junitPath = join(tmpRoot, "report.xml");
+      const importMapArg = `--importmap=${
+        join(quartoRoot, "src", "import_map.json")
+      }`;
+
+      const command = new Deno.Command(Deno.execPath(), {
+        args: [
+          "test",
+          "--config",
+          join(testsDir, "test-conf.json"),
+          "--v8-flags=--enable-experimental-regexp-engine",
+          "--unstable-kv",
+          "--unstable-ffi",
+          "--no-lock",
+          "--allow-all",
+          importMapArg,
+          `--junit-path=${junitPath}`,
+          "smoke/smoke-all.test.ts",
+          "--",
+          join(projectDir, "broken-a.qmd"),
+          join(projectDir, "broken-b.qmd"),
+          join(projectDir, "broken-c.qmd"),
+          join(smokeAllDir, "healthy-canary.qmd"),
+        ],
+        cwd: testsDir,
+        stdout: "piped",
+        stderr: "piped",
+      });
+
+      const output = await command.output();
+      const stdout = new TextDecoder().decode(output.stdout);
+      const stderr = new TextDecoder().decode(output.stderr);
+      const combined = stdout + stderr;
+
+      const junitXml = Deno.readTextFileSync(junitPath);
+      const { total, failures, cases } = parseJUnit(junitXml);
+
+      // Coverage-loss canary: the trailing healthy file must still be
+      // registered and pass.
+      const healthyCase = cases.find((c) => c.name.includes("healthy-canary"));
+      assert(
+        healthyCase !== undefined,
+        `expected a testcase for healthy-canary.qmd; got: ${
+          JSON.stringify(cases)
+        }\n---\n${combined}`,
+      );
+      assertEquals(healthyCase!.failed, false);
+
+      // Exactly one synthetic failure for the broken project, despite two
+      // annotated files (catches a retry-per-annotation implementation).
+      const projectFailures = cases.filter((c) =>
+        /_prerender-crash/.test(c.name) && c.failed
+      );
+      assertEquals(
+        projectFailures.length,
+        1,
+        `expected exactly one synthetic failure for the broken project; got: ${
+          JSON.stringify(cases)
+        }`,
+      );
+
+      // No test registered for any file of the broken project, including
+      // the unannotated one that precedes it in argument order (catches a
+      // one-pass implementation).
+      for (const fileName of ["broken-a.qmd", "broken-b.qmd", "broken-c.qmd"]) {
+        const registered = cases.some((c) => c.name.includes(fileName));
+        assert(
+          !registered,
+          `${fileName} must not be registered as its own test; got: ${
+            JSON.stringify(cases)
+          }`,
+        );
+      }
+
+      // Exactly two testcases total: the healthy canary and the one
+      // synthetic failure. Nothing from the broken project is silently
+      // dropped from the count either.
+      assertEquals(
+        total,
+        2,
+        `expected exactly 2 total testcases; got: ${JSON.stringify(cases)}`,
+      );
+      assertEquals(failures, 1);
+
+      // No module-evaluation abort: the failure is isolated to a synthetic
+      // test, not a crash of the whole smoke-all.test.ts file.
+      assert(
+        !combined.includes("Uncaught error from"),
+        `expected no module-evaluation abort; got:\n${combined}`,
+      );
+
+      assertEquals(
+        output.code,
+        1,
+        "the run still reports failure overall (one synthetic test failed)",
+      );
+
+      // Cleanup: the entry synthesized for the skipped broken-a.qmd must
+      // still remove its sentinel outputs, and the in-place project root
+      // itself must survive.
+      assert(
+        !safeExistsSync(join(projectDir, "broken-a.html")),
+        "broken-a.html should have been removed by the synthesized cleanup entry",
+      );
+      assert(
+        !safeExistsSync(supportDir),
+        "broken-a_files/ should have been removed by the synthesized cleanup entry",
+      );
+      assert(
+        safeExistsSync(projectDir),
+        "the in-place project root itself must survive cleanup",
+      );
+    } finally {
+      Deno.removeSync(tmpRoot, { recursive: true });
+    }
+  },
+);

From 04994b171d2fe31e016a899c5413b62765c2c045 Mon Sep 17 00:00:00 2001
From: Christophe Dervieux <christophe.dervieux@gmail.com>
Date: Tue, 22 Sep 2026 15:33:16 +0200
Subject: [PATCH 2/5] Sweep post-render cleanup for skipped smoke-all files,
 document kill-tree race

A registered file's teardown sweeps postRenderCleanupFiles (custom paths a
testSpec declares via postRenderCleanup), but a file skipped because its
project's pre-render failed never runs teardown. If a project's pre-render
partially succeeds before failing (rendering some of its own files as a
side effect) and none of the invocation's other files happen to run a
normal teardown, those custom paths were never swept. Sweep once in the
skip branch instead, after pass 1.5 has already run so any such artifact
already exists.

Documented, rather than attempted to close, the inherent TOCTOU gap in the
Unix process-tree kill: the pgrep enumeration and the kill pass are not
atomic, so a descendant spawned or reparented in between is invisible to
the walk even when cancelled comes back true. Closing that gap needs a
process group/job object, out of scope for test infrastructure and already
weighed and rejected during design.
---
 tests/quarto-cmd.ts                           | 35 ++++++--------
 tests/smoke/smoke-all.test.ts                 | 47 ++++++++-----------
 .../smoke-all-prerender-isolation.test.ts     | 32 +++++--------
 3 files changed, 46 insertions(+), 68 deletions(-)

diff --git a/tests/quarto-cmd.ts b/tests/quarto-cmd.ts
index 74e80961318..13cd47420e2 100644
--- a/tests/quarto-cmd.ts
+++ b/tests/quarto-cmd.ts
@@ -183,12 +183,10 @@ export function assertTestBinary(bin: string) {
   checkedBinary = bin;
 }
 
-// Outcome of a process-tree kill attempt. `cancelled` is true only when
-// every signalled node was positively enumerated: any spawn throw, a
-// non-zero/unparseable pgrep result, a non-zero taskkill exit, or a
-// Deno.kill failure other than NotFound leaves it false. This fail-safe
-// default is the point: an implementer who forgets a branch gets "fatal",
-// not "silently continue beside a possible orphan".
+// Result of attempting to stop a process tree. `cancelled` is true only when
+// tree enumeration and termination are confirmed. Enumeration, command, or
+// kill errors leave it false so callers do not continue while descendants
+// may still be running.
 interface KillOutcome {
   cancelled: boolean;
   detail: string;
@@ -198,9 +196,8 @@ interface KillOutcome {
 async function killProcessTree(pid: number): Promise<KillOutcome> {
   if (isWindows) {
     try {
-      // taskkill reports failure through its exit code; outputSync-style
-      // Deno.Command does not throw on a non-zero exit, only on a spawn
-      // failure, so the exit code is the only reliable signal here.
+      // `Deno.Command.output()` returns non-zero exit codes without throwing.
+      // Only a spawn failure throws, so check the exit code explicitly.
       const result = await new Deno.Command("taskkill", {
         args: ["/PID", String(pid), "/T", "/F"],
         stdout: "null",
@@ -239,13 +236,13 @@ async function killProcessTree(pid: number): Promise<KillOutcome> {
     }
   }
 
-  // Unix: enumerate with pgrep -P (Linux and macOS/BSD), classifying each
-  // invocation by exit code rather than by whether it threw -- outputSync()
-  // does not throw on a non-zero exit. Exit 0 with parseable stdout or
-  // exit 1 (a genuine childless leaf) are the only enumerated outcomes; a
-  // spawn throw, any other exit code, or unparseable stdout forces the
-  // aggregate `cancelled` to false without aborting the walk (still signal
-  // every pid already collected).
+  // On Unix, enumerate descendants with pgrep -P. Exit 0 must have parseable
+  // output, while exit 1 means no children. Any other result leaves
+  // cancellation unconfirmed, but all collected pids are still signalled.
+  //
+  // This confirms only descendants found during the pgrep walk. A process
+  // spawned or reparented before termination may not be included. Avoiding
+  // this race would require process-group support.
   const pids: number[] = [];
   const stack = [pid];
   let confirmed = true;
@@ -492,10 +489,8 @@ async function runBinaryQuarto(
     });
   }
 
-  // throwOnFailure: false callers (e.g. testQuartoCmd) get the one
-  // diagnostic for an unconfirmed kill; throwing callers get no log here
-  // (see the QuartoTimeoutError branch below) since a confirmed kill on
-  // this path has no orphan to warn about.
+  // Warn non-throwing callers when cancellation is unconfirmed. Throwing
+  // callers receive the same detail in QuartoTimeoutError below.
   if (timedOut && !killOutcome.cancelled && !throwOnFailure) {
     console.error(
       `[binary mode] process-tree kill UNCONFIRMED: ${commandLine} ` +
diff --git a/tests/smoke/smoke-all.test.ts b/tests/smoke/smoke-all.test.ts
index 7121ec91a56..27b6701b4c4 100644
--- a/tests/smoke/smoke-all.test.ts
+++ b/tests/smoke/smoke-all.test.ts
@@ -412,8 +412,7 @@ const projectFilePromises: Map<string, Promise<void>[]> = new Map();
 // Create an array to hold all the promises for the tests of files
 let testFilesPromises = [];
 
-// Records, keyed by project path, of a project whose pre-render failed
-// (populated in the pre-render pass below).
+// Pre-render failures keyed by project path.
 const failedProjectPreRenders: Map<string, Error> = new Map();
 
 interface DiscoveredFile {
@@ -424,11 +423,9 @@ interface DiscoveredFile {
   projectPath: string | undefined;
 }
 
-// Pass 1 (discovery, no registration). render-project is per-file front
-// matter, not a project-level setting, so a project can mix annotated and
-// unannotated files with the unannotated ones sorting first -- collecting
-// every file before registering (or pre-rendering) any of them is what
-// makes "skip every file of a failed project" achievable at all.
+// Pass 1: discover all files before registering tests. Because render-project
+// is per-file metadata, discovery must finish before all files belonging to a
+// failed project can be skipped.
 const discovered: DiscoveredFile[] = [];
 for (const { path: fileName } of files) {
   const input = relative(Deno.cwd(), fileName);
@@ -465,8 +462,8 @@ for (const { path: fileName } of files) {
   discovered.push({ input, metadata, testSpecs, projectPath });
 }
 
-// Pass 1.5 (pre-render). One attempt per distinct project that any
-// collected file marks render-project, ahead of any registration below.
+// Pass 1.5: pre-render each project requested by any discovered file before
+// registering tests.
 const projectsNeedingPreRender = new Set<string>();
 for (const entry of discovered) {
   if ((entry.metadata["_quarto"] as any)?.["render-project"] && entry.projectPath) {
@@ -475,17 +472,13 @@ for (const entry of discovered) {
 }
 for (const projectPath of projectsNeedingPreRender) {
   try {
-    // dispatches to the built binary when QUARTO_TEST_BIN is set; a
-    // failure here isolates this project's files (see
-    // failedProjectPreRenders below) rather than aborting the whole file.
+    // Use the built binary when QUARTO_TEST_BIN is set. A failure skips this
+    // project's tests instead of aborting module evaluation.
     await runQuarto(["render", projectPath]);
     renderedProjects.add(projectPath);
   } catch (err) {
-    // A timeout whose render was not confirmably stopped is fatal: an
-    // uncancelled dev-mode render, or a binary-mode process tree that
-    // could not be confirmed killed, could keep writing into this
-    // project's directory while the rest of the suite -- and the cleanup
-    // block below -- run alongside it.
+    // Abort if the timed-out render may still be running. It could keep
+    // writing while the remaining tests and cleanup run.
     if (isQuartoTimeoutError(err) && !err.renderCancelled) {
       console.error(
         `[smoke-all] project pre-render for ${projectPath} timed out after ` +
@@ -503,9 +496,7 @@ for (const projectPath of projectsNeedingPreRender) {
   }
 }
 
-// One synthetic failing test per failed project (not per skipped file):
-// one clearly named failure pointing at the cause, rather than N red lines
-// for one root cause.
+// Register one synthetic failing test per failed project.
 for (const [projectPath, error] of failedProjectPreRenders) {
   unitTest(`smoke-all project pre-render failed: ${projectPath}`, async () => {
     throw error;
@@ -520,13 +511,10 @@ for (const entry of discovered) {
     console.log(
       `Skipping tests for ${input}: its project's pre-render failed (${projectPath})`,
     );
-    // Mirror exactly what this file's own teardown would have pushed to
-    // projectCleanupEntries, so a skipped file lands at exact parity with
-    // a registered one. Only specs that would take the normal `render`
-    // branch push an entry -- the editor-support-crossref branch's
-    // teardown pushes none (it only removes its own temp file), and
-    // synthesizing one would reduce through parseFormatString to base
-    // editor and delete an unrelated html support directory.
+    // Add the cleanup entries that this file's teardown would have added.
+    // editor-support-crossref creates no project cleanup entry; adding one
+    // would resolve its base format to editor and could delete an unrelated
+    // HTML support directory.
     for (const testSpec of testSpecs) {
       if (testSpec.format === "editor-support-crossref") {
         continue;
@@ -540,6 +528,9 @@ for (const entry of discovered) {
         metadata,
       });
     }
+    // Skipped files do not run teardown, so sweep any custom cleanup paths
+    // registered during discovery.
+    postRenderCleanup();
     continue;
   }
 
@@ -650,4 +641,4 @@ function findRootTestsProjectDir(input: string) {
   const RootTestsRegex = new RegExp(`${smokeAllRootDir}|${ffMatrixRootDir}`);
   
   return findProjectDir(input, RootTestsRegex);
-}
\ No newline at end of file
+}
diff --git a/tests/unit/smoke-all-prerender-isolation.test.ts b/tests/unit/smoke-all-prerender-isolation.test.ts
index e9371939237..66f432cd659 100644
--- a/tests/unit/smoke-all-prerender-isolation.test.ts
+++ b/tests/unit/smoke-all-prerender-isolation.test.ts
@@ -75,11 +75,9 @@ unitTest(
           "    - broken-b.qmd\n    - broken-c.qmd\n",
       );
 
-      // Unannotated, excluded from project.render (so the pre-render never
-      // visits it), and pinned to html so the cleanup entry synthesized for
-      // a skipped file resolves to exactly broken-a.html / broken-a_files.
-      // Sorts first alphabetically, so a one-pass implementation would
-      // register it before the pre-render below ever runs.
+      // This unannotated file sorts before the annotated files but is excluded
+      // from project.render. Pinning it to HTML defines the cleanup paths for
+      // a skipped file.
       Deno.writeTextFileSync(
         join(projectDir, "broken-a.qmd"),
         [
@@ -96,10 +94,8 @@ unitTest(
           "",
         ].join("\n"),
       );
-      // Sentinels, pre-created so their post-run absence is attributable
-      // only to the cleanup entry synthesized for the skipped file, never
-      // to render ordering: the pre-render's project.render list excludes
-      // broken-a.qmd, so nothing else in the run can touch these paths.
+      // Pre-create outputs for broken-a.qmd. Because project.render excludes
+      // the file, their removal verifies cleanup for a skipped file.
       Deno.writeTextFileSync(
         join(projectDir, "broken-a.html"),
         "<html><body>sentinel</body></html>\n",
@@ -111,7 +107,7 @@ unitTest(
         "sentinel\n",
       );
 
-      // Annotated: the file whose render fails the project pre-render.
+      // This annotated file causes the project pre-render to fail.
       Deno.writeTextFileSync(
         join(projectDir, "broken-b.qmd"),
         [
@@ -127,9 +123,8 @@ unitTest(
           "",
         ].join("\n"),
       );
-      // A second annotated file: with only one, "exactly one synthetic
-      // failure" would be trivially satisfied by an implementation that
-      // retries the pre-render once per annotation.
+      // A second annotated file verifies that the pre-render runs once per
+      // project.
       Deno.writeTextFileSync(
         join(projectDir, "broken-c.qmd"),
         [
@@ -144,8 +139,8 @@ unitTest(
         ].join("\n"),
       );
 
-      // Coverage-loss canary: must still be registered and run despite
-      // sorting after the broken project in the explicit file-argument list.
+      // This healthy control file sorts after the broken project and must
+      // still run.
       Deno.writeTextFileSync(
         join(smokeAllDir, "healthy-canary.qmd"),
         ["---", "title: healthy", "---", "", "# healthy", ""].join("\n"),
@@ -197,8 +192,7 @@ unitTest(
       const junitXml = Deno.readTextFileSync(junitPath);
       const { total, failures, cases } = parseJUnit(junitXml);
 
-      // Coverage-loss canary: the trailing healthy file must still be
-      // registered and pass.
+      // The trailing healthy control file must still be registered and pass.
       const healthyCase = cases.find((c) => c.name.includes("healthy-canary"));
       assert(
         healthyCase !== undefined,
@@ -234,9 +228,7 @@ unitTest(
         );
       }
 
-      // Exactly two testcases total: the healthy canary and the one
-      // synthetic failure. Nothing from the broken project is silently
-      // dropped from the count either.
+      // Expect only the healthy control and one synthetic project failure.
       assertEquals(
         total,
         2,

From 326780dbaa29899b1fdff4cbe95d63efb2564f09 Mon Sep 17 00:00:00 2001
From: Christophe Dervieux <christophe.dervieux@gmail.com>
Date: Tue, 22 Sep 2026 15:47:22 +0200
Subject: [PATCH 3/5] Scope skipped-file cleanup sweep to its own input, not
 every registered path

postRenderCleanup() swept every currently-registered custom cleanup path
regardless of which file registered it. That's safe at a normal per-file
teardown, since the suite runs one file at a time so nothing else has
created a matching artifact yet by the time that teardown fires. The
skip-branch sweep added for a failed project runs at a different point in
time: after pass 1.5, when every project's pre-render (not just the failed
one) has already completed, so an unscoped sweep there could delete a
different, healthy project's artifact before that project's own tests get
to verify it. Track which input file registered each cleanup path and
scope the skip-branch sweep to that one file.
---
 tests/smoke/smoke-all.test.ts | 39 ++++++++++++++++++++++++-----------
 1 file changed, 27 insertions(+), 12 deletions(-)

diff --git a/tests/smoke/smoke-all.test.ts b/tests/smoke/smoke-all.test.ts
index 27b6701b4c4..f3ffb3a1dd6 100644
--- a/tests/smoke/smoke-all.test.ts
+++ b/tests/smoke/smoke-all.test.ts
@@ -162,20 +162,32 @@ interface QuartoInlineTestSpec {
 // postRenderCleanupFiles is a module-global list swept by EVERY render
 // teardown, so a registered entry only logs/removes at the teardowns where the
 // file actually exists (its owning document), not on every subsequent teardown.
-const postRenderCleanupFiles: string[] = [];
-function registerPostRenderCleanupFile(file: string): void {
-  postRenderCleanupFiles.push(file);
+// Each entry records which input file registered it, so a sweep can be
+// scoped to just that file's own entries (see postRenderCleanup below).
+const postRenderCleanupFiles: Array<{ file: string; input: string }> = [];
+function registerPostRenderCleanupFile(file: string, input: string): void {
+  postRenderCleanupFiles.push({ file, input });
 }
-const postRenderCleanup = () => {
+// With no `onlyForInputs`, sweeps every currently-registered path -- safe at
+// a normal teardown because the suite runs one file at a time, so nothing
+// else has created a matching artifact yet. A skipped file's own teardown
+// never runs, so the pass-2 skip branch instead scopes the sweep to just
+// that file's own input: by the time it runs, every project's pass-1.5
+// pre-render has already completed, so an unscoped sweep here could delete
+// another (healthy) project's not-yet-verified artifact.
+const postRenderCleanup = (onlyForInputs?: Set<string>) => {
   if (Deno.env.get("QUARTO_TEST_KEEP_OUTPUTS")) {
     return;
   }
-  for (const file of postRenderCleanupFiles) {
-    if (safeExistsSync(file)) {
-      console.log(`Cleaning up ${file} in ${Deno.cwd()}`);
+  for (const entry of postRenderCleanupFiles) {
+    if (onlyForInputs && !onlyForInputs.has(entry.input)) {
+      continue;
+    }
+    if (safeExistsSync(entry.file)) {
+      console.log(`Cleaning up ${entry.file} in ${Deno.cwd()}`);
       // recursive so a registered entry can be a directory (e.g. an embedded
       // notebook's `*_files` support dir), not just a single file
-      safeRemoveSync(file, { recursive: true });
+      safeRemoveSync(entry.file, { recursive: true });
     }
   }
 }
@@ -243,7 +255,7 @@ function resolveTestSpecs(
               file = file.replace("${input_stem}", inputStem);
             }
             // file is registered for cleanup in testQuartoCmd teardown step
-            registerPostRenderCleanupFile(join(dirname(input), file));
+            registerPostRenderCleanupFile(join(dirname(input), file), input);
           }
         } else if (key == "shouldError") {
           checkWarnings = false;
@@ -528,9 +540,12 @@ for (const entry of discovered) {
         metadata,
       });
     }
-    // Skipped files do not run teardown, so sweep any custom cleanup paths
-    // registered during discovery.
-    postRenderCleanup();
+    // Skipped files do not run teardown, so sweep this file's own custom
+    // cleanup paths registered during discovery. Scoped to just this input:
+    // every project's pass-1.5 pre-render has already run by this point, so
+    // an unscoped sweep could delete another (healthy) project's artifact
+    // before that project's own tests get to verify it.
+    postRenderCleanup(new Set([input]));
     continue;
   }
 

From 3ddac2cb238c1ced8c526123935c62294304fab6 Mon Sep 17 00:00:00 2001
From: Christophe Dervieux <christophe.dervieux@gmail.com>
Date: Tue, 22 Sep 2026 16:01:38 +0200
Subject: [PATCH 4/5] Clarify smoke-all isolation comments

---
 tests/quarto-cmd.ts                           |  4 ++--
 tests/smoke/smoke-all.test.ts                 | 23 ++++++++-----------
 .../smoke-all-prerender-isolation.test.ts     | 22 ++++++++----------
 3 files changed, 21 insertions(+), 28 deletions(-)

diff --git a/tests/quarto-cmd.ts b/tests/quarto-cmd.ts
index 13cd47420e2..258d11631b9 100644
--- a/tests/quarto-cmd.ts
+++ b/tests/quarto-cmd.ts
@@ -388,8 +388,8 @@ async function runDevQuarto(
       new QuartoTimeoutError(
         `timed out after ${timeoutMs}ms`,
         timeoutMs,
-        // Dev mode never attempts a kill: it can never cancel an
-        // in-process render, only lose the race below.
+        // Dev mode cannot stop an in-process render. A timeout rejects the
+        // caller, but the render continues.
         false,
       ),
     );
diff --git a/tests/smoke/smoke-all.test.ts b/tests/smoke/smoke-all.test.ts
index f3ffb3a1dd6..94afead08eb 100644
--- a/tests/smoke/smoke-all.test.ts
+++ b/tests/smoke/smoke-all.test.ts
@@ -168,13 +168,12 @@ const postRenderCleanupFiles: Array<{ file: string; input: string }> = [];
 function registerPostRenderCleanupFile(file: string, input: string): void {
   postRenderCleanupFiles.push({ file, input });
 }
-// With no `onlyForInputs`, sweeps every currently-registered path -- safe at
-// a normal teardown because the suite runs one file at a time, so nothing
-// else has created a matching artifact yet. A skipped file's own teardown
-// never runs, so the pass-2 skip branch instead scopes the sweep to just
-// that file's own input: by the time it runs, every project's pass-1.5
-// pre-render has already completed, so an unscoped sweep here could delete
-// another (healthy) project's not-yet-verified artifact.
+// Without `onlyForInputs`, this sweeps every registered path. That is safe
+// during normal teardown because the suite runs one file at a time and no
+// other test has created a matching artifact. A skipped file never runs
+// teardown, so the pass-2 branch scopes the sweep to that file's input. All
+// project pre-renders have finished by then; an unscoped sweep could delete
+// another project's artifact before verification.
 const postRenderCleanup = (onlyForInputs?: Set<string>) => {
   if (Deno.env.get("QUARTO_TEST_KEEP_OUTPUTS")) {
     return;
@@ -424,7 +423,6 @@ const projectFilePromises: Map<string, Promise<void>[]> = new Map();
 // Create an array to hold all the promises for the tests of files
 let testFilesPromises = [];
 
-// Pre-render failures keyed by project path.
 const failedProjectPreRenders: Map<string, Error> = new Map();
 
 interface DiscoveredFile {
@@ -508,7 +506,6 @@ for (const projectPath of projectsNeedingPreRender) {
   }
 }
 
-// Register one synthetic failing test per failed project.
 for (const [projectPath, error] of failedProjectPreRenders) {
   unitTest(`smoke-all project pre-render failed: ${projectPath}`, async () => {
     throw error;
@@ -540,11 +537,9 @@ for (const entry of discovered) {
         metadata,
       });
     }
-    // Skipped files do not run teardown, so sweep this file's own custom
-    // cleanup paths registered during discovery. Scoped to just this input:
-    // every project's pass-1.5 pre-render has already run by this point, so
-    // an unscoped sweep could delete another (healthy) project's artifact
-    // before that project's own tests get to verify it.
+    // Skipped files do not run teardown. Sweep only the custom cleanup paths
+    // registered for this input; an unscoped sweep could delete another
+    // project's artifact before its tests verify it.
     postRenderCleanup(new Set([input]));
     continue;
   }
diff --git a/tests/unit/smoke-all-prerender-isolation.test.ts b/tests/unit/smoke-all-prerender-isolation.test.ts
index 66f432cd659..a7e163f548b 100644
--- a/tests/unit/smoke-all-prerender-isolation.test.ts
+++ b/tests/unit/smoke-all-prerender-isolation.test.ts
@@ -75,9 +75,9 @@ unitTest(
           "    - broken-b.qmd\n    - broken-c.qmd\n",
       );
 
-      // This unannotated file sorts before the annotated files but is excluded
-      // from project.render. Pinning it to HTML defines the cleanup paths for
-      // a skipped file.
+      // This file does not request a project pre-render. It sorts before files
+      // that do, but project.render excludes it. Its HTML test spec registers
+      // cleanup paths for a skipped file.
       Deno.writeTextFileSync(
         join(projectDir, "broken-a.qmd"),
         [
@@ -107,7 +107,7 @@ unitTest(
         "sentinel\n",
       );
 
-      // This annotated file causes the project pre-render to fail.
+      // This file requests the project pre-render and makes it fail.
       Deno.writeTextFileSync(
         join(projectDir, "broken-b.qmd"),
         [
@@ -123,8 +123,8 @@ unitTest(
           "",
         ].join("\n"),
       );
-      // A second annotated file verifies that the pre-render runs once per
-      // project.
+      // A second file requests the same project pre-render, verifying that it
+      // runs once per project.
       Deno.writeTextFileSync(
         join(projectDir, "broken-c.qmd"),
         [
@@ -192,7 +192,6 @@ unitTest(
       const junitXml = Deno.readTextFileSync(junitPath);
       const { total, failures, cases } = parseJUnit(junitXml);
 
-      // The trailing healthy control file must still be registered and pass.
       const healthyCase = cases.find((c) => c.name.includes("healthy-canary"));
       assert(
         healthyCase !== undefined,
@@ -202,8 +201,8 @@ unitTest(
       );
       assertEquals(healthyCase!.failed, false);
 
-      // Exactly one synthetic failure for the broken project, despite two
-      // annotated files (catches a retry-per-annotation implementation).
+      // Exactly one synthetic failure is reported for the broken project,
+      // even though two files request its pre-render.
       const projectFailures = cases.filter((c) =>
         /_prerender-crash/.test(c.name) && c.failed
       );
@@ -215,9 +214,8 @@ unitTest(
         }`,
       );
 
-      // No test registered for any file of the broken project, including
-      // the unannotated one that precedes it in argument order (catches a
-      // one-pass implementation).
+      // No test is registered for any file in the broken project, including
+      // the file encountered before any file requests a project pre-render.
       for (const fileName of ["broken-a.qmd", "broken-b.qmd", "broken-c.qmd"]) {
         const registered = cases.some((c) => c.name.includes(fileName));
         assert(

From 0dc5d83c9aa3ab9b2efcf1e1811e284f157cd652 Mon Sep 17 00:00:00 2001
From: Christophe Dervieux <christophe.dervieux@gmail.com>
Date: Wed, 23 Sep 2026 16:37:14 +0200
Subject: [PATCH 5/5] Share project cleanup-entry logic between skipped and
 tested files

A skipped file must register the same cleanup entries its teardown would
have, including never registering one for editor-support-crossref. Keeping
that rule in one helper makes the parity structural instead of relying on
two copies staying in sync.
---
 tests/smoke/smoke-all.test.ts | 40 +++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 18 deletions(-)

diff --git a/tests/smoke/smoke-all.test.ts b/tests/smoke/smoke-all.test.ts
index 94afead08eb..127e00fb506 100644
--- a/tests/smoke/smoke-all.test.ts
+++ b/tests/smoke/smoke-all.test.ts
@@ -416,6 +416,26 @@ const projectCleanupEntries: Map<
   string,
   Array<{ input: string; format: string; metadata: Record<string, any> }>
 > = new Map();
+
+function addProjectCleanupEntry(
+  projectPath: string,
+  input: string,
+  format: string,
+  // deno-lint-ignore no-explicit-any
+  metadata: Record<string, any>,
+) {
+  // editor-support-crossref creates no project cleanup entry; adding one
+  // would resolve its base format to editor and could delete an unrelated
+  // HTML support directory.
+  if (format === "editor-support-crossref") {
+    return;
+  }
+  if (!projectCleanupEntries.has(projectPath)) {
+    projectCleanupEntries.set(projectPath, []);
+  }
+  projectCleanupEntries.get(projectPath)!.push({ input, format, metadata });
+}
+
 // The promise for each file's tests, grouped by the project it belongs to, so
 // we know when a given project's own files are all done (see above).
 const projectFilePromises: Map<string, Promise<void>[]> = new Map();
@@ -521,21 +541,8 @@ for (const entry of discovered) {
       `Skipping tests for ${input}: its project's pre-render failed (${projectPath})`,
     );
     // Add the cleanup entries that this file's teardown would have added.
-    // editor-support-crossref creates no project cleanup entry; adding one
-    // would resolve its base format to editor and could delete an unrelated
-    // HTML support directory.
     for (const testSpec of testSpecs) {
-      if (testSpec.format === "editor-support-crossref") {
-        continue;
-      }
-      if (!projectCleanupEntries.has(projectPath)) {
-        projectCleanupEntries.set(projectPath, []);
-      }
-      projectCleanupEntries.get(projectPath)!.push({
-        input,
-        format: testSpec.format,
-        metadata,
-      });
+      addProjectCleanupEntry(projectPath, input, testSpec.format, metadata);
     }
     // Skipped files do not run teardown. Sweep only the custom cleanup paths
     // registered for this input; an unscoped sweep could delete another
@@ -580,10 +587,7 @@ for (const entry of discovered) {
                   // files are cleaned once the whole project is done testing
                   // (see projectCleanupEntries above).
                   if (projectPath) {
-                    if (!projectCleanupEntries.has(projectPath)) {
-                      projectCleanupEntries.set(projectPath, []);
-                    }
-                    projectCleanupEntries.get(projectPath)!.push({ input, format, metadata });
+                    addProjectCleanupEntry(projectPath, input, format, metadata);
                   } else {
                     cleanoutput(input, format, undefined, undefined, metadata);
                   }