|
| 1 | +# PR 02d: the ratchet counts Deno reached indirectly |
| 2 | + |
| 3 | +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. |
| 4 | +> Steps use checkbox (`- [ ]`) syntax for tracking. |
| 5 | +
|
| 6 | +**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. |
| 7 | +**Phase:** 0. |
| 8 | +**Depends on:** PR 02c (merged). |
| 9 | +**Unblocks:** none; every later PR is checked by the stricter count once this lands. |
| 10 | +**Branch:** `rt/02d-ratchet-counts-indirect-deno`. |
| 11 | +**Suggested executor:** Sonnet: one regex and its key extraction, with every case fixed by the tests below. |
| 12 | +**Size:** S (4 files: the tool, its unit test, the baseline, README). |
| 13 | +**Ships on:** Deno; the existing test suite must stay green. |
| 14 | + |
| 15 | +## Spec |
| 16 | + |
| 17 | +### Context |
| 18 | + |
| 19 | +The ratchet counts `\bDeno\.` (`tools/check-runtime-boundary.ts:118`). |
| 20 | +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"). |
| 21 | +The current regex misses three spellings of the same access: |
| 22 | + |
| 23 | +1. optional chaining, `Deno?.errors` (also `globalThis.Deno?.build.os`, `(globalThis as any).Deno?.errors`); |
| 24 | +2. bracket access, `globalThis["Deno"]?.errors`, `globalThis['Deno']`; |
| 25 | +3. a bare `globalThis.Deno` that aliases the namespace (`Deno = globalThis.Deno;`). |
| 26 | + |
| 27 | +`globalThis.Deno.cwd()` is already counted: `\b` matches between `.` and `D`. |
| 28 | +`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. |
| 29 | + |
| 30 | +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`). |
| 31 | +`src/deno_ral/` stays exempt (`kSrcExclusions`): its `(globalThis as any).Deno?.errors` in `errors.ts` is the layer's own runtime check. |
| 32 | + |
| 33 | +### Scope |
| 34 | + |
| 35 | +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. |
| 36 | +Out: new exclusions; replacement-map entries; any change to how findings are attributed or reported (they consume `countText`'s keys unchanged). |
| 37 | + |
| 38 | +### Interfaces |
| 39 | + |
| 40 | +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 `""`. |
| 41 | + |
| 42 | +### Files |
| 43 | + |
| 44 | +- Modify: `tools/check-runtime-boundary.ts:109-130` (`FileCounts` doc comment, `kDenoRe`, the key in `countText`) |
| 45 | +- Modify: `tests/unit/tools/check-runtime-boundary.test.ts` (two tests after "countText never swallows a following prefix") |
| 46 | +- Modify: `tools/runtime-boundary-baseline.json` |
| 47 | +- Modify: `dev-docs/runtime-migration/README.md` (row in "The sequence") |
| 48 | + |
| 49 | +### Acceptance criteria |
| 50 | + |
| 51 | +- `tests/unit/tools/check-runtime-boundary.test.ts` passes, including the two new tests and the existing `countText` tests unchanged. |
| 52 | +- `quarto run --dev tools/check-runtime-boundary.ts` on the branch: `src` 992, `resources` 12, other scopes unchanged, and no "rose" message. |
| 53 | + |
| 54 | +## Plan |
| 55 | + |
| 56 | +### Task 1: count indirect access |
| 57 | + |
| 58 | +**Files:** `tools/check-runtime-boundary.ts`, `tests/unit/tools/check-runtime-boundary.test.ts` |
| 59 | + |
| 60 | +- [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: |
| 61 | + |
| 62 | + ```typescript |
| 63 | + const DENO_ID = DENO.slice(0, -1); |
| 64 | + |
| 65 | + // deno-lint-ignore require-await |
| 66 | + unitTest("check-runtime-boundary - countText counts optional-chaining, bracket and globalThis access", async () => { |
| 67 | + const text = [ |
| 68 | + `${DENO_ID}?.errors`, |
| 69 | + `${DENO_ID}?.errors?.NotFound`, |
| 70 | + `globalThis.${DENO_ID}?.build.os`, |
| 71 | + `(globalThis as any).${DENO_ID}?.errors`, |
| 72 | + `(globalThis as any)["${DENO_ID}"]?.errors`, |
| 73 | + `globalThis['${DENO_ID}']`, |
| 74 | + `${DENO_ID} = globalThis.${DENO_ID};`, |
| 75 | + `globalThis.${DENO_ID}.cwd()`, |
| 76 | + ].join("\n"); |
| 77 | + assertEquals(countText(text).deno, { errors: 3, "errors.NotFound": 1, "build.os": 1, "": 2, cwd: 1 }); |
| 78 | + }); |
| 79 | + |
| 80 | + // deno-lint-ignore require-await |
| 81 | + unitTest("check-runtime-boundary - countText ignores the name outside an access", async () => { |
| 82 | + const text = [ |
| 83 | + `typeof ${DENO_ID} === "undefined"`, |
| 84 | + `const s = "${DENO_ID}";`, |
| 85 | + `${DENO_ID}Kv.open()`, |
| 86 | + `my${DENO_ID}.x`, |
| 87 | + `const names = ["${DENO_ID}"];`, |
| 88 | + `obj["${DENO_ID}"].x`, |
| 89 | + ].join("\n"); |
| 90 | + assertEquals(countText(text).deno, {}); |
| 91 | + }); |
| 92 | + ``` |
| 93 | + |
| 94 | +- [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`. |
| 95 | + 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). |
| 96 | + |
| 97 | +- [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`: |
| 98 | + |
| 99 | + ```typescript |
| 100 | + /** Per-file counts. `deno` maps the API key to its occurrences: "build.os" for `Deno.build.os`, |
| 101 | + * "readTextFileSync" for `Deno.readTextFileSync(`, "" for a `Deno.` followed by no identifier. |
| 102 | + * Indirect access keys the same way: `Deno?.errors` and `globalThis["Deno"]?.errors` as "errors", |
| 103 | + * a bare `globalThis.Deno` or `globalThis["Deno"]` as "". `std` lists counted std specifiers |
| 104 | + * (allow-listed excluded), in source order, duplicates kept. */ |
| 105 | + ``` |
| 106 | + |
| 107 | + ```typescript |
| 108 | + // Three alternatives: `Deno.` or `Deno?.`; `globalThis["Deno"]` (any quote, also `globalThis?.[`); |
| 109 | + // a bare `globalThis.Deno` not followed by an access (the first alternative counts that one). |
| 110 | + // The globalThis receiver may be cast, `(globalThis as any)`; requiring it keeps `["Deno"]` in an |
| 111 | + // array or on another object uncounted. The key is read inside a lookahead so a following |
| 112 | + // `Deno.` is never consumed. |
| 113 | + const kDenoRe = |
| 114 | + /\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; |
| 115 | + ``` |
| 116 | + |
| 117 | + ```typescript |
| 118 | + for (const match of text.matchAll(kDenoRe)) { |
| 119 | + const first = match[1] ?? match[3] ?? ""; |
| 120 | + const second = match[2] ?? match[4]; |
| 121 | + const key = first + (second ? "." + second : ""); |
| 122 | + deno[key] = (deno[key] ?? 0) + 1; |
| 123 | + } |
| 124 | + ``` |
| 125 | + |
| 126 | +- [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. |
| 127 | + |
| 128 | +- [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". |
| 129 | + |
| 130 | +### Task 2: baseline and README |
| 131 | + |
| 132 | +**Files:** `tools/runtime-boundary-baseline.json`, `dev-docs/runtime-migration/README.md` |
| 133 | + |
| 134 | +- [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. |
| 135 | +- [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. |
| 136 | +- [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). |
| 137 | +- [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)". |
| 138 | + |
| 139 | +## Verification |
| 140 | + |
| 141 | +- `QUARTO_TESTS_NO_CONFIG=true ./run-tests.sh --agent unit/tools/check-runtime-boundary.test.ts` from `tests/`: no failure. |
| 142 | +- `quarto run --dev tools/check-runtime-boundary.ts`: no "rose" or "fell" message. |
| 143 | +- The PR description lists the three newly counted lines from Context, as the `--accept-increase` warning asks. |
| 144 | + |
| 145 | +## Review checklist |
| 146 | + |
| 147 | +- [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. |
| 148 | +- [x] `globalThis.Deno.cwd()` counts once, not twice (the bare alternative's negative lookahead). |
| 149 | +- [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`). |
| 150 | +- [x] No file outside the four in Files changed. |
| 151 | +- [x] No new helper duplicates an existing one (`tests/utils.ts`, `tests/verify.ts`, `tests/unit/**/utils.ts`, `src/`). |
| 152 | +- [x] No code outside `src/deno_ral/` reaches `Deno` indirectly to stay out of the ratchet count. |
| 153 | +- [x] Comments in `src/` and `tests/` state the current reason only, with no plan or PR numbers. |
| 154 | + |
| 155 | +## Risks and rollback |
| 156 | + |
| 157 | +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. |
| 158 | +Known gaps, left uncounted on purpose: a cast with more than one word (`(globalThis as Record<string, unknown>)["Deno"]`), destructuring (`const { Deno: d } = globalThis`), an alias (`const d = Deno`) and `self.Deno`/`window.Deno`. |
| 159 | +Measured on `main` at `364ee3bae`: none appears in a counted scope (the only destructuring is in the excluded `src/resources/extension-subtrees`). |
| 160 | +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). |
| 161 | +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. |
| 162 | +Rollback: revert the PR; the baseline goes back with it. |
| 163 | + |
| 164 | +## Execution log |
| 165 | + |
| 166 | +### Task 1: count indirect access |
| 167 | + |
| 168 | +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. |
| 169 | +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. |
| 170 | +Setup: `./configure.cmd` ran; `package/dist/bin/quarto.cmd` exists. |
| 171 | + |
| 172 | +Step 2 (RED), from `tests/`: `$env:QUARTO_TESTS_NO_CONFIG="true"; pwsh -File run-tests.ps1 --agent unit/tools/check-runtime-boundary.test.ts` |
| 173 | + |
| 174 | +```text |
| 175 | +[unit] > check-runtime-boundary - countText counts optional-chaining, bracket and globalThis access => ./test.ts:794:8 |
| 176 | +Values are not equal. |
| 177 | + [Diff] Actual / Expected |
| 178 | ++ { |
| 179 | ++ "": 2, |
| 180 | ++ "build.os": 1, |
| 181 | ++ "errors.NotFound": 1, |
| 182 | +- [Object: null prototype] { |
| 183 | + cwd: 1, |
| 184 | ++ errors: 3, |
| 185 | + } |
| 186 | +FAILED | 42 passed | 1 failed (12s) |
| 187 | +``` |
| 188 | + |
| 189 | +The actual value is `{ cwd: 1 }`; the second new test passed. |
| 190 | + |
| 191 | +Step 4 (GREEN), same command: |
| 192 | + |
| 193 | +```text |
| 194 | +ok | 43 passed | 0 failed (14s) |
| 195 | +``` |
| 196 | + |
| 197 | +### Task 2: baseline and README |
| 198 | + |
| 199 | +Step 1, from the repository root: `./package/dist/bin/quarto.cmd run --dev tools/check-runtime-boundary.ts` |
| 200 | + |
| 201 | +```text |
| 202 | +scope Deno.* calls std-import files |
| 203 | +src 992 (baseline 991) 49 (baseline 49) |
| 204 | +src-js 7 (baseline 7) 0 (baseline 0) |
| 205 | +resources 12 (baseline 10) 1 (baseline 1) |
| 206 | +package/src 240 (baseline 240) 3 (baseline 3) |
| 207 | +tests 874 (baseline 874) 176 (baseline 176) |
| 208 | +src: Deno.* calls rose from 991 to 992 on main. ... |
| 209 | +resources: Deno.* calls rose from 10 to 12 on main. ... |
| 210 | +``` |
| 211 | + |
| 212 | +The numbers match the plan, so the Context measurement stands. |
| 213 | + |
| 214 | +Step 2: the same command with `--update-baseline --accept-increase`: |
| 215 | + |
| 216 | +```text |
| 217 | +::warning::src baseline raised from 991 to 992 (Deno.* calls); list the new occurrences in the PR description |
| 218 | +::warning::resources baseline raised from 10 to 12 (Deno.* calls); list the new occurrences in the PR description |
| 219 | +Baseline written to tools/runtime-boundary-baseline.json |
| 220 | +``` |
| 221 | + |
| 222 | +Re-run without the flags: |
| 223 | + |
| 224 | +```text |
| 225 | +scope Deno.* calls std-import files |
| 226 | +src 992 (baseline 992) 49 (baseline 49) |
| 227 | +src-js 7 (baseline 7) 0 (baseline 0) |
| 228 | +resources 12 (baseline 12) 1 (baseline 1) |
| 229 | +package/src 240 (baseline 240) 3 (baseline 3) |
| 230 | +tests 874 (baseline 874) 176 (baseline 176) |
| 231 | +OK: no count exceeds the baseline. |
| 232 | +``` |
| 233 | + |
| 234 | +Step 3: the `02d` row in `dev-docs/runtime-migration/README.md` now reads `in progress`. |
0 commit comments