From ca739b11d8318aa2a044995df9700488150d7236 Mon Sep 17 00:00:00 2001 From: montjeffrey Date: Thu, 8 Oct 2026 15:30:58 +0000 Subject: [PATCH 01/22] test(carriers): pin runtime closure checks --- .../scripts/validate-carrier-closure.test.js | 98 +++++++++++++++++++ 1 file changed, 98 insertions(+) create mode 100644 tests/scripts/validate-carrier-closure.test.js diff --git a/tests/scripts/validate-carrier-closure.test.js b/tests/scripts/validate-carrier-closure.test.js new file mode 100644 index 000000000..2aa0b22c5 --- /dev/null +++ b/tests/scripts/validate-carrier-closure.test.js @@ -0,0 +1,98 @@ +'use strict'; + +const assert = require('node:assert/strict'); +const path = require('node:path'); +const childProcess = require('node:child_process'); +const test = require('node:test'); +const { withFixture, write } = require('../lib/helpers/context-fixture'); + +const REPO_ROOT = path.resolve(__dirname, '../..'); +const SCRIPT_PATH = path.join(REPO_ROOT, 'scripts/ci/validate-carrier-closure.js'); + +function validator() { + return require(SCRIPT_PATH); +} + +function plantSkillScript(root, source) { + write(root, 'skills/ecc-guide/scripts/tool.js', source); +} + +function specifiersOf(failures) { + return failures.map(failure => failure.specifier); +} + +test('the real registry closes: every planned script resolves inside its carrier', () => { + const result = validator().validate(); + assert.equal(result.status, 'success'); + assert.deepEqual(result.failures, []); + assert.equal(result.layouts.length, 5); + assert.deepEqual(result.profiles, ['lean@1', 'full@1']); + assert.ok(result.scriptCount > 0, 'expected planned JavaScript files to inspect'); + assert.ok(result.resolvedCount > 0, 'expected at least one resolved relative specifier'); + assert.ok(result.plannedFileCount > result.scriptCount); +}); + +test('a planted require that leaves the carrier tree fails with its specifier', () => withFixture(root => { + const escape = '../../../../../scripts/lib/context-profile-support'; + plantSkillScript(root, `'use strict';\n\nconst support = require('${escape}');\n\nmodule.exports = { support };\n`); + const result = validator().validate(root); + assert.equal(result.status, 'failure'); + assert.ok(specifiersOf(result.failures).includes(escape)); + assert.ok(result.failures.some(failure => failure.reason === 'outside-carrier-tree')); + assert.ok(result.failures.every(failure => failure.file.endsWith('skills/ecc-guide/scripts/tool.js'))); +})); + +test('a relative require that stays inside the tree but is unplanned fails', () => withFixture(root => { + plantSkillScript(root, "'use strict';\n\nrequire('../../shared/helper');\n"); + const result = validator().validate(root); + assert.equal(result.status, 'failure'); + assert.ok(specifiersOf(result.failures).includes('../../shared/helper')); + assert.ok(result.failures.some(failure => failure.reason === 'unplanned-target')); +})); + +test('a dynamic require warns without failing', () => withFixture(root => { + plantSkillScript(root, "'use strict';\n\nmodule.exports = require(process.env.ECC_PLUGIN_PATH);\n"); + const result = validator().validate(root); + assert.equal(result.status, 'success'); + assert.deepEqual(result.failures, []); + assert.ok(result.warnings.length > 0); + assert.ok(result.warnings.every(warning => warning.expression === 'process.env.ECC_PLUGIN_PATH')); + assert.ok(result.warnings.every(warning => warning.file.endsWith('skills/ecc-guide/scripts/tool.js'))); + assert.equal(result.dynamicCount, result.warnings.length); +})); + +test('a require shape quoted inside a string literal is not a dependency', () => withFixture(root => { + plantSkillScript(root, "'use strict';\n\nconst snippet = \"require('../../../../../etc/passwd')\";\n\nmodule.exports = { snippet };\n"); + const result = validator().validate(root); + assert.equal(result.status, 'success'); + assert.deepEqual(result.warnings, []); +})); + +test('--json emits a machine-readable receipt and exits zero on a clean registry', () => { + const result = childProcess.spawnSync(process.execPath, [SCRIPT_PATH, '--json'], { + cwd: REPO_ROOT, encoding: 'utf8', timeout: 120_000, + }); + assert.equal(result.status, 0, result.stderr); + const receipt = JSON.parse(result.stdout); + assert.equal(receipt.status, 'success'); + assert.equal(receipt.schemaVersion, 'ecc.carrier-closure.v1'); + assert.equal(receipt.projectionCount, receipt.layouts.length * receipt.profiles.length); + assert.deepEqual(receipt.failures, []); + assert.ok(Array.isArray(receipt.warnings)); + assert.ok(Number.isInteger(receipt.scriptCount)); + assert.equal(receipt.loadSmoke, 'deferred'); +}); + +test('unknown flags are rejected', () => { + const result = childProcess.spawnSync(process.execPath, [SCRIPT_PATH, '--write'], { + cwd: REPO_ROOT, encoding: 'utf8', timeout: 120_000, + }); + assert.equal(result.status, 1); + assert.match(result.stderr, /Unknown argument/); +}); + +test('the gate is registered in the normal test workflow', () => { + const { scripts } = require('../../package.json'); + assert.equal(scripts['carrier-closure:check'], 'node scripts/ci/validate-carrier-closure.js'); + assert.ok(scripts.test.includes('validate-carrier-closure.js')); +}); From d3e6ce84cd8f25386230027f6f32b5f16b711df6 Mon Sep 17 00:00:00 2001 From: montjeffrey Date: Thu, 8 Oct 2026 15:35:03 +0000 Subject: [PATCH 02/22] feat(carriers): validate carrier runtime closure in CI --- package.json | 5 +- scripts/ci/validate-carrier-closure.js | 281 +++++++++++++++++++++++++ 2 files changed, 284 insertions(+), 2 deletions(-) create mode 100644 scripts/ci/validate-carrier-closure.js diff --git a/package.json b/package.json index 38c067438..76153fc5d 100644 --- a/package.json +++ b/package.json @@ -487,6 +487,7 @@ "welcome": "echo '\\n ecc-universal installed!\\n Run: ecc typescript\\n Compat: ecc-install typescript\\n Docs: https://github.com/affaan-m/ECC\\n Run or self-host any open-source model.\\n Compute: Itô is the preferred compute sponsor — https://compute.itomarkets.com\\n Any GPU provider works. This sponsorship link is passive: it does not invoke an RFQ, reserve capacity, provision compute, or configure serving.\\n Separately, the opt-in ecc ito find bridge invokes the explicitly configured canonical Itô CLI and submits a live authenticated RFQ; it does not reserve capacity.\\n Managed inference through Itô is not live yet.\\n'", "catalog:check": "node scripts/ci/catalog.js --text", "context-profiles:check": "node scripts/ci/validate-context-profiles.js", + "carrier-closure:check": "node scripts/ci/validate-carrier-closure.js", "catalog:sync": "node scripts/ci/catalog.js --write --text", "command-registry:generate": "node scripts/ci/generate-command-registry.js", "command-registry:write": "node scripts/ci/generate-command-registry.js --write", @@ -510,10 +511,10 @@ "orchestrate:status": "node scripts/orchestration-status.js", "orchestrate:worker": "bash scripts/orchestrate-codex-worker.sh", "orchestrate:tmux": "node scripts/orchestrate-worktrees.js", - "test:validators": "node scripts/ci/check-unicode-safety.js && node scripts/ci/validate-agents.js && node scripts/ci/validate-commands.js && node scripts/ci/validate-rules.js && node scripts/ci/validate-skills.js && node scripts/ci/validate-hooks.js && node scripts/ci/check-hooks-schema-keys.js && node scripts/ci/validate-install-manifests.js && node scripts/ci/validate-context-profiles.js && node scripts/ci/validate-no-personal-paths.js && npm run catalog:check && npm run command-registry:check", + "test:validators": "node scripts/ci/check-unicode-safety.js && node scripts/ci/validate-agents.js && node scripts/ci/validate-commands.js && node scripts/ci/validate-rules.js && node scripts/ci/validate-skills.js && node scripts/ci/validate-hooks.js && node scripts/ci/check-hooks-schema-keys.js && node scripts/ci/validate-install-manifests.js && node scripts/ci/validate-context-profiles.js && node scripts/ci/validate-carrier-closure.js && node scripts/ci/validate-no-personal-paths.js && npm run catalog:check && npm run command-registry:check", "test:js": "node tests/run-all.js", "test:python": "python3 -m pytest tests/ -q", - "test": "node scripts/ci/check-unicode-safety.js && node scripts/ci/validate-agents.js && node scripts/ci/validate-commands.js && node scripts/ci/validate-rules.js && node scripts/ci/validate-skills.js && node scripts/ci/validate-hooks.js && node scripts/ci/check-hooks-schema-keys.js && node scripts/ci/validate-install-manifests.js && node scripts/ci/validate-context-profiles.js && node scripts/ci/validate-no-personal-paths.js && npm run catalog:check && npm run command-registry:check && node tests/run-all.js", + "test": "node scripts/ci/check-unicode-safety.js && node scripts/ci/validate-agents.js && node scripts/ci/validate-commands.js && node scripts/ci/validate-rules.js && node scripts/ci/validate-skills.js && node scripts/ci/validate-hooks.js && node scripts/ci/check-hooks-schema-keys.js && node scripts/ci/validate-install-manifests.js && node scripts/ci/validate-context-profiles.js && node scripts/ci/validate-carrier-closure.js && node scripts/ci/validate-no-personal-paths.js && npm run catalog:check && npm run command-registry:check && node tests/run-all.js", "coverage:js": "c8 --all --include=\"scripts/**/*.js\" --include=\"scripts/**/*.mjs\" --check-coverage --lines 80 --functions 80 --branches 80 --statements 80 --reporter=text --reporter=lcov node tests/run-all.js", "coverage:python": "python3 -m pytest tests/ -q --cov=src --cov-report=term --cov-fail-under=80", "coverage": "c8 --all --include=\"scripts/**/*.js\" --include=\"scripts/**/*.mjs\" --check-coverage --lines 80 --functions 80 --branches 80 --statements 80 --reporter=text --reporter=lcov node tests/run-all.js", diff --git a/scripts/ci/validate-carrier-closure.js b/scripts/ci/validate-carrier-closure.js new file mode 100644 index 000000000..8e0e7a995 --- /dev/null +++ b/scripts/ci/validate-carrier-closure.js @@ -0,0 +1,281 @@ +#!/usr/bin/env node +/** + * Carrier runtime-closure validator. + * + * Carriers are read-only file projections. Nothing here executes a planned + * script: every relative module specifier is extracted statically and checked + * against the planned file set of the carrier it would ship in. + * + * The static require-graph extraction technique is adapted from community + * PR #2788 (affaan-m/ECC). The staged load smoke from that work -- actually + * loading entry scripts from inside a materialized carrier -- is deliberately + * not ported here, and neither is command pruning. + */ + +'use strict'; + +const fs = require('node:fs'); +const path = require('node:path'); +const { planContextCarrier } = require('../lib/context-carriers'); +const { loadContextRegistry } = require('../lib/context-pack-registry'); +const { DEFAULT_REPO_ROOT } = require('../lib/context-profile-support'); + +const PROFILES = Object.freeze(['lean@1', 'full@1']); +const SCRIPT_EXTENSIONS = new Set(['.js', '.cjs', '.mjs']); +const MARK = '\u0000'; +const SLOT = `${MARK}(\\d+)${MARK}`; +const CALL_PATTERN = new RegExp(`\\b(?:require|import)\\(\\s*${SLOT}\\s*\\)`, 'g'); +const FROM_PATTERN = new RegExp(`\\bfrom\\s*${SLOT}`, 'g'); +const SIDE_EFFECT_PATTERN = new RegExp(`\\bimport\\s*${SLOT}`, 'g'); +const DIRNAME_JOIN_PATTERN = new RegExp( + `\\brequire\\(\\s*path\\.join\\(\\s*__dirname\\s*((?:,\\s*${SLOT}\\s*)+)\\)\\s*\\)`, 'g'); +const DYNAMIC_PATTERN = /\b(?:require|import)\(\s*((?:[^()]|\([^()]*\))+)\)/g; +const STATIC_JOIN_PATTERN = new RegExp(`^path\\.join\\(\\s*__dirname\\s*(?:,\\s*${SLOT}\\s*)+\\)$`); +const SLOT_PATTERN = new RegExp(SLOT, 'g'); +const REGEX_PRECEDES = /(?:[([{,;:=!&|?+\-*/%~^<>]|\b(?:return|typeof|case|in|of|new|delete|void|instanceof|do|else|yield|await))$/; + +function readRegexLiteral(source, start) { + let index = start + 1; + let inClass = false; + while (index < source.length) { + const char = source[index]; + if (char === '\\') { index += 2; continue; } + if (char === '\n') return index; + if (char === '[') inClass = true; + else if (char === ']') inClass = false; + else if (char === '/' && !inClass) return index + 1; + index += 1; + } + return index; +} + +/** + * Reduce source to a code skeleton in which every comment is gone and every + * string literal is replaced by an indexed slot. A `require` shape written + * inside a comment, a string, or a regex therefore cannot be mistaken for a + * dependency, while `${...}` interpolations stay code. + */ +function scanSource(rawSource) { + const source = String(rawSource || ''); + const literals = []; + const stack = []; + let code = ''; + let index = 0; + const slot = value => { code += `${MARK}${literals.push(value) - 1}${MARK}`; }; + const frame = () => (stack.length ? stack[stack.length - 1] : null); + + while (index < source.length) { + const char = source[index]; + const next = source[index + 1]; + const open = frame(); + + if (open && open.type === 'template') { + if (char === '\\') { open.text += source.substr(index, 2); index += 2; continue; } + if (char === '`') { stack.pop(); slot(open.text); index += 1; continue; } + if (char === '$' && next === '{') { + slot(open.text); + open.text = ''; + stack.push({ type: 'interpolation', depth: 0 }); + index += 2; + continue; + } + open.text += char; + index += 1; + continue; + } + if (char === '/' && next === '/') { + while (index < source.length && source[index] !== '\n') index += 1; + continue; + } + if (char === '/' && next === '*') { + index = source.indexOf('*/', index + 2); + index = index === -1 ? source.length : index + 2; + continue; + } + if (char === '/' && REGEX_PRECEDES.test(code.trimEnd())) { + index = readRegexLiteral(source, index); + code += ' '; + continue; + } + if (char === '\'' || char === '"') { + let cursor = index + 1; + let value = ''; + while (cursor < source.length && source[cursor] !== char && source[cursor] !== '\n') { + if (source[cursor] === '\\') { value += source.substr(cursor, 2); cursor += 2; continue; } + value += source[cursor]; + cursor += 1; + } + slot(value); + index = source[cursor] === char ? cursor + 1 : cursor; + continue; + } + if (char === '`') { stack.push({ type: 'template', text: '' }); index += 1; continue; } + if (open && open.type === 'interpolation') { + if (char === '{') open.depth += 1; + else if (char === '}') { + if (open.depth === 0) { stack.pop(); index += 1; code += ' '; continue; } + open.depth -= 1; + } + } + code += char; + index += 1; + } + if (frame() && frame().type === 'template') slot(frame().text); + return { code, literals }; +} + +function collect(pattern, code, handler) { + pattern.lastIndex = 0; + let match = pattern.exec(code); + while (match !== null) { + handler(match); + match = pattern.exec(code); + } +} + +function renderExpression(text, literals) { + return text.replace(SLOT_PATTERN, (_whole, slotIndex) => `'${literals[Number(slotIndex)]}'`).trim(); +} + +/** + * Extract the relative module specifiers a script declares, plus the + * non-literal `require`/`import` arguments that cannot be resolved without + * running the code. + */ +function extractReferences(rawSource) { + const { code, literals } = scanSource(rawSource); + const specifiers = new Set(); + const dynamic = new Set(); + const relative = value => typeof value === 'string' && value.startsWith('.'); + const literalAt = match => literals[Number(match[1])]; + + for (const pattern of [CALL_PATTERN, FROM_PATTERN, SIDE_EFFECT_PATTERN]) { + collect(pattern, code, match => { + if (relative(literalAt(match))) specifiers.add(literalAt(match)); + }); + } + collect(DIRNAME_JOIN_PATTERN, code, match => { + const segments = []; + collect(SLOT_PATTERN, match[1], slotMatch => segments.push(literals[Number(slotMatch[1])])); + specifiers.add(`./${segments.join('/')}`); + }); + collect(DYNAMIC_PATTERN, code, match => { + const argument = match[1].trim(); + if (new RegExp(`^${SLOT}$`).test(argument) || STATIC_JOIN_PATTERN.test(argument)) return; + dynamic.add(renderExpression(argument, literals)); + }); + return { specifiers: [...specifiers], dynamic: [...dynamic] }; +} + +function candidateDestinations(destinationPath, specifier) { + const base = path.posix.join(path.posix.dirname(destinationPath), specifier); + if (base === '..' || base.startsWith('../')) return { base, candidates: null }; + const candidates = destinationPath.endsWith('.mjs') + ? [base] + : [base, `${base}.js`, `${base}.cjs`, `${base}.mjs`, `${base}.json`, `${base}/index.js`]; + return { base, candidates }; +} + +function sourceReader(repoRoot) { + const cache = new Map(); + return file => { + if (file.kind === 'generated') return file.content; + if (!cache.has(file.sourcePath)) { + cache.set(file.sourcePath, fs.readFileSync(path.join(repoRoot, ...file.sourcePath.split('/')), 'utf8')); + } + return cache.get(file.sourcePath); + }; +} + +function inspectProjection(carrier, profileId, read, report) { + const planned = new Set(carrier.files.map(file => file.destinationPath)); + const scripts = carrier.files.filter(file => SCRIPT_EXTENSIONS.has(path.posix.extname(file.destinationPath))); + for (const file of scripts) { + const { specifiers, dynamic } = extractReferences(read(file)); + const location = { + target: carrier.target, layout: carrier.layout.id, profileId, + file: file.destinationPath, source: file.sourcePath || null, + }; + for (const specifier of specifiers) { + report.specifierCount += 1; + const { base, candidates } = candidateDestinations(file.destinationPath, specifier); + if (!candidates) { + report.failures.push({ ...location, specifier, resolved: base, reason: 'outside-carrier-tree' }); + } else if (candidates.some(candidate => planned.has(candidate))) { + report.resolvedCount += 1; + } else { + report.failures.push({ ...location, specifier, resolved: base, reason: 'unplanned-target' }); + } + } + for (const expression of dynamic) { + report.dynamicCount += 1; + report.warnings.push({ ...location, expression }); + } + } + report.plannedFileCount += carrier.files.length; + report.scriptCount += scripts.length; +} + +function validate(repoRoot = DEFAULT_REPO_ROOT) { + const registry = loadContextRegistry({ repoRoot }); + const read = sourceReader(repoRoot); + const report = { + plannedFileCount: 0, scriptCount: 0, specifierCount: 0, resolvedCount: 0, dynamicCount: 0, + failures: [], warnings: [], + }; + const layouts = []; + const unsupportedTargets = []; + + for (const target of registry.targets) { + let supported = null; + for (const profileId of PROFILES) { + const carrier = planContextCarrier({ repoRoot, profileId, target }); + if (!carrier.layout) break; + supported = { target, layout: carrier.layout.id, skillRoot: carrier.layout.skillRoot }; + inspectProjection(carrier, profileId, read, report); + } + if (supported) layouts.push(supported); else unsupportedTargets.push(target); + } + + return { + schemaVersion: 'ecc.carrier-closure.v1', + status: report.failures.length ? 'failure' : 'success', + profiles: [...PROFILES], layouts, unsupportedTargets, + projectionCount: layouts.length * PROFILES.length, + registryDigest: registry.registryDigest, + loadSmoke: 'deferred', + lineage: 'static require-graph technique adapted from community PR #2788', + ...report, + }; +} + +function uniqueLines(entries, render) { + return [...new Set(entries.map(render))]; +} + +function main(args = process.argv.slice(2)) { + try { + for (const arg of args) { + if (arg !== '--json') throw new Error(`Unknown argument: ${arg}`); + } + const result = validate(); + if (args.includes('--json')) { + console.log(JSON.stringify(result, null, 2)); + } else { + for (const line of uniqueLines(result.warnings, w => `${w.file} -> ${w.expression}`)) { + console.warn(`Dynamic require, not resolvable statically: ${line}`); + } + for (const line of uniqueLines(result.failures, f => `${f.layout} ${f.profileId}: ${f.file} -> ${f.specifier} (${f.reason})`)) { + console.error(line); + } + console.log(`Carrier closure ${result.status}: ${result.layouts.length} layouts x ${result.profiles.length} profiles = ${result.projectionCount} projections, ${result.plannedFileCount} planned files, ${result.scriptCount} scripts, ${result.resolvedCount}/${result.specifierCount} relative specifiers resolved, ${result.failures.length} escapes, ${result.dynamicCount} dynamic requires. Load smoke: deferred.`); + } + return result.status === 'success' ? 0 : 1; + } catch (error) { + console.error(`Carrier closure validation failed: ${error.message}`); + return 1; + } +} + +if (require.main === module) process.exitCode = main(); +module.exports = { extractReferences, main, scanSource, validate }; From 4814561d1a0797c6547fecd017ebc4fbf4ebd77b Mon Sep 17 00:00:00 2001 From: montjeffrey Date: Thu, 8 Oct 2026 15:36:04 +0000 Subject: [PATCH 03/22] docs(carriers): record the runtime-closure gate slice --- docs/design/context-carriers.tdd.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/docs/design/context-carriers.tdd.md b/docs/design/context-carriers.tdd.md index 8655fee37..2628773ef 100644 --- a/docs/design/context-carriers.tdd.md +++ b/docs/design/context-carriers.tdd.md @@ -76,3 +76,17 @@ Lower-priority review suggestions remain explicit follow-ups: failing projection ## Claims deliberately left unobserved Native discovery, exact native exclusions, invocation, executable-mode needs, workflow outcomes, activation, hook consent, rollback, automatic routing and actual token savings still require their own gates. Schema validation checks shape; it cannot certify supplied artifact semantics. The independently compiled fixture checks exact layout, selection, file set and bytes. It uses a trusted private temporary parent and does not certify an arbitrary-destination transaction writer against hostile concurrent mutation. No native provider, model, container or VM was launched, and no package was published. + +## October 8 static runtime-closure gate + +Date: October 8, 2026. `scripts/ci/validate-carrier-closure.js` now plans every supported carrier layout for both Lean and Full and checks that each planned JavaScript file's relative `require`/`import` specifiers resolve to a file the same carrier plans. The static require-graph extraction technique is adapted from community PR #2788; that work's staged load smoke, which materializes a carrier and actually loads entry scripts from inside it, is deliberately not ported, and neither is command pruning. Both wait for carrier scope to widen beyond skills. + +RED: `tests/scripts/validate-carrier-closure.test.js` added 8 `node:test` cases; all 8 failed, seven because the validator module was absent and the eighth because the npm gate was unregistered. GREEN: all 8 pass. The supported-layout list is derived by planning every registry target and keeping the ones that return a layout, so a new layout is picked up without editing the validator. + +Against the real registry the gate reports 5 supported layouts of 16 declared targets — `claude-plugin@1`, `codex-plugin@1`, `pi-package@1`, `opencode-project@1` and `cursor-project@1` — giving 10 projections and 2,941 planned files. 65 of those planned files are JavaScript: 13 per Full projection and none in Lean. They declare 35 relative specifiers, all 35 resolving inside their own carrier; every one is a sibling `./shared.mjs` import within `skills/ck`. The validator finds zero escapes and zero dynamic requires. + +Zero escapes is the expected result and not evidence the check is idle. Carriers are skill-only today, so no skill script has a reason to reach past its own directory. The gate exists so that the first command, agent, rule or hook admitted into carrier scope cannot silently ship a script reference that the projection does not contain. Detection is pinned by fixtures rather than by the live inventory: a planted skill script requiring `../../../../../scripts/lib/context-profile-support` fails as `outside-carrier-tree` in all five layouts and names the specifier, a planted sibling-skill reference fails as `unplanned-target`, a planted non-literal `require` warns without failing, and a `require` shape quoted inside a string literal produces neither. + +Limits. Nothing is executed; the check reads bytes and resolves paths. Bare specifiers are out of scope, so a skill script that depends on an npm package or a Node builtin is not examined for availability inside a carrier. Resolution assumes CommonJS extension candidates for `.js` and `.cjs` and exact specifiers for `.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. + +Focused verification: 22 carrier cases, 12 compiler cases, 8 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`. From be604c073ab22d418317918e71b96a7081e46cdf Mon Sep 17 00:00:00 2001 From: montjeffrey Date: Thu, 8 Oct 2026 15:46:49 +0000 Subject: [PATCH 04/22] test(carriers): pin scanner edge cases and share one registry run --- .../scripts/validate-carrier-closure.test.js | 63 +++++++++++++------ 1 file changed, 45 insertions(+), 18 deletions(-) diff --git a/tests/scripts/validate-carrier-closure.test.js b/tests/scripts/validate-carrier-closure.test.js index 2aa0b22c5..61dfd07f4 100644 --- a/tests/scripts/validate-carrier-closure.test.js +++ b/tests/scripts/validate-carrier-closure.test.js @@ -13,6 +13,17 @@ function validator() { return require(SCRIPT_PATH); } +let realRun = null; +function realRegistryRun() { + if (!realRun) { + const result = childProcess.spawnSync(process.execPath, [SCRIPT_PATH, '--json'], { + cwd: REPO_ROOT, encoding: 'utf8', timeout: 120_000, + }); + realRun = { status: result.status, stderr: result.stderr, receipt: JSON.parse(result.stdout) }; + } + return realRun; +} + function plantSkillScript(root, source) { write(root, 'skills/ecc-guide/scripts/tool.js', source); } @@ -21,17 +32,6 @@ function specifiersOf(failures) { return failures.map(failure => failure.specifier); } -test('the real registry closes: every planned script resolves inside its carrier', () => { - const result = validator().validate(); - assert.equal(result.status, 'success'); - assert.deepEqual(result.failures, []); - assert.equal(result.layouts.length, 5); - assert.deepEqual(result.profiles, ['lean@1', 'full@1']); - assert.ok(result.scriptCount > 0, 'expected planned JavaScript files to inspect'); - assert.ok(result.resolvedCount > 0, 'expected at least one resolved relative specifier'); - assert.ok(result.plannedFileCount > result.scriptCount); -}); - test('a planted require that leaves the carrier tree fails with its specifier', () => withFixture(root => { const escape = '../../../../../scripts/lib/context-profile-support'; plantSkillScript(root, `'use strict';\n\nconst support = require('${escape}');\n\nmodule.exports = { support };\n`); @@ -69,20 +69,47 @@ test('a require shape quoted inside a string literal is not a dependency', () => })); test('--json emits a machine-readable receipt and exits zero on a clean registry', () => { - const result = childProcess.spawnSync(process.execPath, [SCRIPT_PATH, '--json'], { - cwd: REPO_ROOT, encoding: 'utf8', timeout: 120_000, - }); - assert.equal(result.status, 0, result.stderr); - const receipt = JSON.parse(result.stdout); - assert.equal(receipt.status, 'success'); + const { status, stderr, receipt } = realRegistryRun(); + assert.equal(status, 0, stderr); assert.equal(receipt.schemaVersion, 'ecc.carrier-closure.v1'); assert.equal(receipt.projectionCount, receipt.layouts.length * receipt.profiles.length); - assert.deepEqual(receipt.failures, []); assert.ok(Array.isArray(receipt.warnings)); assert.ok(Number.isInteger(receipt.scriptCount)); assert.equal(receipt.loadSmoke, 'deferred'); }); +test('the real registry closes: every planned script resolves inside its carrier', () => { + const { receipt } = realRegistryRun(); + assert.equal(receipt.status, 'success'); + assert.deepEqual(receipt.failures, []); + assert.equal(receipt.layouts.length, 5); + assert.deepEqual(receipt.profiles, ['lean@1', 'full@1']); + assert.ok(receipt.scriptCount > 0, 'expected planned JavaScript files to inspect'); + assert.ok(receipt.resolvedCount > 0, 'expected at least one resolved relative specifier'); + assert.ok(receipt.plannedFileCount > receipt.scriptCount); +}); + +const SCANNER_CASES = [ + ['division before a require', "const r = a / b; require('./after-division');", ['./after-division']], + ['regex holding a quote', "const re = /it's/; require('./after-regex');", ['./after-regex']], + ['regex holding a slash in a class', "const re = /[/]/g; require('./after-class');", ['./after-class']], + ['regex after return', "function f() { return /x/.test(y); } require('./after-return');", ['./after-return']], + ['division after a closing paren', "const v = (a) / 2 / 3; require('./after-paren');", ['./after-paren']], + ['object braces inside an interpolation', "const s = `a ${ {k: 1}.k } b`; require('./after-template');", ['./after-template']], + ['require text inside a template', "const s = `require('./fake')`;", []], + ['require inside an interpolation', "const s = `${require('./in-interpolation')}`;", ['./in-interpolation']], + ['block comment', "/* require('./commented') */ require('./real');", ['./real']], + ['multi-line import', "import {\n a,\n b\n} from\n './multi';", ['./multi']], + ['re-export', 'export * from "./reexport.js";', ['./reexport.js']], + ['path.join from __dirname', "require(path.join(__dirname, '..', 'lib', 'x.js'));", ['./../lib/x.js']], +]; + +for (const [label, source, expected] of SCANNER_CASES) { + test(`scanner: ${label}`, () => { + assert.deepEqual(validator().extractReferences(source).specifiers.sort(), [...expected].sort()); + }); +} + test('unknown flags are rejected', () => { const result = childProcess.spawnSync(process.execPath, [SCRIPT_PATH, '--write'], { cwd: REPO_ROOT, encoding: 'utf8', timeout: 120_000, From d606375366c370e9cf3eb2994c6ea866eb33f694 Mon Sep 17 00:00:00 2001 From: montjeffrey Date: Thu, 8 Oct 2026 15:47:10 +0000 Subject: [PATCH 05/22] test(carriers): flag absolute and package imports a carrier cannot ship --- .../scripts/validate-carrier-closure.test.js | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/tests/scripts/validate-carrier-closure.test.js b/tests/scripts/validate-carrier-closure.test.js index 61dfd07f4..df3747a00 100644 --- a/tests/scripts/validate-carrier-closure.test.js +++ b/tests/scripts/validate-carrier-closure.test.js @@ -56,11 +56,29 @@ test('a dynamic require warns without failing', () => withFixture(root => { assert.equal(result.status, 'success'); assert.deepEqual(result.failures, []); assert.ok(result.warnings.length > 0); - assert.ok(result.warnings.every(warning => warning.expression === 'process.env.ECC_PLUGIN_PATH')); + assert.ok(result.warnings.every(warning => warning.kind === 'dynamic' && warning.expression === 'process.env.ECC_PLUGIN_PATH')); assert.ok(result.warnings.every(warning => warning.file.endsWith('skills/ecc-guide/scripts/tool.js'))); assert.equal(result.dynamicCount, result.warnings.length); })); +test('an absolute require leaves the carrier tree and fails', () => withFixture(root => { + plantSkillScript(root, "'use strict';\n\nrequire('/opt/shared/helper');\n"); + const result = validator().validate(root); + assert.equal(result.status, 'failure'); + assert.ok(specifiersOf(result.failures).includes('/opt/shared/helper')); + assert.ok(result.failures.every(failure => failure.reason === 'outside-carrier-tree')); +})); + +test('a package import warns because carriers ship no node_modules; builtins do not', () => withFixture(root => { + plantSkillScript(root, "'use strict';\n\nconst yaml = require('js-yaml');\nconst { x } = require('@scope/pkg/deep');\n" + + "const fs = require('node:fs');\nconst path = require('path');\nmodule.exports = { yaml, x, fs, path };\n"); + const result = validator().validate(root); + assert.equal(result.status, 'success'); + const packages = result.warnings.filter(warning => warning.kind === 'package').map(warning => warning.package); + assert.deepEqual([...new Set(packages)].sort(), ['@scope/pkg', 'js-yaml']); + assert.equal(result.packageCount, packages.length); +})); + test('a require shape quoted inside a string literal is not a dependency', () => withFixture(root => { plantSkillScript(root, "'use strict';\n\nconst snippet = \"require('../../../../../etc/passwd')\";\n\nmodule.exports = { snippet };\n"); const result = validator().validate(root); From 9d082716387fc9f439d136120b9aaf66e2b601aa Mon Sep 17 00:00:00 2001 From: montjeffrey Date: Thu, 8 Oct 2026 15:47:34 +0000 Subject: [PATCH 06/22] feat(carriers): flag absolute and package imports a carrier cannot ship --- docs/design/context-carriers.tdd.md | 10 ++++++--- scripts/ci/validate-carrier-closure.js | 31 ++++++++++++++++++-------- 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/docs/design/context-carriers.tdd.md b/docs/design/context-carriers.tdd.md index 2628773ef..1f0d9d675 100644 --- a/docs/design/context-carriers.tdd.md +++ b/docs/design/context-carriers.tdd.md @@ -81,12 +81,16 @@ Native discovery, exact native exclusions, invocation, executable-mode needs, wo Date: October 8, 2026. `scripts/ci/validate-carrier-closure.js` now plans every supported carrier layout for both Lean and Full and checks that each planned JavaScript file's relative `require`/`import` specifiers resolve to a file the same carrier plans. The static require-graph extraction technique is adapted from community PR #2788; that work's staged load smoke, which materializes a carrier and actually loads entry scripts from inside it, is deliberately not ported, and neither is command pruning. Both wait for carrier scope to widen beyond skills. -RED: `tests/scripts/validate-carrier-closure.test.js` added 8 `node:test` cases; all 8 failed, seven because the validator module was absent and the eighth because the npm gate was unregistered. GREEN: all 8 pass. The supported-layout list is derived by planning every registry target and keeping the ones that return a layout, so a new layout is picked up without editing the validator. +RED: `tests/scripts/validate-carrier-closure.test.js` added 8 `node:test` cases; all 8 failed, seven because the validator module was absent and the eighth because the npm gate was unregistered. GREEN: all 8 pass. A second RED for absolute specifiers and package imports failed 3 cases (the absolute path was ignored, packages were ignored, warnings had no kind); GREEN passes them. The supported-layout list is derived by planning every registry target and keeping the ones that return a layout, so a new layout is picked up without editing the validator. Against the real registry the gate reports 5 supported layouts of 16 declared targets — `claude-plugin@1`, `codex-plugin@1`, `pi-package@1`, `opencode-project@1` and `cursor-project@1` — giving 10 projections and 2,941 planned files. 65 of those planned files are JavaScript: 13 per Full projection and none in Lean. They declare 35 relative specifiers, all 35 resolving inside their own carrier; every one is a sibling `./shared.mjs` import within `skills/ck`. The validator finds zero escapes and zero dynamic requires. Zero escapes is the expected result and not evidence the check is idle. Carriers are skill-only today, so no skill script has a reason to reach past its own directory. The gate exists so that the first command, agent, rule or hook admitted into carrier scope cannot silently ship a script reference that the projection does not contain. Detection is pinned by fixtures rather than by the live inventory: a planted skill script requiring `../../../../../scripts/lib/context-profile-support` fails as `outside-carrier-tree` in all five layouts and names the specifier, a planted sibling-skill reference fails as `unplanned-target`, a planted non-literal `require` warns without failing, and a `require` shape quoted inside a string literal produces neither. -Limits. Nothing is executed; the check reads bytes and resolves paths. Bare specifiers are out of scope, so a skill script that depends on an npm package or a Node builtin is not examined for availability inside a carrier. Resolution assumes CommonJS extension candidates for `.js` and `.cjs` and exact specifiers for `.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. +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. -Focused verification: 22 carrier cases, 12 compiler cases, 8 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`. +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 candidates for `.js` and `.cjs` and exact specifiers for `.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. + +Focused verification: 22 carrier cases, 12 compiler cases, 22 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`. diff --git a/scripts/ci/validate-carrier-closure.js b/scripts/ci/validate-carrier-closure.js index 8e0e7a995..8738db360 100644 --- a/scripts/ci/validate-carrier-closure.js +++ b/scripts/ci/validate-carrier-closure.js @@ -16,11 +16,13 @@ const fs = require('node:fs'); const path = require('node:path'); +const { builtinModules } = require('node:module'); const { planContextCarrier } = require('../lib/context-carriers'); const { loadContextRegistry } = require('../lib/context-pack-registry'); const { DEFAULT_REPO_ROOT } = require('../lib/context-profile-support'); const PROFILES = Object.freeze(['lean@1', 'full@1']); +const BUILTINS = new Set(builtinModules); const SCRIPT_EXTENSIONS = new Set(['.js', '.cjs', '.mjs']); const MARK = '\u0000'; const SLOT = `${MARK}(\\d+)${MARK}`; @@ -145,13 +147,17 @@ function renderExpression(text, literals) { function extractReferences(rawSource) { const { code, literals } = scanSource(rawSource); const specifiers = new Set(); + const packages = new Set(); const dynamic = new Set(); - const relative = value => typeof value === 'string' && value.startsWith('.'); const literalAt = match => literals[Number(match[1])]; for (const pattern of [CALL_PATTERN, FROM_PATTERN, SIDE_EFFECT_PATTERN]) { collect(pattern, code, match => { - if (relative(literalAt(match))) specifiers.add(literalAt(match)); + const value = literalAt(match); + if (value.startsWith('.') || value.startsWith('/')) specifiers.add(value); + else if (!value.startsWith('node:') && !BUILTINS.has(value.split('/')[0])) { + packages.add(value.startsWith('@') ? value.split('/').slice(0, 2).join('/') : value.split('/')[0]); + } }); } collect(DIRNAME_JOIN_PATTERN, code, match => { @@ -164,10 +170,11 @@ function extractReferences(rawSource) { if (new RegExp(`^${SLOT}$`).test(argument) || STATIC_JOIN_PATTERN.test(argument)) return; dynamic.add(renderExpression(argument, literals)); }); - return { specifiers: [...specifiers], dynamic: [...dynamic] }; + return { specifiers: [...specifiers], packages: [...packages], dynamic: [...dynamic] }; } function candidateDestinations(destinationPath, specifier) { + if (specifier.startsWith('/')) return { base: specifier, candidates: null }; const base = path.posix.join(path.posix.dirname(destinationPath), specifier); if (base === '..' || base.startsWith('../')) return { base, candidates: null }; const candidates = destinationPath.endsWith('.mjs') @@ -191,7 +198,7 @@ function inspectProjection(carrier, profileId, read, report) { const planned = new Set(carrier.files.map(file => file.destinationPath)); const scripts = carrier.files.filter(file => SCRIPT_EXTENSIONS.has(path.posix.extname(file.destinationPath))); for (const file of scripts) { - const { specifiers, dynamic } = extractReferences(read(file)); + const { specifiers, packages, dynamic } = extractReferences(read(file)); const location = { target: carrier.target, layout: carrier.layout.id, profileId, file: file.destinationPath, source: file.sourcePath || null, @@ -209,7 +216,11 @@ function inspectProjection(carrier, profileId, read, report) { } for (const expression of dynamic) { report.dynamicCount += 1; - report.warnings.push({ ...location, expression }); + report.warnings.push({ ...location, kind: 'dynamic', expression }); + } + for (const name of packages) { + report.packageCount += 1; + report.warnings.push({ ...location, kind: 'package', package: name }); } } report.plannedFileCount += carrier.files.length; @@ -220,7 +231,7 @@ function validate(repoRoot = DEFAULT_REPO_ROOT) { const registry = loadContextRegistry({ repoRoot }); const read = sourceReader(repoRoot); const report = { - plannedFileCount: 0, scriptCount: 0, specifierCount: 0, resolvedCount: 0, dynamicCount: 0, + plannedFileCount: 0, scriptCount: 0, specifierCount: 0, resolvedCount: 0, dynamicCount: 0, packageCount: 0, failures: [], warnings: [], }; const layouts = []; @@ -262,13 +273,15 @@ function main(args = process.argv.slice(2)) { if (args.includes('--json')) { console.log(JSON.stringify(result, null, 2)); } else { - for (const line of uniqueLines(result.warnings, w => `${w.file} -> ${w.expression}`)) { - console.warn(`Dynamic require, not resolvable statically: ${line}`); + for (const line of uniqueLines(result.warnings, w => (w.kind === 'package' + ? `Package import, not shipped in carriers: ${w.file} -> ${w.package}` + : `Dynamic require, not resolvable statically: ${w.file} -> ${w.expression}`))) { + console.warn(line); } for (const line of uniqueLines(result.failures, f => `${f.layout} ${f.profileId}: ${f.file} -> ${f.specifier} (${f.reason})`)) { console.error(line); } - console.log(`Carrier closure ${result.status}: ${result.layouts.length} layouts x ${result.profiles.length} profiles = ${result.projectionCount} projections, ${result.plannedFileCount} planned files, ${result.scriptCount} scripts, ${result.resolvedCount}/${result.specifierCount} relative specifiers resolved, ${result.failures.length} escapes, ${result.dynamicCount} dynamic requires. Load smoke: deferred.`); + console.log(`Carrier closure ${result.status}: ${result.layouts.length} layouts x ${result.profiles.length} profiles = ${result.projectionCount} projections, ${result.plannedFileCount} planned files, ${result.scriptCount} scripts, ${result.resolvedCount}/${result.specifierCount} relative specifiers resolved, ${result.failures.length} escapes, ${result.dynamicCount} dynamic requires, ${result.packageCount} package imports. Load smoke: deferred.`); } return result.status === 'success' ? 0 : 1; } catch (error) { From 4cd7741e747cd7dd029ab1d3f25c0d2a459b38d5 Mon Sep 17 00:00:00 2001 From: montjeffrey Date: Thu, 8 Oct 2026 16:47:54 +0000 Subject: [PATCH 07/22] test(carriers): cover spaced calls, escaped specifiers, Windows paths and index.json --- .../scripts/validate-carrier-closure.test.js | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/scripts/validate-carrier-closure.test.js b/tests/scripts/validate-carrier-closure.test.js index df3747a00..a0ce842aa 100644 --- a/tests/scripts/validate-carrier-closure.test.js +++ b/tests/scripts/validate-carrier-closure.test.js @@ -69,6 +69,24 @@ test('an absolute require leaves the carrier tree and fails', () => withFixture( assert.ok(result.failures.every(failure => failure.reason === 'outside-carrier-tree')); })); +for (const specifier of ['C:/shared/helper', 'C:\\shared\\helper', '\\\\server\\share\\helper', 'file:///opt/helper.js']) { + test(`an absolute Windows or file URL require fails: ${specifier}`, () => withFixture(root => { + plantSkillScript(root, `'use strict';\n\nrequire(${JSON.stringify(specifier)});\n`); + const result = validator().validate(root); + assert.equal(result.status, 'failure'); + assert.ok(result.failures.length > 0); + assert.ok(result.failures.every(failure => failure.reason === 'outside-carrier-tree')); + assert.deepEqual(result.warnings, []); + })); +} + +test('a directory require resolves through index.json', () => withFixture(root => { + write(root, 'skills/ecc-guide/scripts/data/index.json', '{}\n'); + plantSkillScript(root, "'use strict';\n\nmodule.exports = require('./data');\n"); + const result = validator().validate(root); + assert.equal(result.status, 'success', JSON.stringify(result.failures)); +})); + test('a package import warns because carriers ship no node_modules; builtins do not', () => withFixture(root => { plantSkillScript(root, "'use strict';\n\nconst yaml = require('js-yaml');\nconst { x } = require('@scope/pkg/deep');\n" + "const fs = require('node:fs');\nconst path = require('path');\nmodule.exports = { yaml, x, fs, path };\n"); @@ -120,6 +138,12 @@ const SCANNER_CASES = [ ['multi-line import', "import {\n a,\n b\n} from\n './multi';", ['./multi']], ['re-export', 'export * from "./reexport.js";', ['./reexport.js']], ['path.join from __dirname', "require(path.join(__dirname, '..', 'lib', 'x.js'));", ['./../lib/x.js']], + ['whitespace before the call paren', "require ('./spaced'); require\n('./newline'); import ('./spaced-import');", + ['./spaced', './newline', './spaced-import']], + ['whitespace inside path.join from __dirname', "require (path.join (__dirname, 'lib', 'y.js'));", ['./lib/y.js']], + ['unicode escape in a specifier', "require('\\u002e./escaped');", ['../escaped']], + ['hex and braced escapes in a specifier', "require('\\x2e/a'); require(`\\u{2e}/b`);", ['./a', './b']], + ['escaped quote and backslash', "require('./it\\'s'); require('.\\\\win');", ["./it's", '.\\win']], ]; for (const [label, source, expected] of SCANNER_CASES) { From cd83d3ce22e39aed6454078553c9f5525693191b Mon Sep 17 00:00:00 2001 From: montjeffrey Date: Thu, 8 Oct 2026 16:48:33 +0000 Subject: [PATCH 08/22] fix(carriers): match spaced calls, cook escapes, treat Windows and file URLs as absolute --- scripts/ci/validate-carrier-closure.js | 39 ++++++++++++++++++++------ 1 file changed, 30 insertions(+), 9 deletions(-) diff --git a/scripts/ci/validate-carrier-closure.js b/scripts/ci/validate-carrier-closure.js index 8738db360..13f898427 100644 --- a/scripts/ci/validate-carrier-closure.js +++ b/scripts/ci/validate-carrier-closure.js @@ -26,16 +26,37 @@ const BUILTINS = new Set(builtinModules); const SCRIPT_EXTENSIONS = new Set(['.js', '.cjs', '.mjs']); const MARK = '\u0000'; const SLOT = `${MARK}(\\d+)${MARK}`; -const CALL_PATTERN = new RegExp(`\\b(?:require|import)\\(\\s*${SLOT}\\s*\\)`, 'g'); +const CALL_PATTERN = new RegExp(`\\b(?:require|import)\\s*\\(\\s*${SLOT}\\s*\\)`, 'g'); const FROM_PATTERN = new RegExp(`\\bfrom\\s*${SLOT}`, 'g'); const SIDE_EFFECT_PATTERN = new RegExp(`\\bimport\\s*${SLOT}`, 'g'); const DIRNAME_JOIN_PATTERN = new RegExp( - `\\brequire\\(\\s*path\\.join\\(\\s*__dirname\\s*((?:,\\s*${SLOT}\\s*)+)\\)\\s*\\)`, 'g'); -const DYNAMIC_PATTERN = /\b(?:require|import)\(\s*((?:[^()]|\([^()]*\))+)\)/g; -const STATIC_JOIN_PATTERN = new RegExp(`^path\\.join\\(\\s*__dirname\\s*(?:,\\s*${SLOT}\\s*)+\\)$`); + `\\brequire\\s*\\(\\s*path\\.join\\s*\\(\\s*__dirname\\s*((?:,\\s*${SLOT}\\s*)+)\\)\\s*\\)`, 'g'); +const DYNAMIC_PATTERN = /\b(?:require|import)\s*\(\s*((?:[^()]|\([^()]*\))+)\)/g; +const STATIC_JOIN_PATTERN = new RegExp(`^path\\.join\\s*\\(\\s*__dirname\\s*(?:,\\s*${SLOT}\\s*)+\\)$`); const SLOT_PATTERN = new RegExp(SLOT, 'g'); +const ESCAPE_PATTERN = /\\(?:u\{([0-9a-fA-F]+)\}|u([0-9a-fA-F]{4})|x([0-9a-fA-F]{2})|(\r\n|[\s\S]))/g; +const SIMPLE_ESCAPES = Object.freeze({ n: '\n', r: '\r', t: '\t', b: '\b', f: '\f', v: '\v', 0: '\0' }); +const LINE_CONTINUATIONS = new Set(['\n', '\r', '\r\n', '\u2028', '\u2029']); const REGEX_PRECEDES = /(?:[([{,;:=!&|?+\-*/%~^<>]|\b(?:return|typeof|case|in|of|new|delete|void|instanceof|do|else|yield|await))$/; +/** Cook a raw string or template chunk the way the JavaScript parser would. */ +function decodeEscapes(raw) { + return raw.replace(ESCAPE_PATTERN, (whole, braced, unicode, hex, other) => { + if (braced !== undefined) { + const codePoint = parseInt(braced, 16); + return codePoint <= 0x10ffff ? String.fromCodePoint(codePoint) : whole; + } + if (unicode !== undefined) return String.fromCharCode(parseInt(unicode, 16)); + if (hex !== undefined) return String.fromCharCode(parseInt(hex, 16)); + if (LINE_CONTINUATIONS.has(other)) return ''; + return SIMPLE_ESCAPES[other] ?? other; + }); +} + +function isAbsoluteSpecifier(value) { + return value.startsWith('/') || value.startsWith('\\\\') || /^[A-Za-z]:[\\/]/.test(value) || /^file:/i.test(value); +} + function readRegexLiteral(source, start) { let index = start + 1; let inClass = false; @@ -63,7 +84,7 @@ function scanSource(rawSource) { const stack = []; let code = ''; let index = 0; - const slot = value => { code += `${MARK}${literals.push(value) - 1}${MARK}`; }; + const slot = value => { code += `${MARK}${literals.push(decodeEscapes(value)) - 1}${MARK}`; }; const frame = () => (stack.length ? stack[stack.length - 1] : null); while (index < source.length) { @@ -154,7 +175,7 @@ function extractReferences(rawSource) { for (const pattern of [CALL_PATTERN, FROM_PATTERN, SIDE_EFFECT_PATTERN]) { collect(pattern, code, match => { const value = literalAt(match); - if (value.startsWith('.') || value.startsWith('/')) specifiers.add(value); + if (value.startsWith('.') || isAbsoluteSpecifier(value)) specifiers.add(value); else if (!value.startsWith('node:') && !BUILTINS.has(value.split('/')[0])) { packages.add(value.startsWith('@') ? value.split('/').slice(0, 2).join('/') : value.split('/')[0]); } @@ -174,12 +195,12 @@ function extractReferences(rawSource) { } function candidateDestinations(destinationPath, specifier) { - if (specifier.startsWith('/')) return { base: specifier, candidates: null }; - const base = path.posix.join(path.posix.dirname(destinationPath), specifier); + if (isAbsoluteSpecifier(specifier)) return { base: specifier, candidates: null }; + const base = path.posix.join(path.posix.dirname(destinationPath), specifier.replace(/\\/g, '/')); if (base === '..' || base.startsWith('../')) return { base, candidates: null }; const candidates = destinationPath.endsWith('.mjs') ? [base] - : [base, `${base}.js`, `${base}.cjs`, `${base}.mjs`, `${base}.json`, `${base}/index.js`]; + : [base, `${base}.js`, `${base}.cjs`, `${base}.mjs`, `${base}.json`, `${base}/index.js`, `${base}/index.json`]; return { base, candidates }; } From 50d59dd1f95ca444da4b878534ad7b185d17b0d3 Mon Sep 17 00:00:00 2001 From: montjeffrey Date: Thu, 8 Oct 2026 16:48:47 +0000 Subject: [PATCH 09/22] test(carriers): an interpolated from identifier is not an import --- tests/scripts/validate-carrier-closure.test.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/scripts/validate-carrier-closure.test.js b/tests/scripts/validate-carrier-closure.test.js index a0ce842aa..5b660914b 100644 --- a/tests/scripts/validate-carrier-closure.test.js +++ b/tests/scripts/validate-carrier-closure.test.js @@ -152,6 +152,11 @@ for (const [label, source, expected] of SCANNER_CASES) { }); } +test('scanner: an identifier named from before a template chunk is not an import', () => { + assert.deepEqual(validator().extractReferences('const s = `${from} is shipped as ${to}`;'), + { specifiers: [], packages: [], dynamic: [] }); +}); + test('unknown flags are rejected', () => { const result = childProcess.spawnSync(process.execPath, [SCRIPT_PATH, '--write'], { cwd: REPO_ROOT, encoding: 'utf8', timeout: 120_000, From 40e426eafcfaaeba2df5ef112cf705778bf53b1d Mon Sep 17 00:00:00 2001 From: montjeffrey Date: Thu, 8 Oct 2026 16:49:08 +0000 Subject: [PATCH 10/22] fix(carriers): keep keyword patterns from spanning an interpolation --- scripts/ci/validate-carrier-closure.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/scripts/ci/validate-carrier-closure.js b/scripts/ci/validate-carrier-closure.js index 13f898427..6005aed05 100644 --- a/scripts/ci/validate-carrier-closure.js +++ b/scripts/ci/validate-carrier-closure.js @@ -34,6 +34,8 @@ const DIRNAME_JOIN_PATTERN = new RegExp( const DYNAMIC_PATTERN = /\b(?:require|import)\s*\(\s*((?:[^()]|\([^()]*\))+)\)/g; const STATIC_JOIN_PATTERN = new RegExp(`^path\\.join\\s*\\(\\s*__dirname\\s*(?:,\\s*${SLOT}\\s*)+\\)$`); const SLOT_PATTERN = new RegExp(SLOT, 'g'); +// Closes an interpolation; no keyword pattern can match across it. +const CLOSE = `${MARK}${MARK}`; const ESCAPE_PATTERN = /\\(?:u\{([0-9a-fA-F]+)\}|u([0-9a-fA-F]{4})|x([0-9a-fA-F]{2})|(\r\n|[\s\S]))/g; const SIMPLE_ESCAPES = Object.freeze({ n: '\n', r: '\r', t: '\t', b: '\b', f: '\f', v: '\v', 0: '\0' }); const LINE_CONTINUATIONS = new Set(['\n', '\r', '\r\n', '\u2028', '\u2029']); @@ -136,7 +138,7 @@ function scanSource(rawSource) { if (open && open.type === 'interpolation') { if (char === '{') open.depth += 1; else if (char === '}') { - if (open.depth === 0) { stack.pop(); index += 1; code += ' '; continue; } + if (open.depth === 0) { stack.pop(); index += 1; code += CLOSE; continue; } open.depth -= 1; } } @@ -157,7 +159,7 @@ function collect(pattern, code, handler) { } function renderExpression(text, literals) { - return text.replace(SLOT_PATTERN, (_whole, slotIndex) => `'${literals[Number(slotIndex)]}'`).trim(); + return text.replace(SLOT_PATTERN, (_whole, slotIndex) => `'${literals[Number(slotIndex)]}'`).split(CLOSE).join(' ').trim(); } /** From bf729d56ba9632f7e9429a62b6f8f618ef891fdf Mon Sep 17 00:00:00 2001 From: montjeffrey Date: Thu, 8 Oct 2026 16:49:30 +0000 Subject: [PATCH 11/22] docs(carriers): record the closure review follow-up --- docs/design/context-carriers.tdd.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/design/context-carriers.tdd.md b/docs/design/context-carriers.tdd.md index 1f0d9d675..e3df49d81 100644 --- a/docs/design/context-carriers.tdd.md +++ b/docs/design/context-carriers.tdd.md @@ -93,4 +93,6 @@ Limits. Nothing is executed; the check reads bytes and resolves paths. A package 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. -Focused verification: 22 carrier cases, 12 compiler cases, 22 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`. +Review follow-up. A third RED added 10 cases from review: spaced calls (`require ('./x')`, `require\n(...)`, `import (...)`, and `path.join (__dirname, ...)`), escaped specifiers (`\u002e`, `\x2e`, `\u{2e}`, escaped quotes and backslashes), Windows drive, UNC and `file:` URL specifiers, and a directory resolving through `index.json`. All 10 failed. GREEN cooks string and template escapes as the parser does, allows whitespace before call parentheses, treats drive, UNC and `file:` specifiers as absolute, normalizes backslash separators in relative specifiers and adds the `index.json` candidate. A fourth RED/GREEN stops `from` and `import` patterns matching across a closed `${...}` interpolation. Rescanning all 802 repository JavaScript files against the previous scanner changed only three results, each a false package warning removed (`${from}` and similar identifiers before a template chunk). The real registry still reports 35/35 resolved, zero escapes, warnings or package imports, in 3.5 s. + +Focused verification: 22 carrier cases, 12 compiler cases, 33 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`. From df82e75a4c84c4a532d4cf6bde7545375ecaa6f3 Mon Sep 17 00:00:00 2001 From: montjeffrey Date: Fri, 9 Oct 2026 16:12:32 -0400 Subject: [PATCH 12/22] test(carriers): pin exact import resolution, portable separators, and regex after a condition Review on aec97150 found four ways the closure gate passes a carrier that fails at run time. Each is reproduced here against the current head: - import('./helper') with only helper.js planned is accepted, but Node's ESM resolver does no extension search. - import('./data') with only data/index.json (or index.js) planned is accepted, but Node rejects directory imports even from CommonJS. - require('./lib\helper.js') with lib/helper.js planned is accepted because backslashes were normalized, but Linux and macOS read the backslash as part of the filename. - In `if (ok) /['"]/.test(text); require('./missing')` the regex after the condition's closing paren is read as division, its quote opens a fake string, and the require disappears. Controls that must keep passing: an exact import() of a planned file, an extensionless require through the CommonJS search, and division after a method named `if` or a call nested inside a condition. extractReferences gains an `imports` list so the resolver can tell the two forms apart. --- .../scripts/validate-carrier-closure.test.js | 53 ++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/tests/scripts/validate-carrier-closure.test.js b/tests/scripts/validate-carrier-closure.test.js index 5b660914b..22430c0f0 100644 --- a/tests/scripts/validate-carrier-closure.test.js +++ b/tests/scripts/validate-carrier-closure.test.js @@ -87,6 +87,46 @@ test('a directory require resolves through index.json', () => withFixture(root = assert.equal(result.status, 'success', JSON.stringify(result.failures)); })); +// Node resolves import() and static imports by exact path: no extension +// search and no directory index, even when the calling file is CommonJS. +for (const [label, planned, source] of [ + ['an extensionless import()', 'helper.js', "module.exports = import('./helper');\n"], + ['a directory import() through index.json', 'data/index.json', "module.exports = import('./data');\n"], + ['a directory import() through index.js', 'data/index.js', "module.exports = import('./data');\n"], + ['an extensionless static import', 'helper.js', "import helper from './helper';\nexport default helper;\n"], +]) { + test(`${label} fails even when the require form would resolve`, () => withFixture(root => { + write(root, `skills/ecc-guide/scripts/${planned}`, planned.endsWith('.json') ? '{}\n' : 'module.exports = 1;\n'); + plantSkillScript(root, source); + const result = validator().validate(root); + assert.equal(result.status, 'failure'); + assert.ok(result.failures.every(failure => failure.reason === 'unplanned-target')); + })); +} + +test('an import() naming the planned file exactly resolves', () => withFixture(root => { + write(root, 'skills/ecc-guide/scripts/helper.js', 'module.exports = 1;\n'); + plantSkillScript(root, "module.exports = import('./helper.js');\n"); + const result = validator().validate(root); + assert.equal(result.status, 'success', JSON.stringify(result.failures)); +})); + +test('an extensionless require still resolves through the CommonJS extension search', () => withFixture(root => { + write(root, 'skills/ecc-guide/scripts/helper.js', 'module.exports = 1;\n'); + plantSkillScript(root, "module.exports = require('./helper');\n"); + const result = validator().validate(root); + assert.equal(result.status, 'success', JSON.stringify(result.failures)); +})); + +test('a backslash-separated relative require fails: Linux and macOS read it as one filename', () => withFixture(root => { + write(root, 'skills/ecc-guide/scripts/lib/helper.js', 'module.exports = 1;\n'); + plantSkillScript(root, "module.exports = require('./lib\\\\helper.js');\n"); + const result = validator().validate(root); + assert.equal(result.status, 'failure'); + assert.ok(specifiersOf(result.failures).includes('./lib\\helper.js')); + assert.ok(result.failures.every(failure => failure.reason === 'non-portable-separator')); +})); + test('a package import warns because carriers ship no node_modules; builtins do not', () => withFixture(root => { plantSkillScript(root, "'use strict';\n\nconst yaml = require('js-yaml');\nconst { x } = require('@scope/pkg/deep');\n" + "const fs = require('node:fs');\nconst path = require('path');\nmodule.exports = { yaml, x, fs, path };\n"); @@ -144,6 +184,10 @@ const SCANNER_CASES = [ ['unicode escape in a specifier', "require('\\u002e./escaped');", ['../escaped']], ['hex and braced escapes in a specifier', "require('\\x2e/a'); require(`\\u{2e}/b`);", ['./a', './b']], ['escaped quote and backslash', "require('./it\\'s'); require('.\\\\win');", ["./it's", '.\\win']], + ['regex after an if condition', "if (ok) /['\"]/.test(text); require('./missing');", ['./missing']], + ['regex after a while condition', "while (more) /\"/.test(s); require('./after-while');", ['./after-while']], + ['division after a method named if', "const v = a.if(x) / 2, q = \"/\"; require('./after-method');", ['./after-method']], + ['division after a call inside a condition', "if (f(a) / 2 > 1) { q = \"/\"; } require('./after-nested');", ['./after-nested']], ]; for (const [label, source, expected] of SCANNER_CASES) { @@ -154,7 +198,14 @@ for (const [label, source, expected] of SCANNER_CASES) { test('scanner: an identifier named from before a template chunk is not an import', () => { assert.deepEqual(validator().extractReferences('const s = `${from} is shipped as ${to}`;'), - { specifiers: [], packages: [], dynamic: [] }); + { specifiers: [], imports: [], packages: [], dynamic: [] }); +}); + +test('scanner: import() and static imports are reported apart from require', () => { + const { specifiers, imports } = validator().extractReferences( + "require('./a'); import('./b'); import c from './c'; import './d'; require(path.join(__dirname, 'e.js'));"); + assert.deepEqual(specifiers.sort(), ['./a', './b', './c', './d', './e.js']); + assert.deepEqual(imports.sort(), ['./b', './c', './d']); }); test('unknown flags are rejected', () => { From a22c65aa6a898807ca6d2191c5a6b4df3b8355bf Mon Sep 17 00:00:00 2001 From: montjeffrey Date: Fri, 9 Oct 2026 16:15:55 -0400 Subject: [PATCH 13/22] fix(carriers): resolve imports exactly, reject backslash separators, see regex after a condition - Track whether each relative specifier arrives through require or through import()/static import, and resolve the import forms by exact path. Node's ESM resolver adds no extension and rejects directory imports even from a CommonJS file, so import('./helper') and import('./data') no longer pass on helper.js or data/index.json alone. require keeps the CommonJS extension and index candidates. - Stop normalizing backslashes in relative specifiers. Linux and macOS read them as filename characters, so require('./lib\helper.js') now fails as non-portable-separator instead of resolving to lib/helper.js. - Record whether each closing paren ends an if/while/for/with condition, so a `/` right after one starts a regex. A method named `if` and calls nested inside a condition still read as division. extractReferences gains an `imports` list. Rescanning all 802 repository JavaScript files against the previous scanner changes no result, and the real registry still reports 35/35 resolved with zero failures. --- scripts/ci/validate-carrier-closure.js | 67 +++++++++++++++++--------- 1 file changed, 45 insertions(+), 22 deletions(-) diff --git a/scripts/ci/validate-carrier-closure.js b/scripts/ci/validate-carrier-closure.js index 6005aed05..08114284c 100644 --- a/scripts/ci/validate-carrier-closure.js +++ b/scripts/ci/validate-carrier-closure.js @@ -26,7 +26,7 @@ const BUILTINS = new Set(builtinModules); const SCRIPT_EXTENSIONS = new Set(['.js', '.cjs', '.mjs']); const MARK = '\u0000'; const SLOT = `${MARK}(\\d+)${MARK}`; -const CALL_PATTERN = new RegExp(`\\b(?:require|import)\\s*\\(\\s*${SLOT}\\s*\\)`, 'g'); +const CALL_PATTERN = new RegExp(`\\b(require|import)\\s*\\(\\s*${SLOT}\\s*\\)`, 'g'); const FROM_PATTERN = new RegExp(`\\bfrom\\s*${SLOT}`, 'g'); const SIDE_EFFECT_PATTERN = new RegExp(`\\bimport\\s*${SLOT}`, 'g'); const DIRNAME_JOIN_PATTERN = new RegExp( @@ -40,6 +40,8 @@ const ESCAPE_PATTERN = /\\(?:u\{([0-9a-fA-F]+)\}|u([0-9a-fA-F]{4})|x([0-9a-fA-F] const SIMPLE_ESCAPES = Object.freeze({ n: '\n', r: '\r', t: '\t', b: '\b', f: '\f', v: '\v', 0: '\0' }); const LINE_CONTINUATIONS = new Set(['\n', '\r', '\r\n', '\u2028', '\u2029']); const REGEX_PRECEDES = /(?:[([{,;:=!&|?+\-*/%~^<>]|\b(?:return|typeof|case|in|of|new|delete|void|instanceof|do|else|yield|await))$/; +// A `(` opening a statement condition; `a.if(` is a method call, not one. +const CONTROL_OPENS = /(?:^|[^.\w$])(?:if|while|for|with)$/; /** Cook a raw string or template chunk the way the JavaScript parser would. */ function decodeEscapes(raw) { @@ -84,6 +86,10 @@ function scanSource(rawSource) { const source = String(rawSource || ''); const literals = []; const stack = []; + // Whether each open paren began a statement condition, and whether the + // last one closed did: `/` after `if (...)` starts a regex, not division. + const parens = []; + let closedCondition = false; let code = ''; let index = 0; const slot = value => { code += `${MARK}${literals.push(decodeEscapes(value)) - 1}${MARK}`; }; @@ -117,7 +123,7 @@ function scanSource(rawSource) { index = index === -1 ? source.length : index + 2; continue; } - if (char === '/' && REGEX_PRECEDES.test(code.trimEnd())) { + if (char === '/' && (REGEX_PRECEDES.test(code.trimEnd()) || (closedCondition && code.trimEnd().endsWith(')')))) { index = readRegexLiteral(source, index); code += ' '; continue; @@ -142,6 +148,8 @@ function scanSource(rawSource) { open.depth -= 1; } } + if (char === '(') parens.push(CONTROL_OPENS.test(code.trimEnd())); + else if (char === ')') closedCondition = parens.pop() === true; code += char; index += 1; } @@ -165,23 +173,27 @@ function renderExpression(text, literals) { /** * Extract the relative module specifiers a script declares, plus the * non-literal `require`/`import` arguments that cannot be resolved without - * running the code. + * running the code. `imports` lists the specifiers reached through `import()` + * or static `import`/`from`, which Node resolves by exact path. */ function extractReferences(rawSource) { const { code, literals } = scanSource(rawSource); const specifiers = new Set(); + const imports = new Set(); const packages = new Set(); const dynamic = new Set(); - const literalAt = match => literals[Number(match[1])]; - for (const pattern of [CALL_PATTERN, FROM_PATTERN, SIDE_EFFECT_PATTERN]) { - collect(pattern, code, match => { - const value = literalAt(match); - if (value.startsWith('.') || isAbsoluteSpecifier(value)) specifiers.add(value); - else if (!value.startsWith('node:') && !BUILTINS.has(value.split('/')[0])) { - packages.add(value.startsWith('@') ? value.split('/').slice(0, 2).join('/') : value.split('/')[0]); - } - }); + const record = (value, viaImport) => { + if (value.startsWith('.') || isAbsoluteSpecifier(value)) { + specifiers.add(value); + if (viaImport) imports.add(value); + } else if (!value.startsWith('node:') && !BUILTINS.has(value.split('/')[0])) { + packages.add(value.startsWith('@') ? value.split('/').slice(0, 2).join('/') : value.split('/')[0]); + } + }; + collect(CALL_PATTERN, code, match => record(literals[Number(match[2])], match[1] === 'import')); + for (const pattern of [FROM_PATTERN, SIDE_EFFECT_PATTERN]) { + collect(pattern, code, match => record(literals[Number(match[1])], true)); } collect(DIRNAME_JOIN_PATTERN, code, match => { const segments = []; @@ -193,17 +205,25 @@ function extractReferences(rawSource) { if (new RegExp(`^${SLOT}$`).test(argument) || STATIC_JOIN_PATTERN.test(argument)) return; dynamic.add(renderExpression(argument, literals)); }); - return { specifiers: [...specifiers], packages: [...packages], dynamic: [...dynamic] }; + return { specifiers: [...specifiers], imports: [...imports], packages: [...packages], dynamic: [...dynamic] }; } -function candidateDestinations(destinationPath, specifier) { - if (isAbsoluteSpecifier(specifier)) return { base: specifier, candidates: null }; - const base = path.posix.join(path.posix.dirname(destinationPath), specifier.replace(/\\/g, '/')); - if (base === '..' || base.startsWith('../')) return { base, candidates: null }; - const candidates = destinationPath.endsWith('.mjs') +/** + * The planned paths that would satisfy a specifier, or the failure reason + * when none can. CommonJS `require` searches extensions and directory + * indexes; ESM (`import`, or any `.mjs` file) resolves only the exact path. + * A relative backslash is not a separator on Linux or macOS, so it is + * rejected rather than normalized. + */ +function candidateDestinations(destinationPath, specifier, exact) { + if (isAbsoluteSpecifier(specifier)) return { base: specifier, candidates: null, reason: 'outside-carrier-tree' }; + if (specifier.includes('\\')) return { base: specifier, candidates: null, reason: 'non-portable-separator' }; + const base = path.posix.join(path.posix.dirname(destinationPath), specifier); + if (base === '..' || base.startsWith('../')) return { base, candidates: null, reason: 'outside-carrier-tree' }; + const candidates = exact ? [base] : [base, `${base}.js`, `${base}.cjs`, `${base}.mjs`, `${base}.json`, `${base}/index.js`, `${base}/index.json`]; - return { base, candidates }; + return { base, candidates, reason: null }; } function sourceReader(repoRoot) { @@ -221,16 +241,19 @@ function inspectProjection(carrier, profileId, read, report) { const planned = new Set(carrier.files.map(file => file.destinationPath)); const scripts = carrier.files.filter(file => SCRIPT_EXTENSIONS.has(path.posix.extname(file.destinationPath))); for (const file of scripts) { - const { specifiers, packages, dynamic } = extractReferences(read(file)); + const { specifiers, imports, packages, dynamic } = extractReferences(read(file)); + const exactOnly = new Set(imports); + const isModule = file.destinationPath.endsWith('.mjs'); const location = { target: carrier.target, layout: carrier.layout.id, profileId, file: file.destinationPath, source: file.sourcePath || null, }; for (const specifier of specifiers) { report.specifierCount += 1; - const { base, candidates } = candidateDestinations(file.destinationPath, specifier); + const exact = isModule || exactOnly.has(specifier); + const { base, candidates, reason } = candidateDestinations(file.destinationPath, specifier, exact); if (!candidates) { - report.failures.push({ ...location, specifier, resolved: base, reason: 'outside-carrier-tree' }); + report.failures.push({ ...location, specifier, resolved: base, reason }); } else if (candidates.some(candidate => planned.has(candidate))) { report.resolvedCount += 1; } else { From a1d9eeed2f9320dd1c6313d363f4113a170d7528 Mon Sep 17 00:00:00 2001 From: montjeffrey Date: Fri, 9 Oct 2026 16:15:55 -0400 Subject: [PATCH 14/22] docs(carriers): record the second closure review follow-up --- docs/design/context-carriers.tdd.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/design/context-carriers.tdd.md b/docs/design/context-carriers.tdd.md index e3df49d81..f9ef8273b 100644 --- a/docs/design/context-carriers.tdd.md +++ b/docs/design/context-carriers.tdd.md @@ -89,10 +89,12 @@ 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 candidates for `.js` and `.cjs` and exact specifiers for `.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 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. 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. Review follow-up. A third RED added 10 cases from review: spaced calls (`require ('./x')`, `require\n(...)`, `import (...)`, and `path.join (__dirname, ...)`), escaped specifiers (`\u002e`, `\x2e`, `\u{2e}`, escaped quotes and backslashes), Windows drive, UNC and `file:` URL specifiers, and a directory resolving through `index.json`. All 10 failed. GREEN cooks string and template escapes as the parser does, allows whitespace before call parentheses, treats drive, UNC and `file:` specifiers as absolute, normalizes backslash separators in relative specifiers and adds the `index.json` candidate. A fourth RED/GREEN stops `from` and `import` patterns matching across a closed `${...}` interpolation. Rescanning all 802 repository JavaScript files against the previous scanner changed only three results, each a false package warning removed (`${from}` and similar identifiers before a template chunk). The real registry still reports 35/35 resolved, zero escapes, warnings or package imports, in 3.5 s. -Focused verification: 22 carrier cases, 12 compiler cases, 33 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`. +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`. From 21784add5d0a0bc0c138fc5b94ce8c484c461c3a Mon Sep 17 00:00:00 2001 From: montjeffrey Date: Fri, 9 Oct 2026 17:46:02 -0400 Subject: [PATCH 15/22] test(carriers): pin CommonJS require candidates, regex after a block, and non-import from Review on dd14493e found four more ways the closure gate misjudges a carrier, each reproduced against that head: - require('./helper') resolves when only helper.cjs or helper.mjs is planned, but Node's CommonJS search tries only .js, .json and .node. - In `if (ok) {} /"/.test(text); require(...)` the regex after the block is read as division and its quote swallows the require. - `obj.\nif(x) / 2` is a method call, but the condition check only saw the character immediately before `if`, so the division read as a regex and swallowed a later require. Introduced by the previous fix. - A tagged template named `from` was recorded as an import; with exact import resolution it now fails a carrier that never loads that path. Introduced by the previous fix. Controls: an extensionless require still resolves to a .json file, and a template literal passed to import() is still a dependency. --- .../scripts/validate-carrier-closure.test.js | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/scripts/validate-carrier-closure.test.js b/tests/scripts/validate-carrier-closure.test.js index 22430c0f0..5c3c25a04 100644 --- a/tests/scripts/validate-carrier-closure.test.js +++ b/tests/scripts/validate-carrier-closure.test.js @@ -118,6 +118,32 @@ test('an extensionless require still resolves through the CommonJS extension sea assert.equal(result.status, 'success', JSON.stringify(result.failures)); })); +// An extensionless require searches only .js, .json and .node, then a +// directory's index; .cjs and .mjs must be named explicitly. +for (const extension of ['cjs', 'mjs']) { + test(`an extensionless require does not resolve to a .${extension} file`, () => withFixture(root => { + write(root, `skills/ecc-guide/scripts/helper.${extension}`, 'module.exports = 1;\n'); + plantSkillScript(root, "module.exports = require('./helper');\n"); + const result = validator().validate(root); + assert.equal(result.status, 'failure'); + assert.ok(result.failures.every(failure => failure.reason === 'unplanned-target')); + })); +} + +test('an extensionless require resolves to a .json file', () => withFixture(root => { + write(root, 'skills/ecc-guide/scripts/helper.json', '{}\n'); + plantSkillScript(root, "module.exports = require('./helper');\n"); + const result = validator().validate(root); + assert.equal(result.status, 'success', JSON.stringify(result.failures)); +})); + +test('a tagged template named from is not an import', () => withFixture(root => { + plantSkillScript(root, 'const from = parts => parts[0];\nmodule.exports = from`./helper`;\n'); + const result = validator().validate(root); + assert.equal(result.status, 'success', JSON.stringify(result.failures)); + assert.deepEqual(result.warnings, []); +})); + test('a backslash-separated relative require fails: Linux and macOS read it as one filename', () => withFixture(root => { write(root, 'skills/ecc-guide/scripts/lib/helper.js', 'module.exports = 1;\n'); plantSkillScript(root, "module.exports = require('./lib\\\\helper.js');\n"); @@ -188,6 +214,13 @@ const SCANNER_CASES = [ ['regex after a while condition', "while (more) /\"/.test(s); require('./after-while');", ['./after-while']], ['division after a method named if', "const v = a.if(x) / 2, q = \"/\"; require('./after-method');", ['./after-method']], ['division after a call inside a condition', "if (f(a) / 2 > 1) { q = \"/\"; } require('./after-nested');", ['./after-nested']], + ['regex after a block', "if (ok) {} /\"/.test(text); require('./after-block');", ['./after-block']], + ['division after a method named if across a line break', + "const n = obj.\nif(x) / 2, q = \"/\"; require('./after-spaced-method');", ['./after-spaced-method']], + ['division after an optional-chained method named while', + "const n = obj?. while(x) / 2, q = \"/\"; require('./after-optional');", ['./after-optional']], + ['a tagged template named from', 'const from = p => p[0]; const s = from`./helper`;', []], + ['a template import() is still a dependency', 'import(`./templated`);', ['./templated']], ]; for (const [label, source, expected] of SCANNER_CASES) { From 498da59b3c0beaeb7619d9332524af5edcc6156a Mon Sep 17 00:00:00 2001 From: montjeffrey Date: Fri, 9 Oct 2026 17:48:27 -0400 Subject: [PATCH 16/22] fix(carriers): follow Node's CommonJS candidates, see regex after a block, ignore non-import from - An extensionless require now tries only the path, .js, .json, and a directory's index.js/index.json, as Node does; .cjs and .mjs must be named, so require('./helper') no longer passes on helper.cjs alone. - Treat `}` as a statement boundary, so a regex after a block is read as a regex and cannot swallow a later require. - Decide whether a paren opens a condition from the token before the keyword, ignoring whitespace, so `obj.\nif(x) / 2` and `obj?. while(x) / 2` stay method calls followed by division. - Record which slots came from template literals and skip them for static `from` and side-effect `import`, which accept only a quoted string; a tagged template named `from` is no longer an import. A template passed to import() is still a dependency. Rescanning all 802 repository JavaScript files against dd14493e changes no result, and the real registry still reports 35/35 resolved with zero failures or warnings. --- scripts/ci/validate-carrier-closure.js | 49 ++++++++++++++++++-------- 1 file changed, 35 insertions(+), 14 deletions(-) diff --git a/scripts/ci/validate-carrier-closure.js b/scripts/ci/validate-carrier-closure.js index 08114284c..cea037909 100644 --- a/scripts/ci/validate-carrier-closure.js +++ b/scripts/ci/validate-carrier-closure.js @@ -39,9 +39,9 @@ const CLOSE = `${MARK}${MARK}`; const ESCAPE_PATTERN = /\\(?:u\{([0-9a-fA-F]+)\}|u([0-9a-fA-F]{4})|x([0-9a-fA-F]{2})|(\r\n|[\s\S]))/g; const SIMPLE_ESCAPES = Object.freeze({ n: '\n', r: '\r', t: '\t', b: '\b', f: '\f', v: '\v', 0: '\0' }); const LINE_CONTINUATIONS = new Set(['\n', '\r', '\r\n', '\u2028', '\u2029']); -const REGEX_PRECEDES = /(?:[([{,;:=!&|?+\-*/%~^<>]|\b(?:return|typeof|case|in|of|new|delete|void|instanceof|do|else|yield|await))$/; -// A `(` opening a statement condition; `a.if(` is a method call, not one. -const CONTROL_OPENS = /(?:^|[^.\w$])(?:if|while|for|with)$/; +// `}` ends a block, so a slash after it starts a statement: a regex. +const REGEX_PRECEDES = /(?:[([{},;:=!&|?+\-*/%~^<>]|\b(?:return|typeof|case|in|of|new|delete|void|instanceof|do|else|yield|await))$/; +const CONDITION_KEYWORD = /(?:^|[^\w$])(if|while|for|with)$/; /** Cook a raw string or template chunk the way the JavaScript parser would. */ function decodeEscapes(raw) { @@ -61,6 +61,16 @@ function isAbsoluteSpecifier(value) { return value.startsWith('/') || value.startsWith('\\\\') || /^[A-Za-z]:[\\/]/.test(value) || /^file:/i.test(value); } +/** + * Whether code ends with a keyword whose `(` opens a statement condition. + * `obj.if(`, `obj?. while(` and `obj.` + newline + `if(` are method calls, + * so the token before the keyword, ignoring whitespace, must not be a dot. + */ +function opensCondition(code) { + const match = CONDITION_KEYWORD.exec(code); + return Boolean(match) && !code.slice(0, code.length - match[1].length).trimEnd().endsWith('.'); +} + function readRegexLiteral(source, start) { let index = start + 1; let inClass = false; @@ -92,7 +102,13 @@ function scanSource(rawSource) { let closedCondition = false; let code = ''; let index = 0; - const slot = value => { code += `${MARK}${literals.push(decodeEscapes(value)) - 1}${MARK}`; }; + // Slots cut from template literals; `from` and `import` need a quoted string. + const templates = new Set(); + const slot = (value, fromTemplate = false) => { + const slotIndex = literals.push(decodeEscapes(value)) - 1; + if (fromTemplate) templates.add(slotIndex); + code += `${MARK}${slotIndex}${MARK}`; + }; const frame = () => (stack.length ? stack[stack.length - 1] : null); while (index < source.length) { @@ -102,9 +118,9 @@ function scanSource(rawSource) { if (open && open.type === 'template') { if (char === '\\') { open.text += source.substr(index, 2); index += 2; continue; } - if (char === '`') { stack.pop(); slot(open.text); index += 1; continue; } + if (char === '`') { stack.pop(); slot(open.text, true); index += 1; continue; } if (char === '$' && next === '{') { - slot(open.text); + slot(open.text, true); open.text = ''; stack.push({ type: 'interpolation', depth: 0 }); index += 2; @@ -148,13 +164,13 @@ function scanSource(rawSource) { open.depth -= 1; } } - if (char === '(') parens.push(CONTROL_OPENS.test(code.trimEnd())); + if (char === '(') parens.push(opensCondition(code.trimEnd())); else if (char === ')') closedCondition = parens.pop() === true; code += char; index += 1; } - if (frame() && frame().type === 'template') slot(frame().text); - return { code, literals }; + if (frame() && frame().type === 'template') slot(frame().text, true); + return { code, literals, templates }; } function collect(pattern, code, handler) { @@ -177,7 +193,7 @@ function renderExpression(text, literals) { * or static `import`/`from`, which Node resolves by exact path. */ function extractReferences(rawSource) { - const { code, literals } = scanSource(rawSource); + const { code, literals, templates } = scanSource(rawSource); const specifiers = new Set(); const imports = new Set(); const packages = new Set(); @@ -192,8 +208,12 @@ function extractReferences(rawSource) { } }; collect(CALL_PATTERN, code, match => record(literals[Number(match[2])], match[1] === 'import')); + // Static `from` and side-effect `import` take only a quoted string, so a + // template there (a tag named `from`, say) is not an import. for (const pattern of [FROM_PATTERN, SIDE_EFFECT_PATTERN]) { - collect(pattern, code, match => record(literals[Number(match[1])], true)); + collect(pattern, code, match => { + if (!templates.has(Number(match[1]))) record(literals[Number(match[1])], true); + }); } collect(DIRNAME_JOIN_PATTERN, code, match => { const segments = []; @@ -210,8 +230,9 @@ function extractReferences(rawSource) { /** * The planned paths that would satisfy a specifier, or the failure reason - * when none can. CommonJS `require` searches extensions and directory - * indexes; ESM (`import`, or any `.mjs` file) resolves only the exact path. + * when none can. CommonJS `require` tries the path, then `.js` and `.json`, + * then a directory's `index.js` and `index.json` (`.cjs` and `.mjs` must be + * named); ESM (`import`, or any `.mjs` file) resolves only the exact path. * A relative backslash is not a separator on Linux or macOS, so it is * rejected rather than normalized. */ @@ -222,7 +243,7 @@ function candidateDestinations(destinationPath, specifier, exact) { if (base === '..' || base.startsWith('../')) return { base, candidates: null, reason: 'outside-carrier-tree' }; const candidates = exact ? [base] - : [base, `${base}.js`, `${base}.cjs`, `${base}.mjs`, `${base}.json`, `${base}/index.js`, `${base}/index.json`]; + : [base, `${base}.js`, `${base}.json`, `${base}/index.js`, `${base}/index.json`]; return { base, candidates, reason: null }; } From d9fe97171d601447a9c1a7d1b6480985ef9395c8 Mon Sep 17 00:00:00 2001 From: montjeffrey Date: Fri, 9 Oct 2026 17:48:50 -0400 Subject: [PATCH 17/22] docs(carriers): record the third closure review follow-up --- docs/design/context-carriers.tdd.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/design/context-carriers.tdd.md b/docs/design/context-carriers.tdd.md index f9ef8273b..405cb1905 100644 --- a/docs/design/context-carriers.tdd.md +++ b/docs/design/context-carriers.tdd.md @@ -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`. From 7497de02ab4e32eed5adcb2aeca75e1c6458ab5d Mon Sep 17 00:00:00 2001 From: montjeffrey Date: Fri, 9 Oct 2026 17:50:38 -0400 Subject: [PATCH 18/22] test(carriers): pin member calls and deeply nested computed arguments Two first-round review findings were never addressed, and both reproduce on the current head: - records.require('./label'), obj?.import('./x') and obj . require('./y') are method calls, but the call pattern's word boundary is satisfied by the dot, so a self-contained object method fails every projection for a file the script never loads. - The dynamic-argument pattern allows one level of nested parentheses, so require(path.join(__dirname, getName())) produces no dynamic warning while require(getName()) does. Control: a genuine require beside the member calls is still a dependency. --- tests/scripts/validate-carrier-closure.test.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/scripts/validate-carrier-closure.test.js b/tests/scripts/validate-carrier-closure.test.js index 5c3c25a04..568bd342e 100644 --- a/tests/scripts/validate-carrier-closure.test.js +++ b/tests/scripts/validate-carrier-closure.test.js @@ -137,6 +137,13 @@ test('an extensionless require resolves to a .json file', () => withFixture(root assert.equal(result.status, 'success', JSON.stringify(result.failures)); })); +test('an object method named require is not a module load', () => withFixture(root => { + plantSkillScript(root, "const records = { require: name => name };\nmodule.exports = records.require('./label');\n"); + const result = validator().validate(root); + assert.equal(result.status, 'success', JSON.stringify(result.failures)); + assert.deepEqual(result.warnings, []); +})); + test('a tagged template named from is not an import', () => withFixture(root => { plantSkillScript(root, 'const from = parts => parts[0];\nmodule.exports = from`./helper`;\n'); const result = validator().validate(root); @@ -221,8 +228,16 @@ const SCANNER_CASES = [ "const n = obj?. while(x) / 2, q = \"/\"; require('./after-optional');", ['./after-optional']], ['a tagged template named from', 'const from = p => p[0]; const s = from`./helper`;', []], ['a template import() is still a dependency', 'import(`./templated`);', ['./templated']], + ['member calls named require or import', + "r.require('./label'); obj?.import('./x'); obj . require('./y'); obj.\nimport('./z'); require('./real');", ['./real']], ]; +test('scanner: computed arguments nested more than one level deep still warn', () => { + const { dynamic } = validator().extractReferences( + "require(path.join(__dirname, getName())); require(a(b(c()))); obj.require(hidden());"); + assert.deepEqual(dynamic.sort(), ['a(b(c()))', 'path.join(__dirname, getName())']); +}); + for (const [label, source, expected] of SCANNER_CASES) { test(`scanner: ${label}`, () => { assert.deepEqual(validator().extractReferences(source).specifiers.sort(), [...expected].sort()); From a54c8df98b2a9115db80b0ffacb9465543eaf4e4 Mon Sep 17 00:00:00 2001 From: montjeffrey Date: Fri, 9 Oct 2026 17:52:55 -0400 Subject: [PATCH 19/22] fix(carriers): ignore member calls named require or import, and warn on deeply nested arguments - Call patterns now skip a require/import preceded by a dot, with or without whitespace or optional chaining, so records.require('./label') and obj?.import('./x') are method calls rather than module loads. - Replace the one-level dynamic-argument pattern with a balanced-paren scan over the string-free skeleton, so require(path.join(__dirname, getName())) and require(a(b(c()))) are reported as dynamic. Rescanning all 802 repository JavaScript files against the previous head changes 6 results, each a newly reported dynamic argument nested deeper than one level (docker eval checks and one test); no specifier or package result changes. The real registry is unchanged: 35/35 resolved, zero failures or warnings. --- scripts/ci/validate-carrier-closure.js | 36 +++++++++++++++++++++----- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/scripts/ci/validate-carrier-closure.js b/scripts/ci/validate-carrier-closure.js index cea037909..b6e4682db 100644 --- a/scripts/ci/validate-carrier-closure.js +++ b/scripts/ci/validate-carrier-closure.js @@ -26,12 +26,14 @@ const BUILTINS = new Set(builtinModules); const SCRIPT_EXTENSIONS = new Set(['.js', '.cjs', '.mjs']); const MARK = '\u0000'; const SLOT = `${MARK}(\\d+)${MARK}`; -const CALL_PATTERN = new RegExp(`\\b(require|import)\\s*\\(\\s*${SLOT}\\s*\\)`, 'g'); +// `obj.require(`, `obj?.import(` and `obj . require(` are method calls, not module loads. +const NOT_MEMBER = '(? { + const start = match.index + match[0].length; + let depth = 1; + let cursor = start; + while (cursor < code.length && depth > 0) { + if (code[cursor] === '(') depth += 1; + else if (code[cursor] === ')') depth -= 1; + cursor += 1; + } + if (depth === 0) found.push(code.slice(start, cursor - 1).trim()); + }); + return found; +} + function renderExpression(text, literals) { return text.replace(SLOT_PATTERN, (_whole, slotIndex) => `'${literals[Number(slotIndex)]}'`).split(CLOSE).join(' ').trim(); } @@ -220,11 +243,10 @@ function extractReferences(rawSource) { collect(SLOT_PATTERN, match[1], slotMatch => segments.push(literals[Number(slotMatch[1])])); specifiers.add(`./${segments.join('/')}`); }); - collect(DYNAMIC_PATTERN, code, match => { - const argument = match[1].trim(); - if (new RegExp(`^${SLOT}$`).test(argument) || STATIC_JOIN_PATTERN.test(argument)) return; + for (const argument of callArguments(code)) { + if (!argument || new RegExp(`^${SLOT}$`).test(argument) || STATIC_JOIN_PATTERN.test(argument)) continue; dynamic.add(renderExpression(argument, literals)); - }); + } return { specifiers: [...specifiers], imports: [...imports], packages: [...packages], dynamic: [...dynamic] }; } From 79f327f6c90461daf36f0c999ffceaa320cee68e Mon Sep 17 00:00:00 2001 From: montjeffrey Date: Fri, 9 Oct 2026 17:53:06 -0400 Subject: [PATCH 20/22] docs(carriers): record the member-call and nested-argument fixes --- docs/design/context-carriers.tdd.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/design/context-carriers.tdd.md b/docs/design/context-carriers.tdd.md index 405cb1905..76ee4c878 100644 --- a/docs/design/context-carriers.tdd.md +++ b/docs/design/context-carriers.tdd.md @@ -99,4 +99,6 @@ Second review follow-up. Review of that head found four ways the gate still pass 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`. +Two first-round findings had stayed open and still reproduced. A member call such as `records.require('./label')` matched the call pattern, because the dot satisfies a word boundary, so a self-contained object method failed every projection. And the dynamic-argument pattern allowed only one level of nested parentheses, so `require(path.join(__dirname, getName()))` produced no warning while `require(getName())` did. A seventh RED added 3 cases, all failing. GREEN skips calls preceded by a dot, including after whitespace or optional chaining, and finds each call's argument by balanced-paren scanning over the string-free skeleton. Rescanning the 802 files changed 6 results, each a newly reported nested dynamic argument in a docker eval check or a test, none of them in a carrier. The real registry is unchanged. + +Focused verification: 22 carrier cases, 12 compiler cases, 57 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`. From 56d4ff433134b6ab220edf0186af9cf9c0f7c396 Mon Sep 17 00:00:00 2001 From: montjeffrey Date: Sat, 10 Oct 2026 12:50:29 -0400 Subject: [PATCH 21/22] test(carriers): pin object-literal division, spread requires and extra arguments Three open review findings still pass the gate with a missing dependency, each reproduced against the real registry: - const ratio = {} / 2; require('../x') -- `}` is treated as a statement boundary, so the division reads as a regex that swallows the require. a.of / 2 does the same through the keyword list. - [...require('../x')] -- the member-call guard reads the last dot of the spread as member access and skips the call. - require('../x', null) -- a literal counts only when the call closes right after it, so the specifier is demoted to a dynamic warning, which never fails. 14 cases, 11 failing. Controls that already pass: a regex after a function body, a class body and a nested block. --- .../scripts/validate-carrier-closure.test.js | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/tests/scripts/validate-carrier-closure.test.js b/tests/scripts/validate-carrier-closure.test.js index 568bd342e..56d624add 100644 --- a/tests/scripts/validate-carrier-closure.test.js +++ b/tests/scripts/validate-carrier-closure.test.js @@ -230,8 +230,51 @@ const SCANNER_CASES = [ ['a template import() is still a dependency', 'import(`./templated`);', ['./templated']], ['member calls named require or import', "r.require('./label'); obj?.import('./x'); obj . require('./y'); obj.\nimport('./z'); require('./real');", ['./real']], + ['division after an object literal', "const ratio = {} / 2; require('../after-object');", ['../after-object']], + ['division after a nested object literal', "const v = { a: { b: 1 } } / 2, q = \"/\"; require('./after-nested-object');", + ['./after-nested-object']], + ['division after an object literal inside an interpolation', + "const s = `${ {} / 2 }`, q = \"/\"; require('./after-interpolated-object');", ['./after-interpolated-object']], + ['regex after a function body', "function f() {} /\"/.test(s); require('./after-function');", ['./after-function']], + ['regex after a class body', "class A {} /\"/.test(s); require('./after-class-body');", ['./after-class-body']], + ['regex after a nested block', "if (a) { if (b) {} } /\"/.test(s); require('./after-nested-block');", + ['./after-nested-block']], + ['division after a member named of', "const n = a.of / 2, q = \"/\"; require('./after-member-of');", + ['./after-member-of']], + ['spread of a require', "const all = [...require('../spread-array')]; f(... require('./spread-arg'));", + ['../spread-array', './spread-arg']], + ['a literal require with extra arguments', "require('../extra-arg', null); require(path.join(__dirname, 'j.js'), 0);", + ['../extra-arg', './j.js']], ]; +test('scanner: an import() with an options argument is an exact-path import, not a dynamic warning', () => { + const { specifiers, imports, dynamic } = validator().extractReferences( + "import('./data.json', { with: { type: 'json' } }); require('./plain', null);"); + assert.deepEqual(specifiers.sort(), ['./data.json', './plain']); + assert.deepEqual(imports, ['./data.json']); + assert.deepEqual(dynamic, []); +}); + +test('scanner: a computed first argument still warns when more arguments follow', () => { + assert.deepEqual(validator().extractReferences("require(name, null); require('./a' + b, c);").dynamic.sort(), + ["'./a' + b", 'name']); +}); + +// Each of these shapes once passed the gate with its dependency missing. +for (const [label, line] of [ + ['division after an object literal', "const ratio = {} / 2; require('../../shared/helper');"], + ['a spread require', "module.exports = [...require('../../shared/helper')];"], + ['a require with an extra argument', "module.exports = require('../../shared/helper', null);"], +]) { + test(`an unplanned require hidden by ${label} still fails`, () => withFixture(root => { + plantSkillScript(root, `'use strict';\n\n${line}\n`); + const result = validator().validate(root); + assert.equal(result.status, 'failure'); + assert.ok(specifiersOf(result.failures).includes('../../shared/helper'), JSON.stringify(result.failures)); + assert.ok(result.failures.every(failure => failure.reason === 'unplanned-target')); + })); +} + test('scanner: computed arguments nested more than one level deep still warn', () => { const { dynamic } = validator().extractReferences( "require(path.join(__dirname, getName())); require(a(b(c()))); obj.require(hidden());"); From f7bc750326a40bca31fa4ff4ef7337c7b480de4c Mon Sep 17 00:00:00 2001 From: montjeffrey Date: Sat, 10 Oct 2026 12:50:29 -0400 Subject: [PATCH 22/22] fix(carriers): close the object-literal, spread and extra-argument false negatives - Record whether each `{` opens a block or an object literal from the token before it; only a block's `}` lets a regex follow, so `{} / 2` divides again while `if (ok) {} /re/` stays a regex. - Match regex-preceding keywords only when no member dot precedes them, so `a.of / 2` divides. - Exempt the last dot of a spread from the member-call guard, so `...require('./x')` is a module load. - Resolve a call's first argument up to its first top-level comma, so `require('./x', null)` and `import('./x.json', { with })` name `./x`. `import(path.join(__dirname, ...))`, previously skipped silently, now warns as dynamic. Rescanning all 802 repository JavaScript files against the previous head changes no result. The real registry is unchanged: 35/35 resolved, zero failures or warnings. The design doc records the fix and states the gate's scope: static relative specifiers in planned JavaScript only. --- docs/design/context-carriers.tdd.md | 4 ++ scripts/ci/validate-carrier-closure.js | 88 ++++++++++++++++++-------- 2 files changed, 67 insertions(+), 25 deletions(-) diff --git a/docs/design/context-carriers.tdd.md b/docs/design/context-carriers.tdd.md index 76ee4c878..65587dccb 100644 --- a/docs/design/context-carriers.tdd.md +++ b/docs/design/context-carriers.tdd.md @@ -102,3 +102,7 @@ Third review follow-up. Review of that head found four more problems, each repro Two first-round findings had stayed open and still reproduced. A member call such as `records.require('./label')` matched the call pattern, because the dot satisfies a word boundary, so a self-contained object method failed every projection. And the dynamic-argument pattern allowed only one level of nested parentheses, so `require(path.join(__dirname, getName()))` produced no warning while `require(getName())` did. A seventh RED added 3 cases, all failing. GREEN skips calls preceded by a dot, including after whitespace or optional chaining, and finds each call's argument by balanced-paren scanning over the string-free skeleton. Rescanning the 802 files changed 6 results, each a newly reported nested dynamic argument in a docker eval check or a test, none of them in a carrier. The real registry is unchanged. Focused verification: 22 carrier cases, 12 compiler cases, 57 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`. + +Fourth review follow-up. Three open review findings still let a carrier with a missing dependency pass the gate, each reproduced against the real registry first. Treating every `}` as a statement boundary, the third follow-up's fix, also caught object literals, so in `const ratio = {} / 2; require('../x')` the division was read as a regex that swallowed the `require`; a member named `of` (`a.of / 2`) did the same through the keyword list. The member-call guard treated the last dot of a spread as member access, so `[...require('../x')]` was skipped. And a literal first argument counted only when the call closed right after it, so `require('../x', null)` was demoted to a dynamic warning, which never fails. An eighth RED added 14 cases, 11 failing; the three controls (regex after a function body, a class body and a nested block) already passed. GREEN records whether each `{` opens a block or an object literal from the token before it, and only a block's `}` lets a regex follow. It matches keywords only when no member dot precedes them, ignores the dot of a spread, and resolves a call's first argument up to its first top-level comma, so `import('./data.json', { with: { type: 'json' } })` is an exact-path import, not a dynamic warning. As a side effect, `import(path.join(__dirname, …))`, which used to be skipped silently, now warns as dynamic, because `import()` takes a URL, not a path. Rescanning all 802 repository JavaScript files against the previous head changed no result. The real registry is unchanged: 35/35 resolved, zero failures or warnings. 71 closure cases pass. + +Scope. The gate checks static, relative module specifiers in planned JavaScript files, and nothing else. It does not see `require.resolve`, an aliased or `createRequire`-made `require`, files a script spawns or reads at run time, or `.sh` and `.py` scripts. It does not model `.node` addons or a directory's `package.json` `main`. Each of those can still ship a carrier that fails at load time. diff --git a/scripts/ci/validate-carrier-closure.js b/scripts/ci/validate-carrier-closure.js index b6e4682db..5c781ba7e 100644 --- a/scripts/ci/validate-carrier-closure.js +++ b/scripts/ci/validate-carrier-closure.js @@ -26,23 +26,29 @@ const BUILTINS = new Set(builtinModules); const SCRIPT_EXTENSIONS = new Set(['.js', '.cjs', '.mjs']); const MARK = '\u0000'; const SLOT = `${MARK}(\\d+)${MARK}`; -// `obj.require(`, `obj?.import(` and `obj . require(` are method calls, not module loads. -const NOT_MEMBER = '(?]|\b(?:return|typeof|case|in|of|new|delete|void|instanceof|do|else|yield|await))$/; +// A keyword, not a member of that name: `a.of / 2` divides. +const KEYWORD = `${NOT_MEMBER}(?]|${KEYWORD}(?:return|typeof|case|in|of|new|delete|void|instanceof|do|else|yield|await))$`); +// After these a `{` opens an object literal; after any other word it opens a block. +const OBJECT_KEYWORD = new RegExp( + `${KEYWORD}(?:return|typeof|case|in|of|new|delete|void|instanceof|yield|await)$`); +const CASE_LABEL = new RegExp(`${KEYWORD}(?:case\\b[^;{}]*|default\\s*):$`); const CONDITION_KEYWORD = /(?:^|[^\w$])(if|while|for|with)$/; /** Cook a raw string or template chunk the way the JavaScript parser would. */ @@ -73,6 +79,21 @@ function opensCondition(code) { return Boolean(match) && !code.slice(0, code.length - match[1].length).trimEnd().endsWith('.'); } +/** + * Whether a `{` after code opens a block (a slash after its `}` starts a + * regex) or an object literal (a slash after its `}` divides). `blocks` holds + * the kind of every enclosing brace, innermost last. + */ +function opensBlock(code, blocks) { + if (!code || code.endsWith('=>')) return true; + const last = code[code.length - 1]; + if (last === ')' || last === ';' || last === '}') return true; + if (last === '{') return blocks[blocks.length - 1] !== false; + if (last === ':') return blocks[blocks.length - 1] !== false && CASE_LABEL.test(code); + if (/[\w$]/.test(last)) return !OBJECT_KEYWORD.test(code); + return false; +} + function readRegexLiteral(source, start) { let index = start + 1; let inClass = false; @@ -102,6 +123,10 @@ function scanSource(rawSource) { // last one closed did: `/` after `if (...)` starts a regex, not division. const parens = []; let closedCondition = false; + // The same for braces: whether each open one began a block, and whether the + // last one closed did: `/` after `{}` divides, after `if (x) {}` it does not. + const blocks = []; + let closedBlock = false; let code = ''; let index = 0; // Slots cut from template literals; `from` and `import` need a quoted string. @@ -141,7 +166,9 @@ function scanSource(rawSource) { index = index === -1 ? source.length : index + 2; continue; } - if (char === '/' && (REGEX_PRECEDES.test(code.trimEnd()) || (closedCondition && code.trimEnd().endsWith(')')))) { + if (char === '/' && (REGEX_PRECEDES.test(code.trimEnd()) + || (closedCondition && code.trimEnd().endsWith(')')) + || (closedBlock && code.trimEnd().endsWith('}')))) { index = readRegexLiteral(source, index); code += ' '; continue; @@ -168,6 +195,8 @@ function scanSource(rawSource) { } if (char === '(') parens.push(opensCondition(code.trimEnd())); else if (char === ')') closedCondition = parens.pop() === true; + else if (char === '{') blocks.push(opensBlock(code.trimEnd(), blocks)); + else if (char === '}') closedBlock = blocks.pop() !== false; code += char; index += 1; } @@ -185,9 +214,11 @@ function collect(pattern, code, handler) { } /** - * The argument text of every `require(...)`/`import(...)` call, matched to - * its closing paren at any nesting depth. Strings are already slots, so a - * paren inside one cannot unbalance the count. + * The first argument of every `require(...)`/`import(...)` call, matched to + * its closing paren at any nesting depth and cut at the first top-level + * comma: Node resolves only the first argument, so `require('./x', null)` and + * `import('./x.json', { with })` still name `./x`. Strings are already slots, + * so a paren or comma inside one cannot unbalance the count. */ function callArguments(code) { const found = []; @@ -195,12 +226,15 @@ function callArguments(code) { const start = match.index + match[0].length; let depth = 1; let cursor = start; + let end = null; while (cursor < code.length && depth > 0) { - if (code[cursor] === '(') depth += 1; - else if (code[cursor] === ')') depth -= 1; + const char = code[cursor]; + if (char === '(' || char === '[' || char === '{') depth += 1; + else if (char === ')' || char === ']' || char === '}') depth -= 1; + else if (char === ',' && depth === 1 && end === null) end = cursor; cursor += 1; } - if (depth === 0) found.push(code.slice(start, cursor - 1).trim()); + if (depth === 0) found.push({ keyword: match[1], argument: code.slice(start, end ?? cursor - 1).trim() }); }); return found; } @@ -230,7 +264,6 @@ function extractReferences(rawSource) { packages.add(value.startsWith('@') ? value.split('/').slice(0, 2).join('/') : value.split('/')[0]); } }; - collect(CALL_PATTERN, code, match => record(literals[Number(match[2])], match[1] === 'import')); // Static `from` and side-effect `import` take only a quoted string, so a // template there (a tag named `from`, say) is not an import. for (const pattern of [FROM_PATTERN, SIDE_EFFECT_PATTERN]) { @@ -238,14 +271,19 @@ function extractReferences(rawSource) { if (!templates.has(Number(match[1]))) record(literals[Number(match[1])], true); }); } - collect(DIRNAME_JOIN_PATTERN, code, match => { - const segments = []; - collect(SLOT_PATTERN, match[1], slotMatch => segments.push(literals[Number(slotMatch[1])])); - specifiers.add(`./${segments.join('/')}`); - }); - for (const argument of callArguments(code)) { - if (!argument || new RegExp(`^${SLOT}$`).test(argument) || STATIC_JOIN_PATTERN.test(argument)) continue; - dynamic.add(renderExpression(argument, literals)); + for (const { keyword, argument } of callArguments(code)) { + if (!argument) continue; + const literal = LITERAL_PATTERN.exec(argument); + const join = keyword === 'require' && STATIC_JOIN_PATTERN.exec(argument); + if (literal) { + record(literals[Number(literal[1])], keyword === 'import'); + } else if (join) { + const segments = []; + collect(SLOT_PATTERN, join[1], slotMatch => segments.push(literals[Number(slotMatch[1])])); + specifiers.add(`./${segments.join('/')}`); + } else { + dynamic.add(renderExpression(argument, literals)); + } } return { specifiers: [...specifiers], imports: [...imports], packages: [...packages], dynamic: [...dynamic] }; }