From 74641b26f97696e2ee38e33e89a794086cc2b823 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Thu, 22 Oct 2026 12:06:40 +0200 Subject: [PATCH 01/14] docs: runtime migration, PR 02c plan --- .../02c-ratchet-pr-delta-and-guidance.md | 372 ++++++++++++++++++ 1 file changed, 372 insertions(+) create mode 100644 dev-docs/runtime-migration/02c-ratchet-pr-delta-and-guidance.md diff --git a/dev-docs/runtime-migration/02c-ratchet-pr-delta-and-guidance.md b/dev-docs/runtime-migration/02c-ratchet-pr-delta-and-guidance.md new file mode 100644 index 0000000000..76cedf626a --- /dev/null +++ b/dev-docs/runtime-migration/02c-ratchet-pr-delta-and-guidance.md @@ -0,0 +1,372 @@ +# PR 02c: the ratchet blames only the PR's own increase and says what to change + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. +> Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** a pull request fails the Runtime Boundary job only for counts it raised itself, only for a `Deno.*` API with a curated, equivalent `src/deno_ral` replacement, and every finding says what to change; everything else is a warning for the maintainer; main keeps the absolute baseline gate. +**Phase:** 0. +**Depends on:** PR 02 (landed, #15135). +**Unblocks:** every PR that changes a count (00a, 00b, 00c, 01, 02b, 03–09, 14b, 20b): they may be prepared in parallel but land after this one, so their own counts are checked by the new rule. +**Branch:** `rt/02c-ratchet-pr-delta`. +**Suggested executor:** Sonnet: the plan fixes every interface, message and test; the counting refactor is guarded by an exact-totals check. +**Size:** S (9 files: the tool, its unit test, the workflow, 6 plan/README docs touched by text edits). +**Ships on:** Deno; the existing test suite must stay green. + +## Spec + +### Context + +PR 02 compares absolute counts per scope with `tools/runtime-boundary-baseline.json`. +On 2026-10-21 #15134 merged 20 minutes after the ratchet; its checks had run on a base without it, so its six new `Deno.*` calls in two test files went unchecked. +main went red, and because `pull_request` runs check out the merge commit, every open PR failed for an increase it did not make (#15136: run 39636363899). +The baseline was raised on main in `8599166fb` (`tests` 826 → 832) to recover. + +Three defects remain: + +1. Absolute comparison on PRs: any overflow on main fails every PR until a maintainer fixes main. The merge race recurs whenever a PR merges without a run against current main. +2. The `::error` always says "Application code must use src/deno_ral/", but `src/deno_ral` has no replacement for most `Deno.*` APIs yet: no `readTextFile`/`writeTextFile`/`makeTempDir`/`remove` (PR 09), no `env` (PR 14), only a thin `call()` for processes (PR 08). Even the covered areas are partial: `platform.ts` has `os`/`arch` but not `Deno.build.target`; `deno_ral/fs.ts` has `existsSync` but not `exists`; `tests/asserts.ts` has no `assertFalse`. A contributor bug fix that adds `Deno.readTextFileSync` in `src` fails with no remedy available. +3. Nothing tells the maintainer, at review time, that a PR needs a baseline bump before merge. + +Decided 2026-10-22: the committed baseline stays as a main-only gate (push runs compare absolute counts); PR runs compare the merge commit with its first parent; increases with no replacement are warnings plus a job-summary block addressed to the maintainer, who pushes the baseline bump into the PR before merging. + +### Scope + +In: +- Per-file counting keyed by Deno API (`Deno.` or `Deno..`) and by counted std specifier, with absolute totals unchanged. +- `--against `: delta mode comparing the working tree with `` over the changed files; per scope, increases net out against decreases of the same Deno API or the same std module before any finding is attributed. +- `kRalDenoReplacements`: exact, hand-curated API keys whose `deno_ral` replacement is equivalent; an increase of one is the only error in PR mode. +- `kStdSuggestions`: std module → a `deno_ral` (or `tests/asserts.ts`) module worth checking. std increases are always warnings: whether a wrapper is a working replacement depends on symbols, submodule semantics (`path/posix`) and type exports, which a name check cannot establish (design review, 2026-10-22). +- Annotations on the added line where one is found; a job summary with the per-scope delta, what to change, and the maintainer block with the expected post-merge counts. +- `--accept-increase` replaces `--accept-tests-increase` and accepts any scope (an uncovered `src` increase needs it too). +- Workflow: `fetch-depth: 2`, `--against HEAD^1` on `pull_request`, absolute mode on `push` and `workflow_dispatch`. +- Docs: README row, ratchet sentence and landing order; the flag rename in live plans; the rule that a PR adding a replacement also adds its map entry. + +Out: +- PR comments (fork PRs have a read-only token; `pull_request_target` is not worth its security surface for text the job summary already carries). +- Branch protection or merge-queue changes. +- The review-time skill (beads child "Skill: handling a Runtime Boundary failure on a PR", written after this lands). +- Delta reporting on push runs (the absolute gate plus `git log` is enough for a maintainer). + +### Interfaces + +All in `tools/check-runtime-boundary.ts`. Existing exports keep their signatures except `acceptTestsIncrease`, which becomes `acceptIncrease`. + +```typescript +/** Per-file counts. `deno` maps the API key to its occurrences: "build.os" for `Deno.build.os`, + * "readTextFileSync" for `Deno.readTextFileSync(`, "" for a `Deno.` followed by no identifier, + * so the sum equals the old \bDeno\. count. `std` lists counted std specifiers (allow-listed excluded), in source order, duplicates kept. */ +export interface FileCounts { + deno: Record; + std: string[]; +} +export function countText(text: string): FileCounts; + +/** The scopes a repository-relative path (forward slashes) is counted in: same exts, exclude, include and kSkip rules as countScope. */ +export function scopesOf(rel: string): string[]; + +/** The std module of a specifier, "@std/" stripped so equivalent spellings agree: + * "fs/ensure-dir", "@std/fs", "jsr:@std/fs@1.0.16/walk" -> "fs"; "testing/asserts" -> "testing"; + * "jsr:/@std/assert" and "https://deno.land/std@0.93.0/assert/assert_object_match.ts" -> "assert"; + * "@std/path", "path" -> "path". */ +export function stdModuleOf(specifier: string): string; + +/** Keyed by exact API key. A PR that adds a deno_ral replacement adds its keys here. */ +export const kRalDenoReplacements: Readonly>; +/** Keyed by stdModuleOf(). `module` is repository-relative. A suggestion only: never turns a finding into an error. */ +export const kStdSuggestions: Readonly>; + +export function denoReplacement(api: string, scope: string): string | undefined; +export function stdSuggestion(specifier: string, scope: string): string | undefined; + +export interface FileChange { + path: string; // repository-relative, forward slashes, verbatim (no git quoting) + before: string | null; // text at the base revision, null only when the path is absent there + after: string | null; // text in the working tree, null if deleted + addedLines: number[]; // 1-based line numbers in `after` that the diff adds +} +export interface Finding { + scope: string; + file: string; + kind: "deno" | "std"; + name: string; // API key, or the std specifier as written + delta: number; // > 0 (deno: this file's increase of the API; std: 1) + line?: number; // first added line containing the occurrence + replacement?: string; // deno only; set = covered = error + suggestion?: string; // std only: module from kStdSuggestions +} +export interface DeltaReport { + deltas: Record; // per scope, after minus before, over the changed files; may be negative + findings: Finding[]; +} +export function diffChanges(changes: FileChange[]): DeltaReport; + +/** Parses `git diff -U0` output for one file into the added line numbers of the new side. */ +export function parseAddedLines(diff: string): number[]; + +/** Changed and untracked files under any scope, working tree vs `rev`, renames split into delete + add. + * Throws Error("check-runtime-boundary: is not a commit") for an unknown revision and + * Error("check-runtime-boundary: git failed: ") for any other failing git call. */ +export async function gitChanges(root: string, rev: string): Promise; + +export interface Annotation { message: string; file: string; line?: number } +export interface Rendered { + errors: Annotation[]; + warnings: Annotation[]; + notices: string[]; + summary: string; // markdown for GITHUB_STEP_SUMMARY + failed: boolean; // errors.length > 0 +} +/** `current` is measure() of the working tree (the merge result on CI), so `current - deltas` is the base. */ +export function renderDelta(report: DeltaReport, current: Baseline, baseline: Baseline): Rendered; + +/** The current counts for every scope, raised or lowered. */ +export function acceptIncrease(current: Baseline): Baseline; +``` + +Map contents at this PR (only what is replaced on main today): + +```typescript +export const kRalDenoReplacements = { + "build.os": { hint: "os (or isWindows, isMac, isLinux) from src/deno_ral/platform.ts" }, + "build.arch": { hint: "arch from src/deno_ral/platform.ts" }, +}; +export const kStdSuggestions = { + "path": { module: "src/deno_ral/path.ts" }, + "fs": { module: "src/deno_ral/fs.ts" }, + "testing": { module: "tests/asserts.ts", scopes: ["tests"] }, + "assert": { module: "tests/asserts.ts", scopes: ["tests"] }, +}; +``` + +Attribution rules for `diffChanges`: +- Deno: only when the scope's `denoCalls` delta is > 0 (ratchet semantics: a PR that lowers a scope's total is never flagged). Then, per API key, `net = Σ(after − before)` over the changed files; only an API with `net > 0` produces findings: one per file whose own change for that API is positive, with that file's `delta`. +- std: only when the scope's `stdImportFiles` delta is > 0. Then, per std module (`stdModuleOf`), `net` = files of the changed set importing it after minus before; only a module with `net > 0` produces findings: one per file that imports it after but not before, `delta: 1`, `name` the file's first specifier of that module, `suggestion` from `stdSuggestion`. Moving an import between files therefore nets out, as for Deno APIs. When the std-file delta is positive but no module nets positive (an import split across more files), one finding per newly counted file with its first counted specifier, so the maintainer block still appears. +- `line`: first entry of `addedLines` whose text contains `Deno.` (deno) or the specifier (std); else undefined. + +Messages (exact; `<...>` substituted; `README` is `dev-docs/runtime-migration/README.md`): + +| Case | Kind | Text | +|---|---|---| +| covered Deno API | error | `: + Deno. in ; use instead (runtime boundary, see README)` | +| uncovered Deno API | warning | `: + Deno. in ; 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).` | +| std import, with suggestion | warning | `: now imports the standard library directly (); if 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).` | +| std import, no suggestion | warning | `: now imports the standard library directly (); there is no runtime-layer replacement for it yet, so nothing to change in this PR. A maintainer accepts the new count before merging (see the job summary).` | +| PR mode, base above baseline | notice | `main is above tools/runtime-boundary-baseline.json (