From 36f91ab5a02383ab88d3ebdc9fe3bab9087e5cfd Mon Sep 17 00:00:00 2001 From: Gergo Magyar Date: Thu, 27 Aug 2026 11:29:42 +0000 Subject: [PATCH] test(ignore-service): make the single-component set guards able to fail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- gitnexus/src/config/ignore-service.ts | 1 - gitnexus/test/helpers/ignore-set-source.ts | 139 ++++++++++++++++++ .../test/unit/ignore-build-output.test.ts | 102 ++++++++++--- 3 files changed, 220 insertions(+), 22 deletions(-) create mode 100644 gitnexus/test/helpers/ignore-set-source.ts diff --git a/gitnexus/src/config/ignore-service.ts b/gitnexus/src/config/ignore-service.ts index 107ab34dd..2129841d1 100644 --- a/gitnexus/src/config/ignore-service.ts +++ b/gitnexus/src/config/ignore-service.ts @@ -107,7 +107,6 @@ const DEFAULT_IGNORE_LIST = new Set([ // remains covered by .gitignore/.gitnexusignore and the unambiguous names. 'monaco-workers', // Monaco editor web-worker bundles generated for browser runtime '.terraform', - '.serverless', // Documentation (optional - might want to keep) // 'docs', diff --git a/gitnexus/test/helpers/ignore-set-source.ts b/gitnexus/test/helpers/ignore-set-source.ts new file mode 100644 index 000000000..d020cf050 --- /dev/null +++ b/gitnexus/test/helpers/ignore-set-source.ts @@ -0,0 +1,139 @@ +/** + * Source-parsing extractor for the bare-name sets in `src/config/ignore-service.ts`. + * + * Those sets are module-private, and exporting them purely to be testable would + * widen a production surface to satisfy a test — the same call + * `receiver-twin-list-drift.test.ts` documents. So the guards read the source + * text instead, and this helper is the one parser they share. It lives here + * rather than inside a single test file because two suites need it: the + * slash-free guard and the cross-package drift guard. + * + * It is a single-pass scanner rather than a regex chain, because comments and + * strings can each contain the other's delimiters and neither can be removed + * independently: + * + * - The ignore-list comments quote paths and carry an apostrophe (`Next.js's`), + * so matching literals before removing comments yields phantom entries — + * several slash-bearing, which would fail the slash assertion on correct + * source. + * - A glob string such as `'**‌/*'` contains a comment-open sequence, so + * removing comments with a regex first swallows the rest of the declaration + * and the scan runs past the closing bracket. + * + * Tracking string and comment state in one pass is the only ordering that is + * correct in both directions. + * + * A source parser still cannot resolve a spread, an interpolation, a + * concatenation, or a later `.add(...)`. Those are rejected loudly rather than + * silently reducing the entry count, because a guard that quietly stops seeing + * members is the defect these guards exist to catch. + */ + +/** Shapes a source-text parser cannot resolve to a fixed list of string literals. */ +const UNRESOLVABLE_SHAPES = ['...', '${', '+'] as const; + +interface ScanResult { + /** String literals declared directly in the block. */ + entries: string[]; + /** Block text with comments and string bodies removed, for shape checks. */ + skeleton: string; +} + +/** + * Walk the bracketed block that starts at `open`, collecting string literals and + * a comment-free, string-free skeleton. Returns null when the block never closes. + */ +const scanBlock = (source: string, open: number): ScanResult | null => { + const entries: string[] = []; + let skeleton = ''; + let depth = 0; + + for (let i = open; i < source.length; i += 1) { + const ch = source[i]; + const next = source[i + 1]; + + if (ch === '/' && next === '*') { + const end = source.indexOf('*/', i + 2); + if (end === -1) return null; + i = end + 1; + continue; + } + if (ch === '/' && next === '/') { + const end = source.indexOf('\n', i + 2); + if (end === -1) return null; + i = end; + continue; + } + if (ch === "'" || ch === '"' || ch === '`') { + const quote = ch; + let literal = ''; + let j = i + 1; + for (; j < source.length; j += 1) { + if (source[j] === '\\') { + literal += source[j + 1] ?? ''; + j += 1; + continue; + } + if (source[j] === quote) break; + literal += source[j]; + } + if (j >= source.length) return null; + if (literal.length > 0) entries.push(literal); + i = j; + continue; + } + + if (ch === '[') depth += 1; + if (ch === ']') { + depth -= 1; + if (depth === 0) return { entries, skeleton }; + } + skeleton += ch; + } + + return null; +}; + +/** + * String literals declared in the `[...]` block introduced by `marker`. + * + * Throws — never returns a short list — when the marker is missing, the block is + * malformed, or the declaration contains a member this parser cannot resolve. + */ +export const setEntries = (source: string, marker: string): string[] => { + const at = source.indexOf(marker); + if (at === -1) { + throw new Error(`${marker} not found in ignore-service.ts — update this test`); + } + + const open = source.indexOf('[', at); + if (open === -1) { + throw new Error(`${marker}: no bracketed block follows the marker`); + } + + const scanned = scanBlock(source, open); + if (scanned === null) { + throw new Error(`${marker}: bracketed block never closes`); + } + + for (const shape of UNRESOLVABLE_SHAPES) { + if (scanned.skeleton.includes(shape)) { + throw new Error( + `${marker}: declaration contains \`${shape}\`, which a source parser cannot resolve. ` + + `Switch this set to a runtime assertion rather than letting the guard read fewer members.`, + ); + } + } + + return scanned.entries; +}; + +/** + * True when `setName` is mutated by `.add(...)` anywhere in `source`. + * + * `setEntries` reads the declaration only, so a member appended afterwards would + * be invisible to it. The guards assert this is false rather than silently + * under-reporting. + */ +export const hasRuntimeAdd = (source: string, setName: string): boolean => + new RegExp(`\\b${setName}\\s*\\.\\s*add\\s*\\(`).test(source); diff --git a/gitnexus/test/unit/ignore-build-output.test.ts b/gitnexus/test/unit/ignore-build-output.test.ts index 920ee8a7d..a2c2800ad 100644 --- a/gitnexus/test/unit/ignore-build-output.test.ts +++ b/gitnexus/test/unit/ignore-build-output.test.ts @@ -2,16 +2,17 @@ import { describe, it, expect } from 'vitest'; import fs from 'node:fs'; import path from 'node:path'; import { fileURLToPath } from 'node:url'; -import { shouldIgnorePath } from '../../src/config/ignore-service.js'; +import { isHardcodedIgnoredDirectory, shouldIgnorePath } from '../../src/config/ignore-service.js'; +import { hasRuntimeAdd, 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 - * `/app/src/main/assets/public/_next/static/` had 40% of its indexed - * files come from minified chunks — and every `Route` node the repo produced - * pointed at a webpack bundle instead of source. + * `/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', () => { @@ -57,10 +58,7 @@ describe('build-output ignores', () => { expect(shouldIgnorePath('src/public/api.ts')).toBe(false); }); - it('keeps the name set free of slashes so a fragment cannot silently die', () => { - // 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. + describe('single-component set guards', () => { const source = fs.readFileSync( path.resolve( path.dirname(fileURLToPath(import.meta.url)), @@ -72,18 +70,80 @@ describe('build-output ignores', () => { ), 'utf8', ); - const block = source.slice( - source.indexOf('const DEFAULT_IGNORE_LIST = new Set(['), - source.indexOf(']);', source.indexOf('const DEFAULT_IGNORE_LIST = new Set([')), - ); - // Parse ENTRY LINES only. Scanning the raw block would also read prose in - // the comments (an apostrophe in "Next.js's" opens a spurious quote). - const entries = block - .split('\n') - .map((line) => line.trim()) - .filter((line) => line.startsWith("'")) - .map((line) => line.slice(1, line.indexOf("'", 1))); - expect(entries.length).toBeGreaterThan(20); // parsed something real - expect(entries.filter((e) => e.includes('/'))).toEqual([]); + + // 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. Counts are pinned exactly rather than floored: a + // floor cannot protect a two-member set, and it hides a partial parse. + const SETS = [ + { marker: 'const DEFAULT_IGNORE_LIST = new Set([', name: 'DEFAULT_IGNORE_LIST', size: 76 }, + { marker: 'const IGNORED_FILES = new Set([', name: 'IGNORED_FILES', size: 33 }, + { + marker: 'const ROOT_ARTIFACT_DIRECTORIES = new Set([', + name: 'ROOT_ARTIFACT_DIRECTORIES', + size: 2, + }, + { marker: 'const IGNORED_EXTENSIONS = new Set([', name: 'IGNORED_EXTENSIONS', size: 104 }, + ] as const; + + it.each(SETS)('$name holds no slash-bearing member', ({ marker }) => { + expect(setEntries(source, marker).filter((entry) => entry.includes('/'))).toEqual([]); + }); + + it.each(SETS)('$name parses to its pinned size', ({ marker, size }) => { + expect(setEntries(source, marker)).toHaveLength(size); + }); + + it.each(SETS)('$name holds no duplicate member', ({ marker }) => { + const entries = setEntries(source, marker); + expect(new Set(entries).size).toBe(entries.length); + }); + + it.each(SETS)('$name is never mutated by .add() after construction', ({ name }) => { + expect(hasRuntimeAdd(source, name)).toBe(false); + }); + + it('every IGNORED_EXTENSIONS member starts with a dot', () => { + const entries = setEntries(source, 'const IGNORED_EXTENSIONS = new Set(['); + 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`). Matching literals before stripping them yields phantom + // entries, several slash-bearing, which would fail the slash assertion on + // correct source. + const entries = setEntries(source, 'const DEFAULT_IGNORE_LIST = new Set(['); + expect(entries).toContain('_next'); + expect(entries).not.toContain('public/build'); + expect(entries).not.toContain('env/'); + expect(entries).not.toContain('packages'); + }); + + it('agrees with the runtime set it claims to describe', () => { + // Catches parser drift without exporting the set: every name the parser + // reports must actually be ignored by the module's own predicate. + const entries = setEntries(source, 'const DEFAULT_IGNORE_LIST = new Set(['); + expect(entries.filter((entry) => !isHardcodedIgnoredDirectory(entry))).toEqual([]); + }); + + it('fails loudly when the marker no longer matches', () => { + expect(() => setEntries(source, 'const NOT_A_REAL_SET = new Set([')).toThrow( + /not found in ignore-service\.ts/, + ); + }); + + it('fails loudly rather than under-reporting an unresolvable declaration', () => { + // A spread, an interpolation, or a concatenation resolves at runtime, not + // in source text. Parsing fewer members and passing is the failure mode + // these guards exist to prevent, so the parser refuses instead. + const poisoned = source.replace( + 'const ROOT_ARTIFACT_DIRECTORIES = new Set([', + 'const ROOT_ARTIFACT_DIRECTORIES = new Set([...OTHER_NAMES,', + ); + expect(() => setEntries(poisoned, 'const ROOT_ARTIFACT_DIRECTORIES = new Set([')).toThrow( + /cannot resolve/, + ); + }); }); });