Fail the runtime boundary PR check on counts main would reject

.github/workflows/runtime-boundary.yml

@@ -2,9 +2,11 @@
 # Counts `Deno.` occurrences and direct std-library imports per scope
 # (src, src-js, resources, package/src, tests). Two modes:
 #   - pull_request: compares the merge commit with its first parent (the base
-#     branch tip) and fails only for a Deno.* API that src/deno_ral replaces;
-#     other increases are warnings plus a maintainer block in the job summary,
-#     or notices once the PR's own baseline accepts them.
+#     branch tip). A Deno.* API that src/deno_ral replaces is an error the
+#     contributor fixes; any other increase above the PR's own baseline is a
+#     warning that keeps the job red (within that baseline: a notice). The contributor
+#     has nothing to change: a maintainer pushes the baseline update to the PR
+#     before merging (job summary), so a green PR never turns main red.
 #   - push (main) and workflow_dispatch: fails when a count rises above
 #     tools/runtime-boundary-baseline.json.
 # See dev-docs/runtime-migration/02-runtime-boundary-check.md.

dev-docs/runtime-migration/README.md

@@ -132,6 +132,7 @@ The rows below are the ones that still need a named person or an external answer
 | PR 18, all tasks (from #15180) | the Deno backend keeps the `v8-<v8Branch>` store directory; reconcile it with the store names `index`/`sass-index` and the `<identifierHash>-<inputHash>.css` naming. Task 2 Step 2 shrinks to the API change: `this.index.list({ prefix: [] })` becomes `this.index.list([])`, and the `clear()` body #15180 shipped stays as is (`storeDir` paths, `safeRemoveSync`, immediate entries skipped; test `projectCache - clear() removes disk and immediate entries of a populated cache`). Drop the Context sentence saying `join` throws in `clear()`, and the Risks lines about that throwing path; one `cache.ts` `Deno.removeSync` is now `safeRemoveSync`. Refresh every Verified usage line number | coordinator | PR 18 step 0 |
 | PR 18 Tasks 1–3 (from the #15119 fix, once `src/deno_ral/kv.ts` is on `main`) | `kv.ts` already exists (`openKv`, `getKv`, `kvOp`, `KvDamagedError`) and both caches go through it, so Task 1 extends the file instead of creating it and its "Module not found" RED step no longer holds. `openKeyValueStore` keeps the damage mapping: a damaged value reads as a miss with one warning, a damaged file fails with `KvDamagedError` naming the folder to delete, any other error is wrapped with the path. The `src` baseline drops by 5, not 7 (the two `Deno.openKv` calls are already in the layer). The `Deno.openKv` grep acceptance check covers `src` and `package` only, since three cache tests build damaged fixtures with `Deno.openKv` on purpose. The `kv.ts` row in `.claude/rules/typescript/deno-ral.md` exists: update it | coordinator | PR 18 step 0 |
 | PR 19 Task 1, PR 27b | both replace the whole body of `platform.ts`: keep the `v8Branch` export (`_TEMPLATE.md`, "`platform.ts`"); PR 27b computes it from `process.versions.v8` | coordinator | step 0 of each |
+| PR 02c (from #15202, which turned `main` red) | Decided 2026-10-27: PR runs fail on any increase that leaves a count above their own `tools/runtime-boundary-baseline.json` (exactly what the push check on `main` will compare), instead of only warning; an increase that still fits because `main` sits below its baseline is a notice; 02c's "everything else is a warning for the maintainer" and its "exit 1 if any error, else 0" no longer hold. Per-line findings stay warnings saying the contributor has nothing to change, plus one `::error` saying a maintainer pushes the baseline update before merging; the remedy is unchanged (update the branch with `main`, `--update-baseline --accept-increase`, push to the PR). Rejected: an acceptance label (the merged tree still carries the old baseline, so `main` fails anyway) and warn-only with a merge checklist (the human step is what failed). A plan whose expected delta raises a count runs `--accept-increase` in its PR (PRs 01 and 05 already do) | maintainer | step 0 of any plan that raises a count |
 | PR 30 | tagged `v8-<branch>/` directories are left behind like the untagged `.kv` files; remove `v8Branch` and its contract-test line together with its last consumer. What "damaged" means for the JSON index (unparsable or truncated file): keep the miss-with-warning versus fail-naming-the-folder split or not. The Deno KV damage tests (`tests/unit/kv-damage.test.ts` and the damaged-store cases in `sass-cache.test.ts` and `project-cache.test.ts`, from the #15119 fix) build real SQLite damage: they are replaced or deleted here, because PR 32 runs the full unit suite on Node, where neither `Deno.openKv` nor its damage exists, and "`sass-cache.test.ts` stays green" is restated accordingly | maintainer (damage semantics), coordinator (the rest) | PR 30 step 0 |
 
 ## Shared rules and tables

dev-docs/runtime-migration/_TEMPLATE.md

@@ -84,7 +84,7 @@ Repository conventions that plans must respect (`.claude/CLAUDE.md` and `.claude
 - Unit tests live in `tests/unit/` (runtime-layer tests under the existing `tests/unit/ral/`), use `unitTest` from `tests/test.ts` and assertions from `tests/asserts.ts` (added by PR 02), and run with `cd tests && QUARTO_TESTS_NO_CONFIG=true ./run-tests.sh --agent unit/<file>.test.ts`.
   The ratchet (PR 02) has a `tests` scope: tests written by PRs 09 and later use `deno_ral` `fs`/`temp`/`process`, never raw `Deno.*`.
 - Landing order inside Phase 1: 02 → 02b → 08 → 22 → 10–13 → 14, with 09 before 10; 22 depends on PR 04 (Phase 0). 02c lands before any other count-changing PR.
-- A PR that adds an equivalent `deno_ral` replacement for a `Deno.*` API adds its exact keys (e.g. `readTextFileSync`) to `kRalDenoReplacements` in `tools/check-runtime-boundary.ts` in the same change, which turns later increases of that API into errors; a PR that adds a std seam may add a `kStdSuggestions` entry (suggestions never fail a PR). Under `kBrowserSharedPaths` (`src/core/lib/`, which the browser-side YAML intelligence imports and which therefore cannot import `deno_ral`) a covered API is only a warning.
+- A PR that adds an equivalent `deno_ral` replacement for a `Deno.*` API adds its exact keys (e.g. `readTextFileSync`) to `kRalDenoReplacements` in `tools/check-runtime-boundary.ts` in the same change, which turns later increases of that API into errors; a PR that adds a std seam may add a `kStdSuggestions` entry (a suggestion never makes a finding an error). Any increase that is not an error still fails the PR run while the count exceeds the PR's own baseline (`--update-baseline --accept-increase`), so a plan that raises a count runs that step in the PR. Under `kBrowserSharedPaths` (`src/core/lib/`, which the browser-side YAML intelligence imports and which therefore cannot import `deno_ral`) a covered API is only a warning.
   A conflict in a codemod PR is resolved by resetting the file to `main` and re-running the codemod, never by hand (the codemod is idempotent, PR 09); a conflict on a shared file (`tools/runtime-boundary-baseline.json`, `deno.lock`, the shim, the `deno-ral.md` table, a changelog) follows README ground rule 8.
 - Commands in the plans write the Linux path `package/dist/bin/tools/x86_64/deno`; on Apple Silicon it is `tools/aarch64/deno`, on Windows `tools\x86_64\deno.exe`.
 - Size is by file count (S < 10, M 10–40, L > 40 files); a plan that is large by volume but small by files says so next to the letter.

tests/unit/tools/check-runtime-boundary.test.ts

@@ -446,10 +446,10 @@ unitTest("check-runtime-boundary - diffChanges an import split across more files
 });
 
 const kUncoveredWarning = (scope: string, n: number, api: string, file: string) =>
-  `${scope}: +${n} ${DENO}${api} in ${file}; src/deno_ral has no replacement yet, so nothing to change in this PR. A maintainer accepts the new count before merging (see the job summary).`;
+  `${scope}: +${n} ${DENO}${api} in ${file}; src/deno_ral has no replacement yet, so nothing to change in this PR. A maintainer accepts the new count before merging; this job stays red until then (see the job summary).`;
 
 // deno-lint-ignore require-await
-unitTest("check-runtime-boundary - renderDelta uncovered increases: warnings and maintainer block, not failed", async () => {
+unitTest("check-runtime-boundary - renderDelta uncovered increases: warnings and maintainer block, failed until accepted", async () => {
   const current = { "tests": { denoCalls: 832, stdImportFiles: 177 } };
   const rendered = renderDelta(
     {
@@ -462,7 +462,7 @@ unitTest("check-runtime-boundary - renderDelta uncovered increases: warnings and
     current,
     { "tests": { denoCalls: 826, stdImportFiles: 177 } },
   );
-  assertEquals(rendered.failed, false);
+  assertEquals(rendered.failed, true);
   assertEquals(rendered.errors, []);
   assertEquals(rendered.notices, []);
   assertEquals(rendered.warnings.map((w) => w.message), [
@@ -471,6 +471,9 @@ unitTest("check-runtime-boundary - renderDelta uncovered increases: warnings and
   ]);
   assert(rendered.summary.includes("| tests | +6 | 0 |"));
   assert(rendered.summary.includes("### Maintainer action before merging"));
+  assert(rendered.summary.includes(
+    "Nothing for the contributor to change: this job stays red until a maintainer accepts the new counts.",
+  ));
   assert(rendered.summary.includes("Expected counts after merge: tests 832 / 177."));
   assert(!rendered.summary.includes("### What to change"));
 });
@@ -514,9 +517,9 @@ unitTest("check-runtime-boundary - renderDelta a std import with a suggestion is
     { "tests": { denoCalls: 832, stdImportFiles: 178 } },
     { "tests": { denoCalls: 832, stdImportFiles: 177 } },
   );
-  assertEquals(rendered.failed, false);
+  assertEquals(rendered.failed, true);
   assertEquals(rendered.warnings.map((w) => w.message), [
-    "tests: tests/unit/c.test.ts now imports the standard library directly (testing/asserts); if tests/asserts.ts exports what this file uses with the same behaviour, import it from there. Otherwise nothing to change in this PR: a maintainer accepts the new count before merging (see the job summary).",
+    "tests: tests/unit/c.test.ts now imports the standard library directly (testing/asserts); if tests/asserts.ts exports what this file uses with the same behaviour, import it from there. Otherwise nothing to change in this PR: a maintainer accepts the new count before merging; this job stays red until then (see the job summary).",
   ]);
 });
 
@@ -675,17 +678,40 @@ unitTest("check-runtime-boundary - renderDelta accepts only counts the PR itself
   // main sits below its baseline for calls; the PR raises only std-import files.
   const base = { "tests": { denoCalls: 832, stdImportFiles: 177 } };
   const rendered = renderDelta(report, current, base, { "tests": { denoCalls: 832, stdImportFiles: 178 } });
+  // The runtime call fits under the untouched baseline: main stays green, so it is a notice, not "accepted by this PR".
   assertEquals(rendered.accepted.map((a) => a.message), [
+    `tests: +1 ${DENO}cwd in tests/unit/a.test.ts; within tools/runtime-boundary-baseline.json, which main sits below.`,
     "tests: tests/unit/c.test.ts now imports the standard library directly (testing/asserts); accepted by this PR's tools/runtime-boundary-baseline.json.",
   ]);
-  assertEquals(rendered.warnings.map((w) => w.file), ["tests/unit/a.test.ts"]);
+  assertEquals(rendered.warnings, []);
+  assertEquals(rendered.failed, false);
   assert(rendered.summary.includes("- tests std-import files: baseline 177 → 178"));
   assert(!rendered.summary.includes(`- tests ${DENO}* calls`));
-  assert(rendered.summary.includes("### Maintainer action before merging"));
+  assert(!rendered.summary.includes("### Maintainer action before merging"));
+});
+
+// deno-lint-ignore require-await
+unitTest("check-runtime-boundary - renderDelta passes once the PR's baseline covers an increase made while main sits below its baseline", async () => {
+  const report = {
+    deltas: { "tests": { denoCalls: 1, stdImportFiles: 0 } },
+    findings: [{ scope: "tests", file: "tests/unit/a.test.ts", kind: "deno" as const, name: "cwd", delta: 1 }],
+  };
+  // main counts 829 against a committed 832; the PR adds one, and --accept-increase writes 830.
+  const rendered = renderDelta(
+    report,
+    { "tests": { denoCalls: 830, stdImportFiles: 177 } },
+    { "tests": { denoCalls: 832, stdImportFiles: 177 } },
+    { "tests": { denoCalls: 830, stdImportFiles: 177 } },
+  );
+  assertEquals(rendered.failed, false);
+  assertEquals(rendered.warnings, []);
+  assertEquals(rendered.accepted.map((a) => a.message), [
+    `tests: +1 ${DENO}cwd in tests/unit/a.test.ts; within tools/runtime-boundary-baseline.json, which main sits below.`,
+  ]);
 });
 
-/** Runs the script in PR mode on `root`, as CI does, and returns its stdout. */
-async function runAgainst(root: string, rev: string): Promise<string> {
+/** Runs the script in PR mode on `root`, as CI does, and returns its exit status and stdout. */
+async function runAgainst(root: string, rev: string): Promise<{ success: boolean; out: string }> {
   const result = await execProcess({
     cmd: resolve(quartoDevBinCmd()),
     args: [
@@ -696,8 +722,7 @@ async function runAgainst(root: string, rev: string): Promise<string> {
     stdout: "piped",
     stderr: "piped",
   });
-  assert(result.success, `check-runtime-boundary failed: ${result.stdout}\n${result.stderr}`);
-  return result.stdout ?? "";
+  return { success: result.success, out: (result.stdout ?? "") + (result.stderr ?? "") };
 }
 
 unitTest("check-runtime-boundary - PR mode: an increase the PR's baseline already accepts is a notice, not a warning", async () => {
@@ -714,16 +739,26 @@ unitTest("check-runtime-boundary - PR mode: an increase the PR's baseline alread
     const raised = { ...kExpected, "tests": { denoCalls: 2, stdImportFiles: 2 } };
     writeFixture(root, "tools/runtime-boundary-baseline.json", JSON.stringify(raised));
 
-    const out = await runAgainst(root, "HEAD");
+    const { success, out } = await runAgainst(root, "HEAD");
+    assert(success, out);
     assert(!out.includes("::warning"), out);
+    assert(!out.includes("::error"), out);
     assert(out.includes("::notice file=tests/j.test.ts,line=1::tests: +1 " + DENO + "cwd in tests/j.test.ts; accepted by this PR's tools/runtime-boundary-baseline.json"), out);
     assert(out.includes("### Accepted in this PR"), out);
     assert(out.includes(`- tests ${DENO}* calls: baseline 1 → 2`), out);
     assert(!out.includes("### Maintainer action before merging"), out);
 
-    // A baseline raised for a smaller increase than the PR makes leaves the warnings.
+    // A baseline raised for a smaller increase than the PR makes leaves the warnings and fails the job,
+    // telling the contributor that accepting the count is the maintainer's step.
     writeFixture(root, "tests/k.test.ts", `${DENO}cwd();\n`);
-    const stale = await runAgainst(root, "HEAD");
+    const { success: staleSuccess, out: stale } = await runAgainst(root, "HEAD");
+    assert(!staleSuccess, stale);
+    assert(
+      stale.includes(
+        "::error::This PR raises runtime boundary counts that tools/runtime-boundary-baseline.json does not accept yet. Nothing for the contributor to change: a maintainer pushes the baseline update to this PR before merging (see the job summary).",
+      ),
+      stale,
+    );
     assert(stale.includes("::warning file=tests/j.test.ts"), stale);
     assert(stale.includes("### Maintainer action before merging"), stale);
     assert(!stale.includes("### Accepted in this PR"), stale);

tools/check-runtime-boundary.ts

@@ -7,9 +7,10 @@
  * Counts, per scope, the `Deno.` occurrences and the files that import the
  * standard library directly instead of through src/deno_ral/. Two modes:
  * without flags it fails when any count exceeds the committed baseline (main);
- * with --against <rev> it compares the working tree with <rev> and fails only
- * for a Deno.* API that src/deno_ral already replaces (pull requests); other
- * increases are warnings until the working tree's baseline accepts them.
+ * with --against <rev> it compares the working tree with <rev> (pull requests):
+ * a Deno.* API that src/deno_ral already replaces is an error, any other
+ * increase above the working tree's baseline is a warning that still fails the
+ * run until a maintainer pushes the accepted baseline to the PR before merging.
  *
  * Usage (from the repository root):
  *   quarto run --dev tools/check-runtime-boundary.ts
@@ -505,15 +506,19 @@ export interface Rendered {
   accepted: Annotation[]; // increases the PR's own baseline already covers
   notices: string[];
   summary: string; // markdown for GITHUB_STEP_SUMMARY
-  failed: boolean; // errors.length > 0
+  failed: boolean; // errors or warnings: a warning is a count above the PR's baseline, which would turn main red
 }
 
 const kReadmeRelPath = "dev-docs/runtime-migration/README.md";
 
 const kAcceptClause =
-  "a maintainer accepts the new count before merging (see the job summary).";
+  "a maintainer accepts the new count before merging; this job stays red until then (see the job summary).";
 const kAcceptSentence = kAcceptClause[0].toUpperCase() + kAcceptClause.slice(1);
 
+/** Printed once in PR mode when a warning fails the job, so the contributor knows the remedy is not theirs. */
+const kPendingAcceptanceError =
+  `This PR raises runtime boundary counts that ${kBaselineRelPath} does not accept yet. Nothing for the contributor to change: a maintainer pushes the baseline update to this PR before merging (see the job summary).`;
+
 const signed = (n: number) => n > 0 ? `+${n}` : `${n}`;
 
 /** `current` is measure() of the working tree (the merge result on CI), so `current - deltas` is the base.
@@ -539,6 +544,11 @@ export function renderDelta(
       (current[scope]?.[field] ?? 0) <= raised;
   };
   const acceptedSuffix = `; accepted by this PR's ${kBaselineRelPath}.`;
+  // The push check on main compares with `merged`: a count within it cannot turn main red,
+  // even when the PR did not raise it (main sits below its baseline).
+  const isCovered = (scope: string, field: keyof BoundaryCounts) =>
+    (current[scope]?.[field] ?? 0) <= (merged[scope]?.[field] ?? 0);
+  const coveredSuffix = `; within ${kBaselineRelPath}, which main sits below.`;
 
   for (const finding of report.findings) {
     const annotation = (message: string): Annotation => {
@@ -556,11 +566,13 @@ export function renderDelta(
       );
       continue;
     }
-    if (isAccepted(finding.scope, finding.kind === "deno" ? "denoCalls" : "stdImportFiles")) {
+    const field = finding.kind === "deno" ? "denoCalls" : "stdImportFiles";
+    if (isAccepted(finding.scope, field) || isCovered(finding.scope, field)) {
+      const suffix = isAccepted(finding.scope, field) ? acceptedSuffix : coveredSuffix;
       accepted.push(annotation(
         finding.kind === "deno"
-          ? `${finding.scope}: +${finding.delta} Deno.${finding.name} in ${finding.file}${acceptedSuffix}`
-          : `${finding.scope}: ${finding.file} now imports the standard library directly (${finding.name})${acceptedSuffix}`,
+          ? `${finding.scope}: +${finding.delta} Deno.${finding.name} in ${finding.file}${suffix}`
+          : `${finding.scope}: ${finding.file} now imports the standard library directly (${finding.name})${suffix}`,
       ));
       continue;
     }
@@ -633,6 +645,8 @@ export function renderDelta(
     summary.push(
       "### Maintainer action before merging",
       "",
+      "Nothing for the contributor to change: this job stays red until a maintainer accepts the new counts.",
+      "",
       "This PR raises counts (warnings above). Where a warning suggests a module, check whether the PR should use it; accept the rest. Before merging, update the branch with `main`, then run",
       "",
       "    quarto run --dev tools/check-runtime-boundary.ts --update-baseline --accept-increase",
@@ -647,7 +661,7 @@ export function renderDelta(
     accepted,
     notices,
     summary: summary.join("\n"),
-    failed: errors.length > 0,
+    failed: errors.length > 0 || warnings.length > 0,
   };
 }
 
@@ -886,6 +900,9 @@ if (import.meta.main) {
     for (const { message, file, line } of rendered.errors) {
       error(message, { file, line });
     }
+    if (rendered.warnings.length > 0) {
+      error(kPendingAcceptanceError);
+    }
     if (!stepSummary(rendered.summary)) {
       console.log(rendered.summary);
     }