GitNexus/gitnexus/test/unit/node-workspace-scope.test.ts
Gergő Magyar 28187bb3a7
Some checks are pending
CodeQL / Analyze (javascript-typescript) (push) Waiting to run
CodeQL / Analyze (python) (push) Waiting to run
Gitleaks / gitleaks (push) Waiting to run
Publish / Classify release event (push) Waiting to run
Publish / RC guard (marker + release-PR skip) (push) Blocked by required conditions
Publish / ci (push) Blocked by required conditions
Publish / Publish to npm (push) Blocked by required conditions
Publish / Build & Push RC Docker images (push) Blocked by required conditions
Scorecard / Scorecard analysis (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-cli) (push) Waiting to run
Trivy Image Scan / Trivy (gitnexus-web) (push) Waiting to run
fix(typescript): resolve imports against declared config, not path suffixes (#2953) (#2956)
* fix(typescript): resolve imports against declared config, not path suffixes (#2953)

TypeScript/JavaScript/Vue import resolution ended in `suffixResolve`, which
answers "does any file in this repo have a path ending in this specifier?" and
answers it by dropping leading segments until something matches. That is not
module resolution, and it failed in both directions at once:

  - `@acme/telemetry/nest`, a registry dependency with no in-repo file, landed
    on the repo's only path ending in `nest/index.ts` — a false IMPORTS edge at
    confidence 1.0, indistinguishable downstream from a real one. The reporter
    measured 44 of 74 `apps/ -> packages/` edges landing on two such files.
  - `@repo/utils`, a first-party workspace package, resolved to nothing: its
    name lives in `packages/utils/package.json` and appears in no file path, so
    a path matcher cannot find it. Zero CALLS from 75 import statements.

Both come from the same missing input — nothing read the config that says what
exists — so both are fixed by reading it.

Replaces the suffix matcher on this path with the algorithm tsc and Node
actually run, in their order: relative/absolute, `#imports`, tsconfig `paths`
(longest literal prefix wins, every target tried), tsconfig `baseUrl`, then the
workspace package's own `exports`/`main`. A specifier none of those declare is
external, and resolves to nothing. There is deliberately no fallback.

New:
  - `typescript/tsconfig.ts` — every tsconfig/jsconfig in the repo with
    `extends` chains resolved, nearest-config-wins per file. The old loader read
    three filenames at the repo root, required `paths` to exist, and kept only
    `targets[0]` — none of which describes a monorepo, where `apps/web/
    tsconfig.json` is what governs `apps/web/src/main.ts`.
  - `typescript/module-resolution.ts` — the algorithm.
  - `typescript/file-candidates.ts` — 11 TS-family extensions, replacing a
    shared 39-entry list spanning every indexed language, so a TypeScript
    import can no longer resolve to a `.py` file.
  - `import-resolvers/node-workspace-packages.ts` — in-repo manifests, with
    `exports` subpath maps, patterns, condition nesting, and the restriction
    that a package declaring `exports` exposes only what it lists.

The per-pass `SuffixIndex` is gone from these three adapters: real resolution
derives nothing from the file list — every candidate comes from a declared
source and is checked with one `Set.has` — so there is nothing left to cache.
Their `*-import-index-reuse` guards and the JS index-vs-scan differential are
deleted with the mechanism they measured; the cross-language contract test
moves the three languages to its existing `KNOWN_UNINDEXED` channel, and pins
the exemption as a list so a fourth arrival is deliberate.

Python, Ruby, Java, Go and the rest still route through `suffixResolve` and are
untouched here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Px638Zyqa9CJMUU7DsJoB

* test(scope-resolution): assert every resolver refuses external imports (#2953)

One property, for all 16 registered resolvers: a specifier naming something
outside the repository must not resolve to a file inside it.

That is the property #2953 was filed against, and its violation is not a
missing edge but a fabricated one — an IMPORTS edge at full confidence between
two files with no relationship, which `impact` then reports as blast radius.
The mechanism is shared (`suffixResolve`), so the guard is too.

Every case pairs an external specifier with a DECOY: an unrelated in-repo file
whose path ends the way the specifier does. Without one a resolver that merely
found nothing would pass while holding no property at all, so each case also
asserts the decoy is reachable by the spelling that SHOULD find it — a typo in
a fixture cannot manufacture a pass.

Two fixtures had to be corrected before the results meant anything, and both
would have recorded a false gap:

  - C# reads its #1881 gate from scanned namespace evidence and fails OPEN
    without any, so passing `undefined` measured nothing. Armed, C# holds.
  - C++ was posting a pass on an extension mismatch (`vector` could never match
    `src/vector.hpp` whatever the resolver did). Given the header spelling, it
    does not hold.

Result: six hold it — TypeScript, JavaScript and Vue because they resolve
against declared config only (#2953); Python (#898) and C# (#1881) because they
gate the fallback on in-repo evidence; Rust because `::` never decomposes into
a path suffix, which the decoy-reachability arm confirms is a real pass rather
than a vacuous one.

Ten do not, and are recorded in KNOWN_GAPS with what each currently answers:
Java, Kotlin, Go, Ruby, PHP, Dart, Swift, C, C++, COBOL. The map is a work
list, not an allowance — the entries are ASSERTED, so a language that starts
holding the property fails here and its line gets deleted deliberately rather
than rotting into a lie.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Px638Zyqa9CJMUU7DsJoB

* fix(typescript): admit only declared workspace packages, and fix four resolver defects (#2953)

Review of #2956 found one boundary bug and four correctness defects. The
boundary one is the same defect class this PR exists to fix, arriving from a
different direction.

## The workspace boundary (review)

`loadNodeWorkspacePackages` registered every `package.json` the repo-wide scan
found, and never read `pnpm-workspace.yaml` or a root `workspaces`
declaration. Finding a manifest is not the same as the workspace admitting one:
an app importing registry package `foo` would bind to an excluded fixture or
example that happens to declare `name: "foo"` — the false-positive half of
#2953, from a new source of evidence. This repository is the example, since
`test/fixtures/**` declares `@repo/utils` among others.

The admitted set now comes from the declaration — `workspaces` (array and yarn
object form), `pnpm-workspace.yaml`, `lerna.json`, with `!` exclusions and
`*`/`**` — plus the root package itself. A repo that declares no workspace has
exactly one package: the root. A negative fixture pins it, with a named package
outside the declared globs that must not resolve.

## Four defects

  - tsconfig `paths` targets were resolved against the config's own directory
    when it declared `paths` but inherited `baseUrl`. tsc resolves them against
    the EFFECTIVE base, so an extending config loaded the right alias pattern
    and pointed every target at the wrong directory.
  - two configs in one directory were ranked by directory-listing order, so
    `tsconfig.base.json` could govern instead of `tsconfig.json` and a config's
    own `paths` went invisible. Found by the test written for the fix above.
  - an unexported package subpath also tried `<dir>/src/<subpath>`. Nothing
    declares that mapping; it is the same kind of guess this PR removes, and
    the import it "resolved" is broken in the real project too.
  - `imports` pattern keys (`"#internal/*"`) were looked up exactly, so a valid
    `#internal/foo` never matched. `exports` and `imports` now share one
    matcher, which is where they should never have diverged.
  - a relative specifier climbing past the repo root was silently clamped, so
    `../../../secret` from `src/main.ts` became `secret` and could resolve a
    root file it never named.

## Test rigor

The conformance suite asserted less than it claimed. The decoy-reachability arm
only checked non-empty, so five cases paired `reachesDecoy` with a different
file than `decoy` and passed while establishing nothing; the KNOWN_GAPS arm
likewise accepted any in-repo answer instead of the recorded one. Both now
assert the exact file. The reachability arm runs only for languages that HOLD
the property — for a gap language the recorded-answer assertion IS that proof,
and for Swift and COBOL no other spelling exists, since `Foundation` and
`EXTERNAL` name the in-repo directory and copybook as well as the external
module, which is precisely why those resolvers cannot tell them apart.

## Benchmarks

Both `--check` guards were red, and both were reporting something true.

`import-target`: the ts-family arms resolved 0 of 3200 imports. Their corpus is
bare specifiers with no config, which the deleted `suffixResolve` answered
without one — so the arms measured an empty branch while printing a clean
scaling ratio. Each now carries the config its corpus is spelled for, and the
`deep` arm's uniform prefix reaches it. THE FINGERPRINTS THEN MATCHED THE
RECORDED BASELINES EXACTLY: same corpus, same targets, once the config it
always implied is passed explicitly. Retained per-pass index went from
26 745 296 B (js, ts) and 28 884 016 B (vue) at 32 000 files to 0-16 B, because
these resolvers no longer build one; they move to the `HEAP_BOUNDED` tier rust
already occupies for the same reason. Depth ratio moved 2.0 -> ~2.2 and the
budget goes to 2.6: candidates now carry the 16-segment baseUrl prefix, so each
`Set.has` hashes a longer string — linear in path LENGTH, independent of file
COUNT.

`scope-capture`: TypeScript capture fingerprint drift, caused by this PR's 12
new `.ts` fixtures entering the corpus. Attribution is exact rather than
inferred — moving that one fixture directory aside returns the fingerprint to
`f719163e…` byte-for-byte with `fixture_count` back at 155 and all 15 languages
passing.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Px638Zyqa9CJMUU7DsJoB

* fix(typescript): honour exports fallback arrays, paths precedence and package extends (#2953)

Second review round. Four findings, judged against what this tool is: a static
analyser building a code graph, not a compiler. The bar is resolving what the
project DECLARES, on a checkout that may never have been built or installed,
and never inventing an edge.

  - `exports` and `imports` ARRAYS were skipped. An array is Node's ordered
    fallback list, and `{"./feature": ["./dist/feature.js", "./src/feature.ts"]}`
    is exactly what a workspace package publishes to mean "built output, or
    source". Skipping it dropped the declaration entirely and left the package
    looking as though it exported no subpaths. The source arm is the one that
    matters here, because `dist/` is build output and is not indexed — and for
    a static analyser the build need not have run at all.
  - an exact `paths` pattern did not reliably outrank a wildcard. `a` and `a*`
    both match `a` with the same literal prefix length, so sorting on length
    alone left tsc's exact-wins rule to declaration order.
  - package-form `extends` (`"@acme/tsconfig"`) was refused outright. Not
    indexing `node_modules` is different from not READING it, and a shared
    internal base is where a monorepo puts the `paths` its packages import
    through. It is now read from disk, walking `node_modules` up from the
    extending config the way Node does, and absent on an un-installed checkout
    it degrades to whatever that config declared itself.

    The test pins what tsc actually does with such a base rather than what one
    might hope: `extends` never rebases `baseUrl`, so a package base's paths
    point at the package's own directory. That is why a published base rarely
    contributes aliases a repo's files resolve through, and why the
    `@tsconfig/*` family — which sets `target` and `lib`, never `paths` — is a
    no-op here either way.
  - CodeQL flagged `String.replace('*', …)` in two places as replacing only the
    first occurrence. Node subpath patterns and tsconfig `paths` both allow AT
    MOST one `*`, so that IS the specified behaviour — but the spelling states
    it by accident and reads as the replace-all footgun. `substituteStar`
    slices at the known index and says the rule.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Px638Zyqa9CJMUU7DsJoB

* fix(typescript): treat `exports` as the whole interface, and keep empty tsconfig scopes (#2953)

Third review round. Two findings, both valid, both cases of this resolver being
laxer than the thing it models — which is the direction that fabricates edges.

  - `exports`, when a manifest declares it, is the package's ENTIRE public
    interface: Node ignores `main` outright and refuses any subpath the map
    does not list. This resolver already honoured that restriction for
    SUBPATHS and not for the package ROOT, which is the same rule. A manifest
    exporting only `"./feature"` therefore still answered a bare `@repo/pkg`
    with `main` or `src/index` — an edge for an import that does not resolve in
    the real project. Legacy and conventional root candidates are now offered
    only when there is no `exports` field at all.

  - a tsconfig declaring neither `baseUrl` nor `paths` was dropped rather than
    kept as an empty scope, so `tsconfigFor` fell through to an enclosing
    config. A package whose own tsconfig declares no `baseUrl` — meaning its
    non-relative specifiers are package lookups — silently inherited the repo
    root's aliases instead. An empty scope is the accurate answer for such a
    file, and only a scope can express it.

Both are pinned at the level they broke: the manifest arms assert what
`readManifest` produces, not a hand-built package, since the resolver honouring
empty entries and the loader producing them are different claims.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015Px638Zyqa9CJMUU7DsJoB

---------

Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-14 10:13:04 +01:00

172 lines
6.5 KiB
TypeScript

/**
* Which directories the workspace ADMITS as packages (#2953 review).
*
* Reading manifests is not the same as trusting them. An earlier draft
* registered every `package.json` the repo-wide scan found, which recreates the
* false-positive half of #2953 from a different source: an app importing
* registry package `foo` binds to an excluded fixture or example that happens
* to declare `name: "foo"`. This repository is itself the example —
* `test/fixtures/**` declares `@repo/utils` among others.
*
* So the boundary is the workspace declaration, and these arms are about where
* it is read from and what it admits.
*/
import { describe, it, expect, afterAll } from 'vitest';
import fs from 'node:fs';
import os from 'node:os';
import path from 'node:path';
import { loadNodeWorkspacePackages } from '../../src/core/ingestion/import-resolvers/node-workspace-packages.js';
const roots: string[] = [];
/** Write a throwaway repo from a `relativePath -> contents` map. */
function repo(files: Readonly<Record<string, string>>): string {
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-ws-'));
roots.push(root);
for (const [rel, contents] of Object.entries(files)) {
const full = path.join(root, rel);
fs.mkdirSync(path.dirname(full), { recursive: true });
fs.writeFileSync(full, contents);
}
return root;
}
const pkg = (name: string): string => JSON.stringify({ name, main: 'src/index.ts' });
afterAll(() => {
for (const root of roots) fs.rmSync(root, { recursive: true, force: true });
});
describe('workspace boundary', () => {
it('admits a package the pnpm globs declare', async () => {
const root = repo({
'pnpm-workspace.yaml': 'packages:\n - "packages/*"\n',
'package.json': JSON.stringify({ name: 'root', private: true }),
'packages/utils/package.json': pkg('@repo/utils'),
});
const packages = await loadNodeWorkspacePackages(root);
expect(packages?.byName.has('@repo/utils')).toBe(true);
});
it('refuses a package outside those globs', async () => {
const root = repo({
'pnpm-workspace.yaml': 'packages:\n - "packages/*"\n',
'package.json': JSON.stringify({ name: 'root', private: true }),
'packages/utils/package.json': pkg('@repo/utils'),
// A fixture or example. Declares a name, is not a workspace member.
'examples/demo/package.json': pkg('lodash'),
});
const packages = await loadNodeWorkspacePackages(root);
expect(packages?.byName.has('@repo/utils')).toBe(true);
// The pointed case: an app importing registry `lodash` must not bind here.
expect(packages?.byName.has('lodash')).toBe(false);
});
it('reads npm/yarn `workspaces` from the root manifest', async () => {
const root = repo({
'package.json': JSON.stringify({ name: 'root', workspaces: ['apps/*'] }),
'apps/web/package.json': pkg('@repo/web'),
'vendored/copy/package.json': pkg('@repo/vendored'),
});
const packages = await loadNodeWorkspacePackages(root);
expect(packages?.byName.has('@repo/web')).toBe(true);
expect(packages?.byName.has('@repo/vendored')).toBe(false);
});
it('reads the yarn object form', async () => {
const root = repo({
'package.json': JSON.stringify({ name: 'root', workspaces: { packages: ['apps/*'] } }),
'apps/web/package.json': pkg('@repo/web'),
});
const packages = await loadNodeWorkspacePackages(root);
expect(packages?.byName.has('@repo/web')).toBe(true);
});
it('honours a `!` exclusion', async () => {
const root = repo({
'pnpm-workspace.yaml': 'packages:\n - "packages/*"\n - "!packages/internal"\n',
'package.json': JSON.stringify({ name: 'root' }),
'packages/utils/package.json': pkg('@repo/utils'),
'packages/internal/package.json': pkg('@repo/internal'),
});
const packages = await loadNodeWorkspacePackages(root);
expect(packages?.byName.has('@repo/utils')).toBe(true);
expect(packages?.byName.has('@repo/internal')).toBe(false);
});
it('matches `**` across segments', async () => {
const root = repo({
'pnpm-workspace.yaml': 'packages:\n - "packages/**"\n',
'package.json': JSON.stringify({ name: 'root' }),
'packages/group/nested/package.json': pkg('@repo/nested'),
});
const packages = await loadNodeWorkspacePackages(root);
expect(packages?.byName.has('@repo/nested')).toBe(true);
});
it('admits only the root when the repo declares no workspace', async () => {
const root = repo({
'package.json': pkg('just-one-package'),
// A nested manifest in a repo with no workspace declaration is not a
// member of anything — which is exactly this repository's own shape, and
// why its `test/fixtures/**` manifests must not register.
'test/fixtures/thing/package.json': pkg('@repo/utils'),
});
const packages = await loadNodeWorkspacePackages(root);
expect(packages?.byName.has('just-one-package')).toBe(true);
expect(packages?.byName.has('@repo/utils')).toBe(false);
});
it('gives a package with a root-less `exports` map no root entries', async () => {
// The loader half of the rule: when `exports` is present it is the whole
// interface, so `main` and the conventional `src/index` fallback must not
// be offered for the bare package name.
const root = repo({
'pnpm-workspace.yaml': 'packages:\n - "packages/*"\n',
'package.json': JSON.stringify({ name: 'root' }),
'packages/inner/package.json': JSON.stringify({
name: '@repo/inner',
main: 'src/index.ts',
exports: { './nest': './src/nest.ts' },
}),
});
const packages = await loadNodeWorkspacePackages(root);
const inner = packages?.byName.get('@repo/inner');
expect(inner?.entries).toEqual([]);
expect(inner?.subpathExports.get('nest')).toEqual(['packages/inner/src/nest']);
});
it('keeps legacy and conventional root entries when there is no `exports`', async () => {
const root = repo({
'pnpm-workspace.yaml': 'packages:\n - "packages/*"\n',
'package.json': JSON.stringify({ name: 'root' }),
'packages/utils/package.json': JSON.stringify({ name: '@repo/utils', main: 'src/index.ts' }),
});
const packages = await loadNodeWorkspacePackages(root);
expect(packages?.byName.get('@repo/utils')?.entries).toContain('packages/utils/src/index');
});
it('returns null when the repo has no manifest at all', async () => {
const root = repo({ 'src/main.ts': 'export const x = 1;\n' });
expect(await loadNodeWorkspacePackages(root)).toBeNull();
});
});