From cb5e9727d00dcb62b2dcfa505c6cd2e36b10fb90 Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Thu, 27 Aug 2026 08:09:42 +0000 Subject: [PATCH] fix(ingestion): drop the inert public/build machinery, discriminate _next by segment, ignore _next on the web upload path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- gitnexus-web/src/lib/upload-filter.test.ts | 16 +++++++++ gitnexus-web/src/lib/upload-filter.ts | 6 ++++ gitnexus/src/config/ignore-service.ts | 35 ++++--------------- .../test/unit/ignore-build-output.test.ts | 22 +++++++++--- 4 files changed, 45 insertions(+), 34 deletions(-) diff --git a/gitnexus-web/src/lib/upload-filter.test.ts b/gitnexus-web/src/lib/upload-filter.test.ts index e97b9ec2c..1943720ba 100644 --- a/gitnexus-web/src/lib/upload-filter.test.ts +++ b/gitnexus-web/src/lib/upload-filter.test.ts @@ -31,6 +31,22 @@ describe('filterRepoFiles', () => { expect(r.droppedCount).toBe(4); }); + it('excludes emitted _next output, including the Capacitor/Cordova copy', () => { + // `.next` was listed but `_next` was not, so a mobile-wrapped Next.js app + // uploaded its whole minified bundle against the server's caps for files + // the analyzer then discards anyway (#3007). + const input = [ + f('repo/android/app/src/main/assets/public/_next/static/chunks/main.js'), + f('repo/ios/App/App/public/_next/static/chunks/framework.js'), + f('repo/_next/static/chunks/x.js'), + f('repo/src/index.ts'), + f('repo/src/_nextgen/index.ts'), + ]; + const r = filterRepoFiles(input); + expect(r.manifest).toEqual(['repo/src/index.ts', 'repo/src/_nextgen/index.ts']); + expect(r.droppedCount).toBe(3); + }); + it('drops files over the per-file size cap', () => { const input = [f('repo/big.bin', MAX_FILE_BYTES + 1), f('repo/small.ts', 10)]; const r = filterRepoFiles(input); diff --git a/gitnexus-web/src/lib/upload-filter.ts b/gitnexus-web/src/lib/upload-filter.ts index a24520f74..9b00b5dab 100644 --- a/gitnexus-web/src/lib/upload-filter.ts +++ b/gitnexus-web/src/lib/upload-filter.ts @@ -22,6 +22,12 @@ export const EXCLUDED_DIRS = new Set([ 'build', 'out', '.next', + // `.next` is the build CACHE, `_next` the EMITTED output — different + // directories. A Capacitor/Cordova shell leaves the emitted bundle at + // `/app/src/main/assets/public/_next/`, so without this the whole + // minified tree is uploaded against the server's file/byte caps only to be + // discarded by the analyzer's own ignore list (#3007). + '_next', '.nuxt', '.cache', 'coverage', diff --git a/gitnexus/src/config/ignore-service.ts b/gitnexus/src/config/ignore-service.ts index cf8447f94..107ab34dd 100644 --- a/gitnexus/src/config/ignore-service.ts +++ b/gitnexus/src/config/ignore-service.ts @@ -71,6 +71,12 @@ const DEFAULT_IGNORE_LIST = new Set([ '.netlify', '.serverless', '_build', + // `'public/build'` used to sit here. This set is tested one path SEGMENT at a + // time, and `isHardcodedIgnoredDirectory(name)` takes a bare directory name, + // so a slash-containing member could never match either — it was inert. Its + // paths were never unignored though: bare `'build'` above already prunes + // `public/build/**`, so removing the entry changes no behavior (#3007). The + // guard test below keeps the next slash-bearing entry from dying the same way. '.parcel-cache', '.turbo', '.svelte-kit', @@ -117,30 +123,6 @@ const DEFAULT_IGNORE_LIST = new Set([ '__snapshots__', ]); -/** - * Multi-segment paths to ignore, matched against the whole POSIX path. - * - * These CANNOT live in {@link DEFAULT_IGNORE_LIST}: that set is tested one path - * SEGMENT at a time (`parts.some(p => DEFAULT_IGNORE_LIST.has(p))`) and is also - * exposed through `isHardcodedIgnoredDirectory(name)`, which receives a bare - * directory name. A slash-containing member can never equal a single segment, - * so `'public/build'` sat in that set matching nothing at all until #3007. A - * guard test pins the invariant that the name set stays slash-free. - */ -const DEFAULT_IGNORED_PATH_FRAGMENTS: readonly string[] = [ - 'public/build', // Remix / Laravel Mix compiled asset output -]; - -/** True when `normalizedPath` contains any ignored multi-segment fragment. */ -function hasIgnoredPathFragment(normalizedPath: string): boolean { - return DEFAULT_IGNORED_PATH_FRAGMENTS.some( - (fragment) => - normalizedPath === fragment || - normalizedPath.startsWith(`${fragment}/`) || - normalizedPath.includes(`/${fragment}/`) || - normalizedPath.endsWith(`/${fragment}`), - ); -} // Ambiguous names that conventionally denote generated artifacts only at the // repository root. Nested directories with these names are frequently source // modules (for example apps/web/src/env or packages/api/generated). @@ -353,11 +335,6 @@ export const shouldIgnorePath = (filePath: string): boolean => { } } - // Multi-segment entries cannot be matched by the per-segment loop above. - if (hasIgnoredPathFragment(normalizedPath)) { - return true; - } - // Check exact filename matches if (IGNORED_FILES.has(fileName) || IGNORED_FILES.has(fileNameLower)) { return true; diff --git a/gitnexus/test/unit/ignore-build-output.test.ts b/gitnexus/test/unit/ignore-build-output.test.ts index 8a1fbe354..920ee8a7d 100644 --- a/gitnexus/test/unit/ignore-build-output.test.ts +++ b/gitnexus/test/unit/ignore-build-output.test.ts @@ -34,9 +34,20 @@ describe('build-output ignores', () => { expect(shouldIgnorePath('packages/next-auth/src/index.ts')).toBe(false); }); - it('ignores public/build, which never matched while it sat in the name set', () => { - // A slash-containing member of DEFAULT_IGNORE_LIST can never equal a single - // path segment, so this entry matched nothing at all before #3007. + 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); }); @@ -47,8 +58,9 @@ describe('build-output ignores', () => { }); it('keeps the name set free of slashes so a fragment cannot silently die', () => { - // The invariant that makes the bug above impossible to reintroduce: entries - // needing a slash belong in DEFAULT_IGNORED_PATH_FRAGMENTS. + // The invariant that makes the inert entry impossible to reintroduce: this + // set is matched one path segment at a time, so a member containing a slash + // is dead on arrival and must be expressed some other way. const source = fs.readFileSync( path.resolve( path.dirname(fileURLToPath(import.meta.url)),