diff --git a/dev-docs/runtime-migration/02d-ratchet-counts-indirect-deno.md b/dev-docs/runtime-migration/02d-ratchet-counts-indirect-deno.md new file mode 100644 index 0000000000..e550fa59a6 --- /dev/null +++ b/dev-docs/runtime-migration/02d-ratchet-counts-indirect-deno.md @@ -0,0 +1,234 @@ +# PR 02d: the ratchet counts Deno reached indirectly + +> **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:** `countText` counts `Deno?.x`, `globalThis["Deno"]` and a bare `globalThis.Deno` the way it counts `Deno.x`, so code cannot stay out of the ratchet by spelling the access differently. +**Phase:** 0. +**Depends on:** PR 02c (merged). +**Unblocks:** none; every later PR is checked by the stricter count once this lands. +**Branch:** `rt/02d-ratchet-counts-indirect-deno`. +**Suggested executor:** Sonnet: one regex and its key extraction, with every case fixed by the tests below. +**Size:** S (4 files: the tool, its unit test, the baseline, README). +**Ships on:** Deno; the existing test suite must stay green. + +## Spec + +### Context + +The ratchet counts `\bDeno\.` (`tools/check-runtime-boundary.ts:118`). +PR 09's plan had a unit test read `(globalThis as any)["Deno"]?.errors` only so that the `tests` count stayed flat; review caught it (PR 09 execution log, "Review of Tasks 1–3"). +The current regex misses three spellings of the same access: + +1. optional chaining, `Deno?.errors` (also `globalThis.Deno?.build.os`, `(globalThis as any).Deno?.errors`); +2. bracket access, `globalThis["Deno"]?.errors`, `globalThis['Deno']`; +3. a bare `globalThis.Deno` that aliases the namespace (`Deno = globalThis.Deno;`). + +`globalThis.Deno.cwd()` is already counted: `\b` matches between `.` and `D`. +`typeof Deno`, the string `"Deno"`, and `["Deno"]` as an array element or a property of any object other than `globalThis` are not access to the namespace and stay uncounted. + +Measured on `main` at `364ee3bae` with the regex below (a scratch script over `git ls-files`, scoped with `scopesOf`): no optional-chaining or bracket hit outside the excluded paths; three bare hits, `src/core/lib/external/colors.ts:28` (`src`) and the generated bundles `src/resources/editor/tools/vs-code.mjs:25954`, `src/resources/editor/tools/yaml/web-worker.js:25968` (`resources`). +`src/deno_ral/` stays exempt (`kSrcExclusions`): its `(globalThis as any).Deno?.errors` in `errors.ts` is the layer's own runtime check. + +### Scope + +In: the counting regex and key extraction in `countText`, the `FileCounts` doc comment, unit tests, the baseline (`src` 991 → 992, `resources` 10 → 12), the README row. +Out: new exclusions; replacement-map entries; any change to how findings are attributed or reported (they consume `countText`'s keys unchanged). + +### Interfaces + +No signature changes. `FileCounts.deno` keys indirect access like direct access: `Deno?.errors` and `globalThis["Deno"]?.errors` key as `"errors"`, a bare `globalThis.Deno` or `globalThis["Deno"]` as `""`. + +### Files + +- Modify: `tools/check-runtime-boundary.ts:109-130` (`FileCounts` doc comment, `kDenoRe`, the key in `countText`) +- Modify: `tests/unit/tools/check-runtime-boundary.test.ts` (two tests after "countText never swallows a following prefix") +- Modify: `tools/runtime-boundary-baseline.json` +- Modify: `dev-docs/runtime-migration/README.md` (row in "The sequence") + +### Acceptance criteria + +- `tests/unit/tools/check-runtime-boundary.test.ts` passes, including the two new tests and the existing `countText` tests unchanged. +- `quarto run --dev tools/check-runtime-boundary.ts` on the branch: `src` 992, `resources` 12, other scopes unchanged, and no "rose" message. + +## Plan + +### Task 1: count indirect access + +**Files:** `tools/check-runtime-boundary.ts`, `tests/unit/tools/check-runtime-boundary.test.ts` + +- [x] **Step 1: Write the failing test.** Insert after the test "countText never swallows a following prefix". The fixtures build the name at run time, like the file's existing `DENO` constant, so the test file itself is not counted: + + ```typescript + const DENO_ID = DENO.slice(0, -1); + + // deno-lint-ignore require-await + unitTest("check-runtime-boundary - countText counts optional-chaining, bracket and globalThis access", async () => { + const text = [ + `${DENO_ID}?.errors`, + `${DENO_ID}?.errors?.NotFound`, + `globalThis.${DENO_ID}?.build.os`, + `(globalThis as any).${DENO_ID}?.errors`, + `(globalThis as any)["${DENO_ID}"]?.errors`, + `globalThis['${DENO_ID}']`, + `${DENO_ID} = globalThis.${DENO_ID};`, + `globalThis.${DENO_ID}.cwd()`, + ].join("\n"); + assertEquals(countText(text).deno, { errors: 3, "errors.NotFound": 1, "build.os": 1, "": 2, cwd: 1 }); + }); + + // deno-lint-ignore require-await + unitTest("check-runtime-boundary - countText ignores the name outside an access", async () => { + const text = [ + `typeof ${DENO_ID} === "undefined"`, + `const s = "${DENO_ID}";`, + `${DENO_ID}Kv.open()`, + `my${DENO_ID}.x`, + `const names = ["${DENO_ID}"];`, + `obj["${DENO_ID}"].x`, + ].join("\n"); + assertEquals(countText(text).deno, {}); + }); + ``` + +- [x] **Step 2: Run it and see it fail.** From `tests/`: `QUARTO_TESTS_NO_CONFIG=true ./run-tests.sh --agent unit/tools/check-runtime-boundary.test.ts`. + Expected: the first new test fails on `assertEquals` with the actual value `{ cwd: 1 }` (only `globalThis.Deno.cwd()` is counted today); the second passes (it guards against over-counting and is not RED). + +- [x] **Step 3: Implement.** In `tools/check-runtime-boundary.ts`, replace the `FileCounts` doc comment, `kDenoRe` and its comment, and the key line in `countText`: + + ```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. + * Indirect access keys the same way: `Deno?.errors` and `globalThis["Deno"]?.errors` as "errors", + * a bare `globalThis.Deno` or `globalThis["Deno"]` as "". `std` lists counted std specifiers + * (allow-listed excluded), in source order, duplicates kept. */ + ``` + + ```typescript + // Three alternatives: `Deno.` or `Deno?.`; `globalThis["Deno"]` (any quote, also `globalThis?.[`); + // a bare `globalThis.Deno` not followed by an access (the first alternative counts that one). + // The globalThis receiver may be cast, `(globalThis as any)`; requiring it keeps `["Deno"]` in an + // array or on another object uncounted. The key is read inside a lookahead so a following + // `Deno.` is never consumed. + const kDenoRe = + /\bDeno\??\.(?=(\w*)(?:\??\.(\w+))?)|\bglobalThis(?:\s+as\s+\w+\s*\))?\s*(?:\?\.)?\[\s*["'`]Deno["'`]\s*\](?:\??\.)?(?=(\w*)(?:\??\.(\w+))?)|\bglobalThis(?:\s+as\s+\w+\s*\))?\s*\??\.\s*Deno\b(?!\??\.)/g; + ``` + + ```typescript + for (const match of text.matchAll(kDenoRe)) { + const first = match[1] ?? match[3] ?? ""; + const second = match[2] ?? match[4]; + const key = first + (second ? "." + second : ""); + deno[key] = (deno[key] ?? 0) + 1; + } + ``` + +- [x] **Step 4: Run the tests.** Same command as Step 2. Expected: the `--agent` tally reports no failure; the existing tests "countText keys Deno APIs by one or two identifiers", "countText never swallows a following prefix" and "inherited property names count as ordinary Deno API keys" pass unchanged. + +- [x] **Step 5: Commit.** `git add tools/check-runtime-boundary.ts tests/unit/tools/check-runtime-boundary.test.ts`, message "Runtime boundary: count Deno reached through optional chaining and globalThis". + +### Task 2: baseline and README + +**Files:** `tools/runtime-boundary-baseline.json`, `dev-docs/runtime-migration/README.md` + +- [x] **Step 1: Run the check.** `quarto run --dev tools/check-runtime-boundary.ts` from the repository root. Expected rows: `src 992 (baseline 991)`, `resources 12 (baseline 10)`, every other count equal to its baseline, and "rose" messages for `src` and `resources`. Different numbers mean `main` moved: re-run the Context measurement and record it in the execution log. +- [x] **Step 2: Update the baseline.** `quarto run --dev tools/check-runtime-boundary.ts --update-baseline --accept-increase`; then re-run Step 1's command and expect no "rose" message. +- [x] **Step 3: README row.** In "The sequence", set the `02d` row's status from `planned` to `in progress` (the row landed with this plan). +- [x] **Step 4: Commit.** `git add tools/runtime-boundary-baseline.json dev-docs/runtime-migration/README.md dev-docs/runtime-migration/02d-ratchet-counts-indirect-deno.md`, message "Runtime boundary: baseline for indirect Deno access (src +1, resources +2)". + +## Verification + +- `QUARTO_TESTS_NO_CONFIG=true ./run-tests.sh --agent unit/tools/check-runtime-boundary.test.ts` from `tests/`: no failure. +- `quarto run --dev tools/check-runtime-boundary.ts`: no "rose" or "fell" message. +- The PR description lists the three newly counted lines from Context, as the `--accept-increase` warning asks. + +## Review checklist + +- [x] `typeof Deno`, a `"Deno"` string, `DenoKv`, `myDeno.x`, `["Deno"]` in an array and `obj["Deno"]` stay uncounted (second new test); bracket access counts only on a `globalThis` receiver. +- [x] `globalThis.Deno.cwd()` counts once, not twice (the bare alternative's negative lookahead). +- [x] The keys of indirect access match the keys of direct access, so `kRalDenoReplacements` findings apply to both (`globalThis.Deno?.build.os` keys as `build.os`). +- [x] No file outside the four in Files changed. +- [x] No new helper duplicates an existing one (`tests/utils.ts`, `tests/verify.ts`, `tests/unit/**/utils.ts`, `src/`). +- [x] No code outside `src/deno_ral/` reaches `Deno` indirectly to stay out of the ratchet count. +- [x] Comments in `src/` and `tests/` state the current reason only, with no plan or PR numbers. + +## Risks and rollback + +A false positive raises a count a PR did not really raise; it shows as a "rose" finding on an unrelated line and is fixed by tightening the regex with a test case. +Known gaps, left uncounted on purpose: a cast with more than one word (`(globalThis as Record)["Deno"]`), destructuring (`const { Deno: d } = globalThis`), an alias (`const d = Deno`) and `self.Deno`/`window.Deno`. +Measured on `main` at `364ee3bae`: none appears in a counted scope (the only destructuring is in the excluded `src/resources/extension-subtrees`). +Counting an alias would mean counting a bare `Deno`, which the comments counted with the code also use in prose; a deliberate dodge always finds a spelling a regex misses, so the review checklist item on indirect access is the check, and code that still reaches Deno fails on Node (PRs 26 and 31). +The PR check (`--against` the base) prints "main is above the baseline ... not caused by this PR" for `src` and `resources`: it counts the base tree with this PR's regex. That notice is expected here, and the PR description says so. +Rollback: revert the PR; the baseline goes back with it. + +## Execution log + +### Task 1: count indirect access + +Step 0 checks: `kDenoRe` at `tools/check-runtime-boundary.ts:118` was `/\bDeno\.(?=(\w*)(?:\.(\w+))?)/g`, `FileCounts` and `countText` at lines 109-130, baseline `src` 991 and `resources` 10. +The plan's code adds no helper duplicating one in `tests/` or `src/` (it reuses the file's `DENO` constant), reaches Deno through `globalThis` only inside fixture strings, and its comments cite no plan or PR numbers. +Setup: `./configure.cmd` ran; `package/dist/bin/quarto.cmd` exists. + +Step 2 (RED), from `tests/`: `$env:QUARTO_TESTS_NO_CONFIG="true"; pwsh -File run-tests.ps1 --agent unit/tools/check-runtime-boundary.test.ts` + +```text +[unit] > check-runtime-boundary - countText counts optional-chaining, bracket and globalThis access => ./test.ts:794:8 +Values are not equal. + [Diff] Actual / Expected ++ { ++ "": 2, ++ "build.os": 1, ++ "errors.NotFound": 1, +- [Object: null prototype] { + cwd: 1, ++ errors: 3, + } +FAILED | 42 passed | 1 failed (12s) +``` + +The actual value is `{ cwd: 1 }`; the second new test passed. + +Step 4 (GREEN), same command: + +```text +ok | 43 passed | 0 failed (14s) +``` + +### Task 2: baseline and README + +Step 1, from the repository root: `./package/dist/bin/quarto.cmd run --dev tools/check-runtime-boundary.ts` + +```text +scope Deno.* calls std-import files +src 992 (baseline 991) 49 (baseline 49) +src-js 7 (baseline 7) 0 (baseline 0) +resources 12 (baseline 10) 1 (baseline 1) +package/src 240 (baseline 240) 3 (baseline 3) +tests 874 (baseline 874) 176 (baseline 176) +src: Deno.* calls rose from 991 to 992 on main. ... +resources: Deno.* calls rose from 10 to 12 on main. ... +``` + +The numbers match the plan, so the Context measurement stands. + +Step 2: the same command with `--update-baseline --accept-increase`: + +```text +::warning::src baseline raised from 991 to 992 (Deno.* calls); list the new occurrences in the PR description +::warning::resources baseline raised from 10 to 12 (Deno.* calls); list the new occurrences in the PR description +Baseline written to tools/runtime-boundary-baseline.json +``` + +Re-run without the flags: + +```text +scope Deno.* calls std-import files +src 992 (baseline 992) 49 (baseline 49) +src-js 7 (baseline 7) 0 (baseline 0) +resources 12 (baseline 12) 1 (baseline 1) +package/src 240 (baseline 240) 3 (baseline 3) +tests 874 (baseline 874) 176 (baseline 176) +OK: no count exceeds the baseline. +``` + +Step 3: the `02d` row in `dev-docs/runtime-migration/README.md` now reads `in progress`. diff --git a/dev-docs/runtime-migration/HANDOFF.md b/dev-docs/runtime-migration/HANDOFF.md index 28816f06c2..d644f82c58 100644 --- a/dev-docs/runtime-migration/HANDOFF.md +++ b/dev-docs/runtime-migration/HANDOFF.md @@ -46,7 +46,7 @@ These were throwaway and are recreated by the plans when needed: `REVIEW-2026-10-15.md` is the principal review of the first draft (13 structural findings, per-plan defects, the landing strategy, the decisions taken in its section 6). Every plan was then reworked against it: six new PRs (00a, 00b, 00c, 02b, 14b, 20b, 27b), PR 21 folded into 25, PR 07 and PR 39 rewritten and renamed, `SPIKES.md` added, `_TEMPLATE.md` vocabulary updated, `00-assessment.md` given an errata section. `README.md` carries the new table, dependency graph, conventions and baseline. -The plans are now executable in the README order; PR 02 (#15135) and 02c (#15153) are merged, and the next to run are 00a, then 00b. +The plans are now executable in the README order; PR 02 (#15135) and 02c (#15153) are merged, 02d (the ratchet also counts `Deno` reached through optional chaining and `globalThis`) is in progress, and the next to run are 00a, then 00b. ## Reconciliation pass (bucket B of `TRIAGE-2026-10-16.md`): done on 2026-10-16 @@ -78,7 +78,7 @@ Two notes for whoever continues: the B1 commit carries the maintainer's author i 2. Read `README.md`, then `00-assessment.md` sections 10 and 11, then `_TEMPLATE.md`. 3. The section 9 decisions are taken (`REVIEW-2026-10-15.md` section 6); the test runner is decided by Spike V in `SPIKES.md` before PR 31. 4. Apply the local corrections of `TRIAGE-2026-10-16.md` bucket C (one task per plan file, in parallel where the triage marks it), then the re-read of 02, 25, 26, 27, 29, 33, 35, 36 the triage's step 4 asks for (02, 25, 26, 27 and 29 done 2026-10-16, 33, 35 and 36 done 2026-10-17: step 4 is complete). -5. PR 02 and 02c are merged; open the remaining plans as PRs against `main` in the README order: 00a and 00b, then 03–06 in parallel, then 07. +5. PR 02 and 02c are merged, 02d is in progress; open the remaining plans as PRs against `main` in the README order: 00a and 00b, then 03–06 in parallel, then 07. Each plan names its branch, executor and verification commands. For the per-worktree session ritual (window model, trust, fresh conversation, concurrency ceiling), check `local/multi-worktree-workflow.md` if it exists — it is a maintainer-local, untracked note. diff --git a/dev-docs/runtime-migration/README.md b/dev-docs/runtime-migration/README.md index d1f4cfa1cf..08a39930e2 100644 --- a/dev-docs/runtime-migration/README.md +++ b/dev-docs/runtime-migration/README.md @@ -201,6 +201,7 @@ Every PR that changes a count lands after 02c; preparing it earlier is fine. |---|---|---|---|---|---|---| | 02 | [02-runtime-boundary-check.md](02-runtime-boundary-check.md) | CI ratchet counting `Deno.*` and direct std imports in five scopes (`src`, `src-js`, `resources`, `package/src`, `tests`); `tests/asserts.ts`; lands first | none | Sonnet | S | merged | | 02c | [02c-ratchet-pr-delta-and-guidance.md](02c-ratchet-pr-delta-and-guidance.md) | PR runs compare with the base branch; findings name a working replacement or ask the maintainer to accept; `--accept-increase` | 02 | Sonnet | S | merged | +| 02d | [02d-ratchet-counts-indirect-deno.md](02d-ratchet-counts-indirect-deno.md) | The ratchet also counts `Deno?.x`, `globalThis["Deno"]` and a bare `globalThis.Deno` | 02c | Sonnet | S | in progress | | 00a | [00a-deno-bump.md](00a-deno-bump.md) | Deno 2.7.14 → newest 2.9.x everywhere the version appears | 02 | Sonnet | S | planned | | 00b | [00b-lockfile-and-bundle-gate.md](00b-lockfile-and-bundle-gate.md) | Committed `deno.lock` with `--frozen` (six entrypoints); `tools/check-test-imports.ts` keeps the unlocked tests inside that graph; three-OS bundle gate (cold cache, external-specifier grep, five renders) | 02, 00a | Opus | M | planned | | 00c | [00c-quarto-run-deprecation-notice.md](00c-quarto-run-deprecation-notice.md) | One-time notice when `quarto run` or a pre/post-render `.ts` script runs on Deno; `QUARTO_RUN_DENO_NOTICE=0` | 02 | Sonnet | S | planned | diff --git a/tests/unit/tools/check-runtime-boundary.test.ts b/tests/unit/tools/check-runtime-boundary.test.ts index 95ff2a6ba0..c8f5f9c21b 100644 --- a/tests/unit/tools/check-runtime-boundary.test.ts +++ b/tests/unit/tools/check-runtime-boundary.test.ts @@ -298,6 +298,36 @@ unitTest("check-runtime-boundary - countText never swallows a following prefix", assertEquals(countText(DENO + DENO + "build.os").deno, { [DENO + "build"]: 1, "build.os": 1 }); }); +const DENO_ID = DENO.slice(0, -1); + +// deno-lint-ignore require-await +unitTest("check-runtime-boundary - countText counts optional-chaining, bracket and globalThis access", async () => { + const text = [ + `${DENO_ID}?.errors`, + `${DENO_ID}?.errors?.NotFound`, + `globalThis.${DENO_ID}?.build.os`, + `(globalThis as any).${DENO_ID}?.errors`, + `(globalThis as any)["${DENO_ID}"]?.errors`, + `globalThis['${DENO_ID}']`, + `${DENO_ID} = globalThis.${DENO_ID};`, + `globalThis.${DENO_ID}.cwd()`, + ].join("\n"); + assertEquals(countText(text).deno, { errors: 3, "errors.NotFound": 1, "build.os": 1, "": 2, cwd: 1 }); +}); + +// deno-lint-ignore require-await +unitTest("check-runtime-boundary - countText ignores the name outside an access", async () => { + const text = [ + `typeof ${DENO_ID} === "undefined"`, + `const s = "${DENO_ID}";`, + `${DENO_ID}Kv.open()`, + `my${DENO_ID}.x`, + `const names = ["${DENO_ID}"];`, + `obj["${DENO_ID}"].x`, + ].join("\n"); + assertEquals(countText(text).deno, {}); +}); + // deno-lint-ignore require-await unitTest("check-runtime-boundary - countText lists counted std specifiers, not allow-listed ones", async () => { const text = [ diff --git a/tools/check-runtime-boundary.ts b/tools/check-runtime-boundary.ts index ce7f4bd04c..b045b4552d 100644 --- a/tools/check-runtime-boundary.ts +++ b/tools/check-runtime-boundary.ts @@ -107,15 +107,22 @@ const kCommentLineRe = /^\s*(\/\/|\/\*|\*)/; const kStdImportAllRe = new RegExp(kStdImportRe.source, "g"); /** 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. */ + * "readTextFileSync" for `Deno.readTextFileSync(`, "" for a `Deno.` followed by no identifier. + * Indirect access keys the same way: `Deno?.errors` and `globalThis["Deno"]?.errors` as "errors", + * a bare `globalThis.Deno` or `globalThis["Deno"]` as "". `std` lists counted std specifiers + * (allow-listed excluded), in source order, duplicates kept. */ export interface FileCounts { deno: Record; std: string[]; } -// The key is read inside a lookahead so a following `Deno.` is never consumed. -const kDenoRe = /\bDeno\.(?=(\w*)(?:\.(\w+))?)/g; +// Three alternatives: `Deno.` or `Deno?.`; `globalThis["Deno"]` (any quote, also `globalThis?.[`); +// a bare `globalThis.Deno` not followed by an access (the first alternative counts that one). +// The globalThis receiver may be cast, `(globalThis as any)`; requiring it keeps `["Deno"]` in an +// array or on another object uncounted. The key is read inside a lookahead so a following +// `Deno.` is never consumed. +const kDenoRe = + /\bDeno\??\.(?=(\w*)(?:\??\.(\w+))?)|\bglobalThis(?:\s+as\s+\w+\s*\))?\s*(?:\?\.)?\[\s*["'`]Deno["'`]\s*\](?:\??\.)?(?=(\w*)(?:\??\.(\w+))?)|\bglobalThis(?:\s+as\s+\w+\s*\))?\s*\??\.\s*Deno\b(?!\??\.)/g; /** Scans every import in the text, so an allow-listed import does not hide a counted one on the * same line, and a `from` separated from its specifier by a newline is still seen. Comment lines @@ -124,7 +131,9 @@ export function countText(text: string): FileCounts { // No prototype: `Deno.constructor` or `Deno.__proto__` in a comment must be an ordinary key. const deno: Record = Object.create(null); for (const match of text.matchAll(kDenoRe)) { - const key = match[1] + (match[2] ? "." + match[2] : ""); + const first = match[1] ?? match[3] ?? ""; + const second = match[2] ?? match[4]; + const key = first + (second ? "." + second : ""); deno[key] = (deno[key] ?? 0) + 1; } const code = text diff --git a/tools/runtime-boundary-baseline.json b/tools/runtime-boundary-baseline.json index 8d98344037..0607c709f1 100644 --- a/tools/runtime-boundary-baseline.json +++ b/tools/runtime-boundary-baseline.json @@ -1,6 +1,6 @@ { "src": { - "denoCalls": 991, + "denoCalls": 992, "stdImportFiles": 49 }, "src-js": { @@ -8,7 +8,7 @@ "stdImportFiles": 0 }, "resources": { - "denoCalls": 10, + "denoCalls": 12, "stdImportFiles": 1 }, "package/src": {