docs(carriers): record the third closure review follow-up

This commit is contained in:
montjeffrey
2026-10-09 17:48:50 -04:00
parent bdb2ceb230
commit 0ca7864ad7
+4 -2
View File
@@ -89,7 +89,7 @@ Zero escapes is the expected result and not evidence the check is idle. Carriers
Absolute specifiers fail as `outside-carrier-tree`. A package import warns rather than fails, because carriers ship no `node_modules`; Node builtins, bare or `node:`-prefixed, are exempt. No skill script imports a package today.
Limits. Nothing is executed; the check reads bytes and resolves paths. A package warning names the dependency but cannot tell whether the host environment provides it. Resolution assumes CommonJS extension and directory-index candidates for `require` in `.js` and `.cjs`, and the exact specifier for `import()`, static `import`/`from`, and anything in `.mjs`; it does not model conditional exports, a nested `package.json` type field, or import maps. A non-literal `require` is reported, not resolved, so a carrier can still fail at load time through a path computed at runtime. Native discovery, executable mode and actual loading remain unobserved, exactly as before.
Limits. Nothing is executed; the check reads bytes and resolves paths. A package warning names the dependency but cannot tell whether the host environment provides it. Resolution assumes Node's CommonJS candidates for `require` in `.js` and `.cjs` (the path, `.js`, `.json`, then a directory's `index.js` or `index.json`), and the exact specifier for `import()`, static `import`/`from`, and anything in `.mjs`; it does not model conditional exports, a nested `package.json` type field, or import maps. A non-literal `require` is reported, not resolved, so a carrier can still fail at load time through a path computed at runtime. Native discovery, executable mode and actual loading remain unobserved, exactly as before.
The scanner is pinned by twelve edge cases (regex versus division, regex classes and quotes, template interpolations with object braces, block comments, multi-line and re-exported `from`, and `path.join(__dirname, …)`). Across all 655 JavaScript files in the repository it found every relative specifier a plain pattern match finds outside comments and strings, with no scanner errors. The suite validates the real registry once, through the CLI, and shares that receipt between its real-registry cases: 4.8 s per run instead of 9.1 s, which matters because the test job runs in about 33 matrix cells.
@@ -97,4 +97,6 @@ Review follow-up. A third RED added 10 cases from review: spaced calls (`require
Second review follow-up. Review of that head found four ways the gate still passed a carrier that fails at run time, each reproduced before any change: an extensionless `import('./helper')` and a directory `import('./data')` resolved through the CommonJS candidates, although Node's ESM resolver does neither even from a CommonJS file; `require('./lib\\helper.js')` resolved to `lib/helper.js`, although Linux and macOS read the backslash as part of the filename; and in `if (ok) /['"]/.test(text); require('./missing')` the regex after the condition was read as division, so its quote opened a fake string that swallowed the `require`. A fifth RED added 12 cases, 9 failing: the four findings, the two regex conditions, and the `imports` shape. The three controls already passed: an exact `import()`, an extensionless `require`, and division after `a.if(x)` or a call inside a condition. GREEN records which specifiers arrive through `import` and resolves those by exact path. It reverses the earlier backslash normalization: a relative specifier containing a backslash now fails as `non-portable-separator`. It also tracks whether each closing paren ends an `if`, `while`, `for` or `with` condition. Rescanning all 802 repository JavaScript files against the previous scanner changed no result; 18 of them declare `import` specifiers. The real registry still reports 35/35 resolved and zero escapes, warnings or package imports, unchanged on every receipt count.
Focused verification: 22 carrier cases, 12 compiler cases, 45 closure cases, 3 CI cases and 2 publish-surface cases pass with zero failures or skips. ESLint passes on the validator and the new test. The validator is CI-only and stays out of the packed surface, matching `scripts/ci/validate-context-profiles.js`.
Third review follow-up. Review of that head found four more problems, each reproduced first. Two were older: an extensionless `require` still offered `.cjs` and `.mjs` candidates, which Node's CommonJS search never tries; and a regex directly after a block (`if (ok) {} /"/`) was read as division, so its quote swallowed a later `require`. Two had been introduced by the previous fix. The condition check looked only at the character before `if`, so `obj.` + newline + `if(x) / 2` read as a condition and its division as a regex. Exact import resolution also turned a tagged template named `from` into a failing import. A sixth RED added 9 cases, 7 failing. Its two controls passed: `require` resolving to `.json`, and a template passed to `import()` still counting as a dependency. GREEN limits CommonJS candidates to the path, `.js`, `.json` and a directory's `index.js`/`index.json`. It treats `}` as a statement boundary before a regex, and decides a condition from the token before the keyword, ignoring whitespace. Static `from` and side-effect `import` now accept only quoted strings, never template slots. Rescanning all 802 repository JavaScript files against the previous head changed no result. The real registry is unchanged on every receipt count: 35/35 resolved, zero failures or warnings.
Focused verification: 22 carrier cases, 12 compiler cases, 54 closure cases, 3 CI cases and 2 publish-surface cases pass with zero failures or skips. ESLint passes on the validator and the new test. The validator is CI-only and stays out of the packed surface, matching `scripts/ci/validate-context-profiles.js`.