From 6abf02bb499d57ccecb0edd81bec48ff1f17d98d Mon Sep 17 00:00:00 2001 From: montjeffrey Date: Thu, 8 Oct 2026 15:47:34 +0000 Subject: [PATCH] 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) {