mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-08-28 05:25:25 +00:00
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) 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>
172 lines
6.5 KiB
TypeScript
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();
|
|
});
|
|
});
|