mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-08-28 05:25:25 +00:00
* fix(ingestion): ignore emitted Next.js build output, and restore the dead public/build entry `DEFAULT_IGNORE_LIST` contained `.next` — the build CACHE — but not `_next`, the emitted OUTPUT, which are different directories. A Capacitor/Cordova shell copies a built Next.js bundle to `<platform>/app/src/main/assets/public/_next/static/`, where no path segment hits the list, so the walker indexed the bundle as source. On a real mobile-wrapped Next.js app that was 256 minified chunk files, and every `Route` node the repo produced pointed at a webpack chunk rather than at source. The filename heuristics did not catch them either: they match `.bundle.`, `.chunk.`, `.generated.` and `.d.ts`, while Next.js emits hashed names like `6862-9d1cdcb99f169a06.js`. Separately, `'public/build'` had been sitting in `DEFAULT_IGNORE_LIST` matching nothing at all. That set is tested one path SEGMENT at a time, and is also read by `isHardcodedIgnoredDirectory(name)`, which receives a bare directory name — so a slash-containing member can never compare equal to anything. Rather than delete the entry and lose its intent, multi-segment paths now live in `DEFAULT_IGNORED_PATH_FRAGMENTS` and are matched against the whole path, so Remix / Laravel Mix asset output is ignored as originally intended. A guard test pins the invariant that made the dead entry possible: no member of the name set may contain a slash. Measured against a production Capacitor-wrapped Next.js app (1558 JS/TS files on disk): 256 newly ignored, none of them under `src/`, and zero files that were previously ignored become indexed. Closes #3007 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(ingestion): drop the inert public/build machinery, discriminate _next by segment, ignore _next on the web upload path Addresses the review findings on #3018. Remove DEFAULT_IGNORED_PATH_FRAGMENTS, hasIgnoredPathFragment and its shouldIgnorePath branch. The mechanism was correct but unreachable: all four of its match forms put a `/` or end-of-string on both sides of `build`, so a fragment match strictly implies `build` is a whole segment, which the per-segment DEFAULT_IGNORE_LIST loop already catches one branch earlier. Measured over 768,420 generated paths: 65,506 fragment matches, 0 of them decisive, 0 implication violations. `'public/build'` really was an inert member of the name set, but its paths were never unignored — bare `'build'` covered them on both sides — so the entry is deleted rather than relocated, which is the other option #3007 offered. The slash-free guard test stays; it is what stops the next slash-bearing entry from dying the same way. Add negative cases pinning that `_next` matches as a whole path segment. The previous suite could not tell a segment rule from a substring rule: replacing the entry with `normalizedPath.includes('_next')` passed all five tests, while eating `src/_nextgen/index.ts`. Rename the public/build test to what it actually pins — that deleting the inert entry changed no behavior — since it is green on both sides by design. Add `_next` to the web upload filter's EXCLUDED_DIRS. That list is the live browser ingestion path (RepoAnalyzer -> filterRepoFiles -> /api/analyze/upload) and had `.next` but not `_next`, so a Capacitor-wrapped Next.js app uploaded its entire minified tree against the server's 20000-file / 250MB caps for files the analyzer then discards. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(ignore-service): make the single-component set guards able to fail The slash-free guard added for #3007 could not fail. It selected entry lines with startsWith("'") and read only the first quoted token per line, so 'public/build' could return as a backtick string, behind an inline block comment, as a second entry on an existing line, or via .add() and every test stayed green. Prettier and eslint miss the backtick and inline-comment forms too, so CI did not catch them either. U1: remove the duplicate '.serverless' entry so the set can be pinned to one exact number. A Set discarded it, so no ignore behaviour changes. U2/U3: replace the line-based parser with a shared single-pass scanner in test/helpers/ignore-set-source.ts, and extend the guard from DEFAULT_IGNORE_LIST to IGNORED_FILES, ROOT_ARTIFACT_DIRECTORIES and IGNORED_EXTENSIONS, which share the same single-component match contract. The scanner tracks string and comment state together because neither can be removed first: the ignore-list comments quote paths and carry an apostrophe, so matching literals before stripping comments yields phantom slash-bearing entries; and a glob string containing a comment-open sequence makes regex comment-stripping swallow the closing bracket. Only a single pass is correct in both directions. Counts are pinned exactly rather than floored — a floor cannot protect a two-member set and hides a partial parse. Shapes a source parser cannot resolve (spread, interpolation, concatenation, later .add) now throw instead of quietly reporting fewer members, and the parsed names are cross-checked against isHardcodedIgnoredDirectory so parser drift fails without exporting the set. Verified by mutation: all six fail-open spellings now turn the suite red; 187 tests pass, tsc clean. * test(ignore-service): pin that _next prunes the directory, not just its files Every measured benefit of ignoring _next comes from never enumerating the bundle tree, and no file list can observe that: anything under _next is rejected whether the walk pruned the directory or descended and rejected each file. childrenIgnored is the only observation that separates them. The existing build-output tests all call shouldIgnorePath, the leaf predicate, so a refactor moving _next to a shouldIgnorePath-only rule would keep them green while silently restoring the full walk. These assertions close that. Also pins that _next matches as a whole segment (_nextgen and my_next are still walked), and that the `!_next/` negation recovers the directory at any depth — the bare form is the one that works, since `!_next/**` alone never gets tested: childrenIgnored prunes the directory before any descendant pattern is reached. Placed in the .gitnexusignore-negation describe block, which owns mkPath and the tmpdir fixture and is registered in scripts/cross-platform-tests.ts. Verified by mutation: disabling only the pruning branch in childrenIgnored leaves the build-output suite at 26/26 green and turns these assertions red. * test(ignore-service): guard the twin build-output ignore lists against drift _next now lives in two lists in two packages — the analyzer's DEFAULT_IGNORE_LIST and the browser upload filter's EXCLUDED_DIRS — with nothing tying them together. This is the seventh twin-list pair in this repo; the header of receiver-twin-list-drift.test.ts records that the previous ones each shipped a bug when one side moved. Containment runs web -> CLI only, and that is the load-bearing direction: the browser filter decides what the server ever sees, and it reads no .gitnexusignore, so a name it drops that the analyzer would have indexed is silent source loss with no recovery. The reverse is not an error — the analyzer prunes far more aggressively than an upload needs to. .gitnexus is the one exemption and has a mechanism: the walker passes dot: false to glob, so it never enumerates dot-directories. Asserted in both directions so re-adding it to the CLI list or dropping it from the web list both fail. Both sides are source-parsed through the shared helper. DEFAULT_IGNORE_LIST is module-private, and no test in this package imports across the package boundary — every cross-package precedent reads source instead. Also corrects the documentation this PR's comments got wrong: the guard test is cited by path rather than as "below", the unreproducible per-repo percentage is gone, the reason _next is deliberately unanchored is recorded next to the entry (no <web-root>/_next form matches a root-level _next/static/…), and the upload filter now states that it consults no repository ignore rules — so unlike the CLI, a negation cannot recover what it drops. Verified by mutation: a web-only addition and a CLI removal each turn the guard red. 194 targeted tests pass; tsc clean in both packages. * refactor(test): read the ignore sets with the TypeScript parser, not a hand-rolled scanner The guards read ignore-service.ts as source because the sets are module-private. The first pass hand-rolled a character scanner to do it, and the repo already vendors the right tool: ts.createSourceFile, used this way in literal-collectors, query-determinism-guard, cli-index-help and group/sync-partial-extraction. The scanner had two silent gaps a real parser does not have: - It rejected `${` by substring, but template literals were consumed whole, so that branch could never fire and an interpolated member was accepted as a literal — the exact under-report the file refused to allow. - It took the first `[` after the marker, which on a type-annotated declaration (`readonly string[] = ...`) is the annotation's empty pair. It returned [] with no throw, which would make every assertion in a suite vacuously true. This is the hazard receiver-twin-list-drift.test.ts documents having hit. Reading the declaration node removes both, along with the comment-vs-string ordering problem that motivated the scanner: a parser cannot mistake a comment for a string or a glob's `/*` for a comment-open. Also drops the four pinned exact counts. They were a ratchet — these sets are edited by unrelated PRs, each of which would have failed a count assertion about nothing it touched — and with a real parser the partial-parse hazard they existed to catch cannot happen silently: a member that is not a plain string literal throws. Markers collapse to set names, and the duplicated path-resolution boilerplate moves into the helper the two suites already share. Net 187 deletions against 123 insertions. Verified by mutation: backtick, inline comment, same-line, double-quote, duplicate, interpolation, spread and runtime .add() are all caught; a type-annotated declaration now reads correctly instead of returning empty. 194 tests pass, tsc clean. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
127 lines
5.6 KiB
TypeScript
127 lines
5.6 KiB
TypeScript
import { describe, it, expect } from 'vitest';
|
|
import { isHardcodedIgnoredDirectory, shouldIgnorePath } from '../../src/config/ignore-service.js';
|
|
import {
|
|
IGNORE_SERVICE_PATH,
|
|
hasRuntimeAdd,
|
|
readSource,
|
|
setEntries,
|
|
} from '../helpers/ignore-set-source.js';
|
|
|
|
/**
|
|
* Emitted build output must not be indexed as source (#3007).
|
|
*
|
|
* `.next` (the build cache) was listed but `_next` (the emitted output) was
|
|
* not, so a Capacitor/Cordova shell that copies a Next.js bundle into
|
|
* `<platform>/app/src/main/assets/public/_next/static/` had its shipped bundle
|
|
* indexed as source — and every `Route` node the repo produced pointed at a
|
|
* webpack bundle instead of code anyone wrote.
|
|
*/
|
|
|
|
describe('build-output ignores', () => {
|
|
it('ignores emitted _next output, including the Capacitor/Cordova copy', () => {
|
|
expect(shouldIgnorePath('_next/static/chunks/main.js')).toBe(true);
|
|
expect(shouldIgnorePath('.next/server/app/page.js')).toBe(true);
|
|
expect(
|
|
shouldIgnorePath(
|
|
'android/app/src/main/assets/public/_next/static/chunks/6862-9d1cdcb99f169a06.js',
|
|
),
|
|
).toBe(true);
|
|
expect(shouldIgnorePath('ios/App/App/public/_next/static/chunks/framework-abc123.js')).toBe(
|
|
true,
|
|
);
|
|
});
|
|
|
|
it('does not ignore ordinary source that merely mentions next', () => {
|
|
expect(shouldIgnorePath('src/next-steps.ts')).toBe(false);
|
|
expect(shouldIgnorePath('src/nextConfig/index.ts')).toBe(false);
|
|
expect(shouldIgnorePath('packages/next-auth/src/index.ts')).toBe(false);
|
|
});
|
|
|
|
it('matches _next as a whole segment, not as a substring', () => {
|
|
// Without these, `normalizedPath.includes('_next')` would satisfy every
|
|
// other assertion in this file — the suite could not tell a segment rule
|
|
// from a substring rule, and a substring rule would eat real source.
|
|
expect(shouldIgnorePath('src/_nextgen/index.ts')).toBe(false);
|
|
expect(shouldIgnorePath('packages/my_next/src/index.ts')).toBe(false);
|
|
expect(shouldIgnorePath('src/prefix_next.ts')).toBe(false);
|
|
});
|
|
|
|
it('keeps public/build ignored after the inert name-set entry was removed', () => {
|
|
// NOT a regression test for new behavior — it pins that DELETING the inert
|
|
// `'public/build'` entry changed nothing, because bare `'build'` matches
|
|
// these as an ordinary segment and always did. Green on both sides of the
|
|
// change by design; that is the point.
|
|
expect(shouldIgnorePath('public/build/entry.client.js')).toBe(true);
|
|
expect(shouldIgnorePath('apps/web/public/build/manifest.js')).toBe(true);
|
|
});
|
|
|
|
it('does not ignore public/ or build/-adjacent source outside that pair', () => {
|
|
expect(shouldIgnorePath('public/favicon-loader.ts')).toBe(false);
|
|
expect(shouldIgnorePath('src/public/api.ts')).toBe(false);
|
|
});
|
|
|
|
describe('single-component set guards', () => {
|
|
const source = readSource(IGNORE_SERVICE_PATH);
|
|
|
|
// Every one of these sets is compared against a single path component, so a
|
|
// member containing `/` is dead on arrival — the defect that left
|
|
// `'public/build'` inert.
|
|
const SET_NAMES = [
|
|
'DEFAULT_IGNORE_LIST',
|
|
'IGNORED_FILES',
|
|
'ROOT_ARTIFACT_DIRECTORIES',
|
|
'IGNORED_EXTENSIONS',
|
|
] as const;
|
|
|
|
it.each(SET_NAMES)('%s holds no slash-bearing member', (setName) => {
|
|
expect(setEntries(source, setName).filter((entry) => entry.includes('/'))).toEqual([]);
|
|
});
|
|
|
|
it.each(SET_NAMES)('%s holds no duplicate member', (setName) => {
|
|
const entries = setEntries(source, setName);
|
|
expect(new Set(entries).size).toBe(entries.length);
|
|
});
|
|
|
|
it.each(SET_NAMES)('%s is never mutated by .add() after construction', (setName) => {
|
|
expect(hasRuntimeAdd(source, setName)).toBe(false);
|
|
});
|
|
|
|
it('every IGNORED_EXTENSIONS member starts with a dot', () => {
|
|
const entries = setEntries(source, 'IGNORED_EXTENSIONS');
|
|
expect(entries.filter((entry) => !entry.startsWith('.'))).toEqual([]);
|
|
});
|
|
|
|
it('reads entries the declaration holds, not text the comments quote', () => {
|
|
// The comments in DEFAULT_IGNORE_LIST quote paths and carry an apostrophe
|
|
// (`Next.js's`), so a text scanner reads phantom entries out of them —
|
|
// several slash-bearing, which would fail the assertion above on correct
|
|
// source. Parsing the declaration cannot see comments at all.
|
|
const entries = setEntries(source, 'DEFAULT_IGNORE_LIST');
|
|
expect(entries).toContain('_next');
|
|
expect(entries).not.toContain('public/build');
|
|
expect(entries).not.toContain('env/');
|
|
});
|
|
|
|
it('agrees with the runtime set it claims to describe', () => {
|
|
// Catches drift between what the guard reads and what the module does,
|
|
// without exporting the set.
|
|
const entries = setEntries(source, 'DEFAULT_IGNORE_LIST');
|
|
expect(entries.filter((entry) => !isHardcodedIgnoredDirectory(entry))).toEqual([]);
|
|
});
|
|
|
|
it('fails loudly when a set is no longer declared as a Set of literals', () => {
|
|
expect(() => setEntries(source, 'NOT_A_REAL_SET')).toThrow(/update this test/);
|
|
});
|
|
|
|
it('refuses a declaration whose members it cannot resolve', () => {
|
|
// A spread, an interpolation, or a concatenation resolves at runtime, not
|
|
// in source. Reading the resolvable members and passing is the failure mode
|
|
// these guards exist to prevent, so the reader refuses instead.
|
|
const poisoned = source.replace(
|
|
'const ROOT_ARTIFACT_DIRECTORIES = new Set([',
|
|
'const ROOT_ARTIFACT_DIRECTORIES = new Set([...OTHER_NAMES,',
|
|
);
|
|
expect(() => setEntries(poisoned, 'ROOT_ARTIFACT_DIRECTORIES')).toThrow(/not plain string/);
|
|
});
|
|
});
|
|
});
|