From 475c6d50ba7e8a5b2e45f0d1cc8774b5203b174b Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 10:49:27 +0000 Subject: [PATCH 1/2] fix(devx): extend check:test-source-alias past the package boundary into deps it aliases to source MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Aliasing a workspace dep to source imports that dep's entire import surface into the consumer's resolution domain. The gate's reachability walk stopped at the package boundary — one hop short of the domain it certifies — so a specifier the consumer's alias list demonstrably mangles was never checked. That gap shipped: #7378 added one runtime import of `@objectstack/spec/shared` inside `packages/core/src`, this gate was green, and CI went red in plugin-hono-server (16 test files) and driver-memory (23) with rule 5's own ENOTDIR signature, for a specifier written in no file of either package. The walk now follows an alias into the dep's source and keeps going, exactly as far as the alias chain reaches. Collected specifiers are judged by rule 5 only; the set-equality-audited KNOWN_UNALIASED_TEST_IMPORTS ledger is deliberately untouched by the crossing. Also strips comments before reading imports. Crossing the boundary made this load-bearing: a JSDoc fence in packages/spec/src/index.ts demonstrating subpath imports was read as code and invented ENOTDIR findings against nine configs. That fix also removes a phantom `@objectstack/core` from service-cache's registry entry, narrowing it to its true measured state. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Jqe56GnYFddggeAyfkZFVz --- scripts/check-test-source-alias.mjs | 359 ++++++++++++++++++++++++++-- 1 file changed, 338 insertions(+), 21 deletions(-) diff --git a/scripts/check-test-source-alias.mjs b/scripts/check-test-source-alias.mjs index 5c9302e69d..d6514eba3b 100644 --- a/scripts/check-test-source-alias.mjs +++ b/scripts/check-test-source-alias.mjs @@ -47,9 +47,12 @@ // For every workspace package that has test files: // // 1. Walk the imports REACHABLE FROM ITS TESTS — the test files plus every -// intra-package file they pull in transitively — and collect the workspace -// packages they import **as values**. `import type` is erased before the -// module ever resolves, so it is not a hazard and is not counted. +// file they pull in transitively — and collect the workspace packages they +// import **as values**. `import type` is erased before the module ever +// resolves, so it is not a hazard and is not counted. The walk follows +// relative imports inside the package, and ALSO crosses into a dependency +// the config aliases to source; "Where the walk goes" below says why, and +// exactly what the crossing does and does not feed. // 2. Keep only the deps that can actually go stale: the dep's own entry point // resolves under `dist/`. A dep whose `exports` already point at source // (the example apps' `objectstack.config.ts`) is not an artifact and needs @@ -77,6 +80,62 @@ // file extension. Either anchor the pattern (`/^@objectstack\/core$/`, the // array form) or list the subpath entry ahead of the bare one. // +// ── Where the walk goes, and why it leaves the package (#8351) ────────────── +// +// **Aliasing a workspace dep to source imports that dep's ENTIRE import surface +// into the consumer's resolution domain.** The consumer's own files are then no +// longer the full list of specifiers its config has to resolve correctly — the +// aliased dep's source is loaded by the consumer's Vite, through the consumer's +// alias list, at the consumer's test time. +// +// A walk that stopped at the package boundary stopped one hop short of the +// domain it certifies, and that gap shipped: #7378 added a single runtime +// `import { pluralToSingular } from '@objectstack/spec/shared'` inside +// `packages/core/src/metadata-service-contract.ts`. This gate was GREEN on that +// diff. CI then went red in `@objectstack/plugin-hono-server` (16 test files +// dead at load) and `@objectstack/driver-memory` (23 dead) with rule 5's own +// signature — `ENOTDIR: not a directory, open '…/packages/spec/src/index.ts/ +// shared'` — because both configs alias bare `@objectstack/spec` to a FILE and +// had no `/shared` entry above it. The specifier appears in no file of either +// failing package. It reached them purely because they alias `@objectstack/core` +// to source. Three more configs (`knowledge-ragflow`, `plugin-dev`, +// `knowledge-memory`) were latently identical and stayed green by luck of +// coverage, not by resolution. +// +// So the walk crosses the boundary into any dependency whose specifier THIS +// config resolves to a real source file, and keeps following — the domain +// extends exactly as far as the alias chain does, which is what run time does. +// +// ⚖️ The crossing feeds **rule 5 only** — the ENOTDIR check — never the +// unaliased-artifact ledger in `KNOWN_UNALIASED_TEST_IMPORTS`. That line is not +// squeamishness, it is what the two checks ARE: +// +// - Rule 5 is a correctness verdict on the alias list the config already +// wrote: an entry it declared mangles a specifier that will really be +// resolved. Crossing finds more instances of the config's OWN defect, and +// touches no registry. +// - The ledger is a coverage measurement of which deps a package has not yet +// aliased. Feeding it transitive surface would silently re-scope it from +// "what this package's code imports" to "what it imports plus everything +// every aliased dep's source imports" — re-measuring all 60+ entries of a +// set-equality-audited, SHRINK-ONLY registry as a side effect of a walk +// change. That is a different card, and it must not arrive disguised as +// this one. +// +// Consequences of that line, stated so they are not rediscovered as bugs: a +// specifier collected across the boundary that resolves to `dist/`, or that no +// alias matches at all, is NOT reported here. The staleness half of the +// cross-boundary domain remains unmeasured. +// +// Crossing is deliberately gated on the alias landing on a file that EXISTS. +// An alias into `dist/` is not crossed (there is no source graph there, and the +// artifact need not exist in a bare checkout); neither is one whose replacement +// this file's static reader cannot turn into a real path. Both are silent, and +// both are the fail-open direction — accepted because the alternative is a gate +// that cannot run on a checkout it does not fully understand, and because the +// same configs are already read fail-closed by step 3 for every specifier the +// consumer writes itself. +// // ── The registry, and why it is shaped like this ──────────────────────────── // // `KNOWN_UNALIASED_TEST_IMPORTS` is the measured state of the repo on the day @@ -285,7 +344,10 @@ const KNOWN_UNALIASED_TEST_IMPORTS = { '@objectstack/objectql', '@objectstack/plugin-security', '@objectstack/service-job', '@objectstack/service-messaging', '@objectstack/spec', ], - '@objectstack/service-cache': ['@objectstack/core', '@objectstack/observability'], + // `@objectstack/core` came off when the import reader started stripping + // comments (#8351): this package's only non-`import type` mention of it is a + // JSDoc usage example in `cache-service-plugin.ts`. Prose, not resolution. + '@objectstack/service-cache': ['@objectstack/observability'], '@objectstack/service-cluster': ['@objectstack/spec'], '@objectstack/service-cluster-redis': ['@objectstack/service-cluster'], '@objectstack/service-datasource': [ @@ -410,12 +472,43 @@ const IMPORT_PATTERNS = * Every module specifier the file loads AT RUNTIME, with type-only imports * dropped: `import type { X } from 'y'` and `import { type X } from 'y'` are * erased before anything resolves, so they cannot read a stale artifact. + * + * Comments are stripped first, for the reason `stripComments` was written for + * configs one level down — and this half was measured, not assumed. Source here + * carries long rationale comments that NAME specifiers, and `packages/spec/ + * src/index.ts` opens with a JSDoc code fence demonstrating the subpath import + * styles: + * + * * import * as UI from '@objectstack/spec/ui'; + * + * Read as code, that is an import of `@objectstack/spec/ui` by every consumer + * that aliases `@objectstack/spec` to source — and once the walk crosses the + * package boundary (#8351) it is an ENOTDIR failure reported against nine + * configs, for a specifier no file anywhere actually imports. Documentation is + * not a resolution hazard. + * + * ⚠️ What this still OVER-counts, measured: a namespace or named import with no + * `type` keyword whose bindings are all used in TYPE positions is elided by + * esbuild exactly as `import type` is, and so never resolves either. It is + * counted here anyway — telling them apart needs a type-aware parser, and this + * gate is deliberately a dependency-free text reader (see `asPath`). + * `packages/core/src/qa/adapter.ts` writes `import * as QA from + * '@objectstack/spec/qa'` and uses `QA.` only in annotations; a live probe + * through vitest confirmed `import('@objectstack/core')` loads clean under a + * config with no `/qa` alias, while `import('@objectstack/spec/qa')` under that + * same config raises `ENOTDIR … spec/src/index.ts/qa`. So such a finding is + * LATENT rather than currently-red: the alias really is missing and the config + * really would mangle the specifier — it is one value-use away from breaking, + * and it is being held green by type erasure, not by resolution. Reporting it + * is the fail-closed direction and the remediation (one alias entry) is correct + * either way, but it is not evidence that a suite is failing today. */ function extractRuntimeImports(text) { const specs = []; + const code = stripComments(text); IMPORT_PATTERNS.lastIndex = 0; let match; - while ((match = IMPORT_PATTERNS.exec(text))) { + while ((match = IMPORT_PATTERNS.exec(code))) { const clause = match[1]; const spec = match[2] ?? match[3] ?? match[4] ?? match[5]; if (!spec) continue; @@ -440,7 +533,17 @@ function isTypeOnlyClause(clause) { const RELATIVE_CANDIDATE_SUFFIXES = ['', '.ts', '.tsx', '.mts', '.cts', '.js', '.mjs', '.cjs', '.jsx']; function resolveRelative(fromFile, spec) { - const base = resolve(dirname(fromFile), spec); + return resolveModulePath(dirname(fromFile), spec); +} + +/** + * The file a path fragment lands on, resolved against `baseDir` with the + * extension and `index.*` candidates a bundler would try. Used both for the + * relative imports inside a package and for turning an alias replacement into + * the file a cross-boundary walk continues into. + */ +function resolveModulePath(baseDir, spec) { + const base = resolve(baseDir, spec); const candidates = []; for (const suffix of RELATIVE_CANDIDATE_SUFFIXES) candidates.push(base + suffix); // NodeNext writes `./x.js` for `./x.ts`. @@ -459,22 +562,34 @@ function resolveRelative(fromFile, spec) { /** * Workspace specifiers reachable from this package's tests, following relative - * imports inside the package. Starting at the tests rather than at every file - * in the package matters: a specifier only decides a verdict if a test can - * actually reach it. + * imports inside the package and crossing into any dependency `crossInto` + * reports as aliased to a real source file. Starting at the tests rather than at + * every file in the package matters: a specifier only decides a verdict if a + * test can actually reach it. + * + * The two returned maps are kept apart on purpose — see "Where the walk goes" + * in the header. `imports` is what THIS package's own files write, and is the + * only input to the unaliased-artifact ledger. `crossPackage` is what an aliased + * dependency's source writes, reaching this config's resolution domain through + * the alias; it feeds rule 5 and nothing else. + * + * @param crossInto (spec) => absolute file path to continue into, or null. */ -function testReachableWorkspaceImports(pkg, workspaceNames) { +function testReachableWorkspaceImports(pkg, workspaceNames, crossInto) { const files = walkFiles(pkg.dir); const tests = files.filter((f) => TEST_FILE.test(f)); if (tests.length === 0) return null; const seen = new Set(); - const queue = [...tests]; + /** `via` is null inside this package, else the specifier that crossed out of it. */ + const queue = tests.map((file) => ({ file, via: null })); /** bare package name -> the specifiers actually written (bare and subpath) */ const imports = new Map(); + /** specifier -> the alias of THIS config that pulled its writer into the graph */ + const crossPackage = new Map(); while (queue.length > 0) { - const file = queue.pop(); + const { file, via } = queue.pop(); if (seen.has(file)) continue; seen.add(file); if (!SOURCE_FILE.test(file)) continue; @@ -487,18 +602,29 @@ function testReachableWorkspaceImports(pkg, workspaceNames) { for (const spec of extractRuntimeImports(text)) { if (spec.startsWith('.')) { const resolved = resolveRelative(file, spec); - if (resolved && !seen.has(resolved)) queue.push(resolved); + if (resolved && !seen.has(resolved)) queue.push({ file: resolved, via }); continue; } const scoped = spec.match(/^(@[^/]+\/[^/]+)(?:\/.*)?$/); const bare = scoped ? scoped[1] : spec.split('/')[0]; - if (bare === pkg.name || !workspaceNames.has(bare)) continue; - if (!imports.has(bare)) imports.set(bare, new Set()); - imports.get(bare).add(spec); + if (!workspaceNames.has(bare)) continue; + if (via == null) { + if (bare === pkg.name) continue; + if (!imports.has(bare)) imports.set(bare, new Set()); + imports.get(bare).add(spec); + } else if (!crossPackage.has(spec)) { + crossPackage.set(spec, via); + } + // Aliasing a dep to source puts ITS import surface in this config's + // resolution domain, so the walk follows the alias exactly as far as run + // time does. `via` keeps the first hop: that is the alias the consumer + // actually chose, and the one its diagnostic has to name. + const target = crossInto ? crossInto(spec) : null; + if (target && !seen.has(target)) queue.push({ file: target, via: via ?? spec }); } } - return { testCount: tests.length, imports }; + return { testCount: tests.length, imports, crossPackage }; } // ── vitest config alias reading ───────────────────────────────────────────── @@ -923,6 +1049,33 @@ function pointsAtSource(path) { return /(^|[\\/])src([\\/]|$)/.test(path) && !/(^|[\\/])dist([\\/]|$)/.test(path); } +/** + * The source file this config's aliases really put behind `spec`, or null when + * the walk must not cross there. Null covers four distinct cases, all of them + * deliberate: the specifier is not aliased at all; the alias resolves THROUGH a + * file (rule 5 is already failing it, and there is nothing to read); the alias + * lands on `dist/` (an artifact, not a source graph, and possibly absent in a + * bare checkout); or the replacement does not name a file that exists. + * + * `asPath` returns a FRAGMENT, not an absolute path — by design, since its other + * two callers only ask it for `src`/extension segments. The real configs spell + * their replacements `path.resolve(__dirname, '…')`, so the fragment is relative + * to the config's own directory; the repo root is tried as a second base for the + * `path.resolve(REPO_ROOT, '…')` spelling. A fragment that answers to neither is + * simply not crossed. + */ +function aliasedSourceFile(spec, entries, configDir, root) { + const resolved = resolveThroughAliases(spec, entries); + if (!resolved) return null; + if (THROUGH_A_FILE.test(resolved.result)) return null; + if (!pointsAtSource(resolved.result)) return null; + for (const base of [configDir, root]) { + const file = resolveModulePath(base, resolved.result); + if (file) return file; + } + return null; +} + // ── the scan ──────────────────────────────────────────────────────────────── /** @@ -935,9 +1088,9 @@ function scan(root) { const packages = []; for (const pkg of workspace) { - const reachable = testReachableWorkspaceImports(pkg, names); - if (!reachable) continue; - + // The config is read BEFORE the walk now: the alias list is what decides + // where the walk is allowed to leave the package, so it cannot be read + // after the graph has already been collected. const configPath = findVitestConfig(pkg.dir); let entries = []; let unreadable = null; @@ -950,6 +1103,11 @@ function scan(root) { } } + const crossInto = + entries.length > 0 ? (spec) => aliasedSourceFile(spec, entries, dirname(configPath), root) : null; + const reachable = testReachableWorkspaceImports(pkg, names, crossInto); + if (!reachable) continue; + const unaliased = []; const throughAFile = []; for (const [dep, specs] of [...reachable.imports].sort(([a], [b]) => a.localeCompare(b))) { @@ -962,7 +1120,7 @@ function scan(root) { continue; } if (THROUGH_A_FILE.test(resolved.result)) { - throughAFile.push({ spec, result: resolved.result }); + throughAFile.push({ spec, result: resolved.result, via: null }); continue; } if (!pointsAtSource(resolved.result)) anyUnaliased = true; @@ -970,6 +1128,17 @@ function scan(root) { if (anyUnaliased) unaliased.push(dep); } + // Rule 5 over the specifiers that reached this config's resolution domain + // through its own alias to a dependency's source. Ledger-neutral by + // construction: nothing here can touch `unaliased`. + const alreadyReported = new Set(throughAFile.map((t) => t.spec)); + for (const [spec, via] of [...reachable.crossPackage].sort(([a], [b]) => a.localeCompare(b))) { + if (alreadyReported.has(spec)) continue; + const resolved = resolveThroughAliases(spec, entries); + if (!resolved || !THROUGH_A_FILE.test(resolved.result)) continue; + throughAFile.push({ spec, result: resolved.result, via }); + } + packages.push({ name: pkg.name, rel: pkg.rel, @@ -1015,6 +1184,11 @@ function check(root, registry) { for (const trap of pkg.throughAFile) { failures.push( `${pkg.rel}: alias resolves \`${trap.spec}\` to \`${trap.result}\` — a path THROUGH a file (ENOTDIR at run time).\n` + + (trap.via + ? ` No file of this package writes that specifier. It is reached through this config's own alias\n` + + ` for \`${trap.via}\`, which points at source: aliasing a workspace dep to src imports THAT dep's\n` + + ' import surface into this config\'s resolution domain, and this entry mangles one of them.\n' + : '') + ' The object alias form matches by PREFIX. Anchor the pattern with the array form\n' + ` (\`{ find: /^${escapeForRegexLiteral(trap.spec)}$/, replacement: … }\`) or list the subpath entry BEFORE the bare one.`, ); @@ -1225,6 +1399,105 @@ function buildFixtureTree() { '] } };\n', }); + // ── (11-14) THE CROSS-BOUNDARY SHAPE (#8351) ────────────────────────────── + // + // A reproduction of the #8349 miss: the specifier that dies at run time is + // written in NO file of the failing package. `@fx/relay` is aliased to source + // by the consumer, so relay's own imports are resolved by the CONSUMER's + // alias list — and the consumer's bare `@fx/core` key, whose replacement is a + // FILE, swallows the `@fx/core/logger` that relay's source reaches. + fixture(root, 'packages/relay', { + 'package.json': ARTIFACT_MANIFEST('@fx/relay'), + // The hop through a relative file matters: the real incident reached the + // subpath from `core/src/index.ts` via `metadata-service-contract.ts`, not + // from the entry point itself. + 'src/index.ts': "export * from './inner.js';\n", + 'src/inner.ts': "import { log } from '@fx/core/logger';\nexport const relayed = log;\n", + }); + + // (11) violating: the consumer aliases relay to src, and mangles relay's own + // subpath import. Nothing in this package mentions `@fx/core/logger`. + fixture(root, 'packages/cross-package-trap', { + 'package.json': ARTIFACT_MANIFEST('@fx/cross-package-trap'), + 'src/thing.test.ts': "import { relayed } from '@fx/relay';\nexport default relayed;\n", + 'vitest.config.ts': + "import path from 'path';\nexport default { resolve: { alias: {\n" + + " '@fx/relay': path.resolve(__dirname, '../relay/src/index.ts'),\n" + + " '@fx/core': path.resolve(__dirname, '../core/src/index.ts'),\n" + + '} } };\n', + }); + + // (12) the negative control that keeps (11) honest: the SAME cross-boundary + // reach, with the subpath entry listed ahead of the bare one. Crossing the + // package boundary must not mean reporting everyone who does it. + fixture(root, 'packages/cross-package-compliant', { + 'package.json': ARTIFACT_MANIFEST('@fx/cross-package-compliant'), + 'src/thing.test.ts': "import { relayed } from '@fx/relay';\nexport default relayed;\n", + 'vitest.config.ts': + "import path from 'path';\nexport default { resolve: { alias: {\n" + + " '@fx/relay': path.resolve(__dirname, '../relay/src/index.ts'),\n" + + " '@fx/core/logger': path.resolve(__dirname, '../core/src/logger.ts'),\n" + + " '@fx/core': path.resolve(__dirname, '../core/src/index.ts'),\n" + + '} } };\n', + }); + + // (13) the domain extends as far as the alias chain does: consumer → relay → + // mid → the mangled subpath. A walk that crossed exactly one boundary and + // stopped would miss this the way the package-scoped walk missed #8349. + fixture(root, 'packages/mid', { + 'package.json': ARTIFACT_MANIFEST('@fx/mid'), + 'src/index.ts': "import { log } from '@fx/core/logger';\nexport const mid = log;\n", + }); + fixture(root, 'packages/relay-deep', { + 'package.json': ARTIFACT_MANIFEST('@fx/relay-deep'), + 'src/index.ts': "import { mid } from '@fx/mid';\nexport const deep = mid;\n", + }); + fixture(root, 'packages/cross-package-depth2', { + 'package.json': ARTIFACT_MANIFEST('@fx/cross-package-depth2'), + 'src/thing.test.ts': "import { deep } from '@fx/relay-deep';\nexport default deep;\n", + 'vitest.config.ts': + "import path from 'path';\nexport default { resolve: { alias: {\n" + + " '@fx/relay-deep': path.resolve(__dirname, '../relay-deep/src/index.ts'),\n" + + " '@fx/mid': path.resolve(__dirname, '../mid/src/index.ts'),\n" + + " '@fx/core': path.resolve(__dirname, '../core/src/index.ts'),\n" + + '} } };\n', + }); + + // (14) crossing is GATED on the alias landing on source. This consumer reaches + // relay too, but does not alias it — at run time relay loads from its own + // `dist/`, resolving its imports itself, so the consumer's `@fx/core` key + // never sees `@fx/core/logger`. Reporting it here would be inventing a + // failure. (The unaliased `@fx/relay` is a LEDGER matter, and registered.) + fixture(root, 'packages/no-cross-unaliased', { + 'package.json': ARTIFACT_MANIFEST('@fx/no-cross-unaliased'), + 'src/thing.test.ts': "import { relayed } from '@fx/relay';\nexport default relayed;\n", + 'vitest.config.ts': + "import path from 'path';\nexport default { resolve: { alias: {\n" + + " '@fx/core': path.resolve(__dirname, '../core/src/index.ts'),\n" + + '} } };\n', + }); + + // (15) comments are not imports. The real `packages/spec/src/index.ts` opens + // with a JSDoc fence demonstrating subpath imports; read as code it invented + // ENOTDIR findings against nine configs for a specifier nobody imports. + fixture(root, 'packages/documented', { + 'package.json': ARTIFACT_MANIFEST('@fx/documented'), + 'src/index.ts': + '/**\n * Usage:\n * ```ts\n' + + " * import * as Docs from '@fx/core/docs-only';\n" + + ' * ```\n */\n' + + "export const documented = 1;\n", + }); + fixture(root, 'packages/reads-documented', { + 'package.json': ARTIFACT_MANIFEST('@fx/reads-documented'), + 'src/thing.test.ts': "import { documented } from '@fx/documented';\nexport default documented;\n", + 'vitest.config.ts': + "import path from 'path';\nexport default { resolve: { alias: {\n" + + " '@fx/documented': path.resolve(__dirname, '../documented/src/index.ts'),\n" + + " '@fx/core': path.resolve(__dirname, '../core/src/index.ts'),\n" + + '} } };\n', + }); + return root; } @@ -1326,6 +1599,50 @@ function selfTest() { const sourceDep = check(root, { '@fx/violator': ['@fx/core', '@fx/other'] }); expect(!has(sourceDep.failures, '@fx/consumes-source'), 'a dep that already resolves to source was reported as a hazard'); + // ── the cross-boundary walk (#8351) ─────────────────────────────────── + // Aliasing a dep to source imports ITS import surface into this config's + // resolution domain. Each of these pins one half of that; the registry + // fixture keeps the ledger-only entries quiet so the rule-5 half is read + // against a clean list. + const cross = check(root, { + '@fx/violator': ['@fx/core', '@fx/other'], + '@fx/no-cross-unaliased': ['@fx/relay'], + '@fx/reads-documented': ['@fx/core'], + }); + expect( + cross.failures.some((f) => f.includes('packages/cross-package-trap') && f.includes('@fx/core/logger')), + 'a subpath import reached ONLY through an aliased-to-source dependency was not judged by rule 5 — the #8349 miss', + ); + expect( + cross.failures.some((f) => f.includes('packages/cross-package-trap') && f.includes('reached through this config')), + 'the cross-boundary diagnostic did not say the specifier comes from an aliased dependency rather than this package', + ); + expect( + !has(cross.failures, 'packages/cross-package-compliant'), + 'a config that correctly orders the subpath entry ahead of the bare one was reported anyway', + ); + expect( + cross.failures.some((f) => f.includes('packages/cross-package-depth2') && f.includes('@fx/core/logger')), + 'the walk stopped after one boundary — a mangled subpath two aliased hops out went unseen', + ); + expect( + !cross.failures.some((f) => f.includes('packages/no-cross-unaliased') && f.includes('ENOTDIR')), + 'a dependency this config does NOT alias to source was walked into anyway, inventing a failure', + ); + // The ledger must be untouched by the crossing: rule 5 gets the cross- + // boundary specifiers, `KNOWN_UNALIASED_TEST_IMPORTS` never does. + expect( + !cross.failures.some( + (f) => f.includes('cross-package-trap') && f.includes('with no source alias'), + ), + 'a cross-boundary specifier leaked into the unaliased-artifact ledger, which is registry-audited', + ); + // Comments are prose, not resolution. + expect( + !has(cross.failures, 'docs-only'), + 'a specifier appearing only inside a JSDoc example was read as a real import', + ); + // Census guard: an empty tree is a broken scanner, never a clean repo. const empty = join(tmpdir(), `os-test-source-alias-empty-${process.pid}`); rmSync(empty, { recursive: true, force: true }); From 3a242b7199ce0e376cadf6b1607c277baebcc6ff Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 10:53:04 +0000 Subject: [PATCH 2/2] perf(devx): cache per-file import extraction across the cross-package walk Crossing the boundary walks the same dependency source once per consuming config (nine alias @objectstack/core). Reading and comment-stripping a file are pure functions of it, so memoize per scan. Full-scan: ~8.5s -> ~5.1s against a ~1.6s baseline. Cleared per scan, not per process: --self-test rewrites fixture files between check() calls to exercise registry drift. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Jqe56GnYFddggeAyfkZFVz --- scripts/check-test-source-alias.mjs | 38 +++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/scripts/check-test-source-alias.mjs b/scripts/check-test-source-alias.mjs index d6514eba3b..9ab8d5a965 100644 --- a/scripts/check-test-source-alias.mjs +++ b/scripts/check-test-source-alias.mjs @@ -530,6 +530,32 @@ function isTypeOnlyClause(clause) { return names.length > 0 && names.every((n) => /^type\s/.test(n)); } +/** + * `path -> specifiers`, for the whole process. Crossing the package boundary + * means the SAME dependency source is walked once per consuming config — nine + * of them alias `@objectstack/core` — and both the read and the comment strip + * are pure functions of the file. + * + * Measured on this repo, full scan, three runs each: ~1.6s before this change; + * ~8.5s with the crossing and no cache; ~5.1s with it. What remains is not the + * crossing (~0.5s) but comment stripping every source file (~3.0s), which is + * correctness, not overhead — see `extractRuntimeImports`. + */ +const importCache = new Map(); + +function fileRuntimeImports(file) { + const cached = importCache.get(file); + if (cached) return cached; + let specs; + try { + specs = extractRuntimeImports(readFileSync(file, 'utf8')); + } catch { + specs = []; + } + importCache.set(file, specs); + return specs; +} + const RELATIVE_CANDIDATE_SUFFIXES = ['', '.ts', '.tsx', '.mts', '.cts', '.js', '.mjs', '.cjs', '.jsx']; function resolveRelative(fromFile, spec) { @@ -593,13 +619,7 @@ function testReachableWorkspaceImports(pkg, workspaceNames, crossInto) { if (seen.has(file)) continue; seen.add(file); if (!SOURCE_FILE.test(file)) continue; - let text; - try { - text = readFileSync(file, 'utf8'); - } catch { - continue; - } - for (const spec of extractRuntimeImports(text)) { + for (const spec of fileRuntimeImports(file)) { if (spec.startsWith('.')) { const resolved = resolveRelative(file, spec); if (resolved && !seen.has(resolved)) queue.push({ file: resolved, via }); @@ -1082,6 +1102,10 @@ function aliasedSourceFile(spec, entries, configDir, root) { * @returns {{ packages: Array, artifactPackages: Set, totalPackages: number }} */ function scan(root) { + // Per-scan, not per-process: `--self-test` rewrites fixture files BETWEEN + // `check()` calls to exercise registry drift, and a cache that outlived a + // scan would answer those later passes from the pre-rewrite content. + importCache.clear(); const workspace = listWorkspacePackages(root); const names = new Set(workspace.map((p) => p.name)); const artifactPackages = new Set(workspace.filter((p) => resolvesToArtifact(p.json)).map((p) => p.name));