From c9d1e9f39daa32c8722adbdfc8c162c1eaab79d7 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 24 Oct 2026 20:39:07 +0000 Subject: [PATCH] fix(pregate): match members globs by code point in one pass, and admit every crate past a size cap Second port review round (outside voice): the matcher counted UTF-16 units, so 'crates/?' missed a crate directory named with a supplementary character and dropped its inherited dependencies (a false green); and each unmatched '[' rescanned the rest of the glob (5 s for one crate with a million of them). Tokenize by code point with a precomputed next-']' table, and past 4096 characters of members globs admit every crate under the workspace: a wrong member fails loudly in cargo, a dropped one is silent. The new supervised probe kills a super-linear matcher at 30 s instead of hanging the file. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- lib/pregate/rust.ts | 29 ++++++++++++++++++----------- test/pregate-rust.test.ts | 30 +++++++++++++++++++++--------- 2 files changed, 39 insertions(+), 20 deletions(-) diff --git a/lib/pregate/rust.ts b/lib/pregate/rust.ts index f842777af..a6319668c 100644 --- a/lib/pregate/rust.ts +++ b/lib/pregate/rust.ts @@ -50,36 +50,41 @@ function joinRel(dir: string, rel: string): string | null { if (joined === '..' || joined.startsWith('../') || path.posix.isAbsolute(joined)) return null; return joined === '.' ? '' : joined; } -/** One glob component (`*`, `?`, `[...]` with `!`/`^` negation and ranges, else literal) against one path component, by dynamic programming: O(pattern × name), no regex backtracking. */ +/** One glob component (`*`, `?`, `[...]` with `!`/`^` negation and ranges, else literal) against one path component, by Unicode code point and dynamic programming: O(pattern × name), no regex backtracking. */ function componentMatch(glob: string, name: string): boolean { + const g = Array.from(glob); + const s = Array.from(name); + const nextClose = new Array(g.length + 1).fill(-1); + for (let i = g.length - 1; i >= 0; i--) nextClose[i] = g[i] === ']' ? i : nextClose[i + 1]!; const tokens: Array<'*' | ((c: string) => boolean)> = []; - for (let i = 0; i < glob.length; i++) { - const ch = glob[i]!; - const negate = ch === '[' && /[!^]/.test(glob[i + 1] ?? ''); - const close = ch === '[' ? glob.indexOf(']', i + (negate ? 3 : 2)) : -1; + for (let i = 0; i < g.length; i++) { + const ch = g[i]!; + const negate = ch === '[' && /[!^]/.test(g[i + 1] ?? ''); + const close = ch === '[' ? nextClose[i + (negate ? 3 : 2)] ?? -1 : -1; if (ch === '*') tokens.push('*'); else if (ch === '?') tokens.push(() => true); else if (close > 0) { - const body = glob.slice(i + (negate ? 2 : 1), close); + const body = g.slice(i + (negate ? 2 : 1), close).map(c => c.codePointAt(0)!); tokens.push(c => { + const p = c.codePointAt(0)!; let hit = false; for (let j = 0; j < body.length; j++) { - if (body[j + 1] === '-' && j + 2 < body.length) { hit ||= c >= body[j]! && c <= body[j + 2]!; j += 2; } else hit ||= c === body[j]; + if (body[j + 1] === 0x2d && j + 2 < body.length) { hit ||= p >= body[j]! && p <= body[j + 2]!; j += 2; } else hit ||= p === body[j]; } return hit !== negate; }); i = close; } else tokens.push(c => c === ch); } - let row = name.split('').map(() => false).concat(false); + let row = s.map(() => false).concat(false); row[0] = true; for (const t of tokens) { const next = row.map(() => false); next[0] = t === '*' && row[0]!; - for (let j = 1; j <= name.length; j++) next[j] = t === '*' ? row[j]! || next[j - 1]! : row[j - 1]! && t(name[j - 1]!); + for (let j = 1; j <= s.length; j++) next[j] = t === '*' ? row[j]! || next[j - 1]! : row[j - 1]! && t(s[j - 1]!); row = next; } - return row[name.length]!; + return row[s.length]!; } /** A Cargo `members` glob against a workspace-relative crate directory, component by component; `**` spans any number of components. */ function memberMatch(glob: string, rel: string): boolean { @@ -94,6 +99,8 @@ function memberMatch(glob: string, rel: string): boolean { } return row[p.length]!; } +/** Past this many characters of `members` globs a workspace admits every crate under it: a wrong member fails loudly in cargo, a dropped one is a silent false green. */ +const MAX_MEMBER_GLOB_CHARS = 4096; const inside = (dir: string, root: string) => root === '' || dir === root || dir.startsWith(`${root}/`); const relTo = (dir: string, root: string) => (root === '' ? dir : dir === root ? '' : dir.slice(root.length + 1)); @@ -150,7 +157,7 @@ export function cargoModel(files: readonly string[], read: ReadFile): CargoModel // Cargo's rules: the root package and a literally listed member always belong; a glob match or a path dependency of a // member inside the workspace belongs unless `exclude` names it or a directory above it. const literal = (r: string) => globs.some(g => !/[*?[]/.test(g) && relTo(r, ws.dir) === g); - const members = new Set([...own].filter(r => r === ws.dir || literal(r) || (globs.some(g => memberMatch(g, relTo(r, ws.dir))) && !excluded(r)))); + const members = new Set([...own].filter(r => r === ws.dir || literal(r) || ((globs.join('').length > MAX_MEMBER_GLOB_CHARS || globs.some(g => memberMatch(g, relTo(r, ws.dir)))) && !excluded(r)))); memberCache.set(ws.dir, members); const queue = [...members]; while (queue.length) { diff --git a/test/pregate-rust.test.ts b/test/pregate-rust.test.ts index 3f087f7e4..e18859cae 100644 --- a/test/pregate-rust.test.ts +++ b/test/pregate-rust.test.ts @@ -9,7 +9,9 @@ */ import { afterEach, describe, expect, test } from 'bun:test'; import * as fs from 'node:fs'; +import { spawnSync } from 'node:child_process'; import * as path from 'node:path'; +import { bunIsolationArgs } from '../lib/bun-isolation'; import { EXIT } from '../lib/headless-artifacts'; import { parseSummary, planRuns } from '../lib/pregate/checks/lanes'; import { DEFAULT_CONFIG } from '../lib/pregate/config'; @@ -223,14 +225,15 @@ describe('Cargo membership and test detection (port review)', () => { // why_new=port review found classes escaped; seam=none; no_claim=cargo's own glob crate test('a members glob with a character class or ** admits the crate, so an inherited dependency links its dependent', () => { const files = { - 'Cargo.toml': '[workspace]\nmembers = ["crates/[ab]", "nested/**/leaf"]\n\n[workspace.dependencies]\nbase = { package = "alpha", path = "crates/a" }\n', + 'Cargo.toml': '[workspace]\nmembers = ["crates/[ab]", "nested/**/leaf", "unicode/?"]\n\n[workspace.dependencies]\nbase = { package = "alpha", path = "crates/a" }\n', 'crates/a/Cargo.toml': pkg('alpha'), 'crates/a/tests/a.rs': '#[test]\nfn a() {}\n', 'crates/b/Cargo.toml': pkg('beta', '\n[dependencies]\nbase = { workspace = true }\n'), 'crates/b/tests/b.rs': '#[test]\nfn b() {}\n', 'crates/c/Cargo.toml': pkg('gamma'), 'nested/x/y/leaf/Cargo.toml': pkg('leaf'), + 'unicode/📦/Cargo.toml': pkg('boxed'), }; const { crates } = cargoModel(...inMemory(files)); - expect(['crates/a', 'crates/b', 'crates/c', 'nested/x/y/leaf'].map(r => crates.get(r)!.workspace)).toEqual(['', '', 'crates/c', '']); + expect(['crates/a', 'crates/b', 'crates/c', 'nested/x/y/leaf', 'unicode/📦'].map(r => crates.get(r)!.workspace)).toEqual(['', '', 'crates/c', '', '']); expect(dependentsOf(files, 'crates/a/Cargo.toml')).toEqual(['crates/a/tests/a.rs', 'crates/b/tests/b.rs']); }); @@ -248,13 +251,22 @@ describe('Cargo membership and test detection (port review)', () => { }); // Value: protects=a hostile members glob or a file of unclosed cfg attributes cannot stall lane selection, which runs before - // any runner deadline; fails_when=member globs go through a backtracking regex again (seconds per crate) or the cfg scan is - // unbounded (quadratic); why_new=port review measured both over 5 s; seam=none; no_claim=a wall-clock budget (the test timeout bounds it) - test('a hostile members glob and a run of unclosed cfg attributes are matched in linear time', () => { - const files = { 'Cargo.toml': `[workspace]\nmembers = ["${'*a'.repeat(20)}b"]\n`, [`${'a'.repeat(40)}/Cargo.toml`]: pkg('k') }; - expect(cargoModel(...inMemory(files)).crates.get('a'.repeat(40))!.workspace).toBe('a'.repeat(40)); - expect(hasCfgTest('#[cfg('.repeat(20_000))).toBe(false); - }); + // any runner deadline; each probe runs in its own supervised Bun so a regression is killed and fails instead of hanging the + // file; fails_when=member globs backtrack again (an exponential '*a*a…b', or a rescan per unmatched '[') or the cfg scan + // rescans from every start (quadratic: about 80 s on 600 KB, against tens of ms now); why_new=port review; seam=none; + // no_claim=a wall-clock budget for selection (the 30 s kill only separates linear from super-linear) + const supervised = (code: string) => spawnSync(process.execPath, [...bunIsolationArgs(), '-e', `import { cargoModel, hasCfgTest } from ${JSON.stringify(path.join(import.meta.dir, '..', 'lib', 'pregate', 'rust.ts'))};\n${code}`], { encoding: 'utf8', timeout: 30_000 }); + test('a hostile members glob is matched without backtracking', () => { + const r = supervised(` + const pkg = n => '[package]\\nname = "' + n + '"\\nversion = "0.1.0"\\n'; + const run = (members, dirs) => { const files = { 'Cargo.toml': '[workspace]\\nmembers = ' + JSON.stringify(members) + '\\n' }; dirs.forEach((d, i) => { files[d + '/Cargo.toml'] = pkg('k' + i); }); return cargoModel(Object.keys(files), f => files[f] ?? null); }; + const exp = run(['*a'.repeat(20) + 'b'], ['a'.repeat(40)]).crates.get('a'.repeat(40)).workspace; + const brackets = run(['['.repeat(1000000)], Array.from({ length: 10 }, (_, i) => 'c' + i)).crates.get('c0').workspace; + console.log(JSON.stringify([exp, brackets]));`); + expect(r.error).toBeUndefined(); + expect(r.stdout.trim()).toBe(JSON.stringify(['a'.repeat(40), ''])); + }, 60_000); + }); describe('gstack-pregate on a Rust workspace', () => {