From fe05b3ccf529ebda02e4a497b47a9bb2d85017be Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 11:59:39 +0000 Subject: [PATCH] fix(pregate): strays matches only the documented scratch names; literals ignores a named literal the branch only adds (GST-1211) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- .../gst-1211-pregate-false-positives.md | 8 +++++ docs/pregate.md | 7 ++-- docs/troubleshooting.md | 4 ++- lib/pregate/checks/literals.ts | 16 ++++++--- lib/pregate/checks/strays.ts | 2 +- test/pregate-selection.test.ts | 34 +++++++++++++++++++ 6 files changed, 62 insertions(+), 9 deletions(-) create mode 100644 changelog.d/gst-1211-pregate-false-positives.md diff --git a/changelog.d/gst-1211-pregate-false-positives.md b/changelog.d/gst-1211-pregate-false-positives.md new file mode 100644 index 000000000..0ee87b79b --- /dev/null +++ b/changelog.d/gst-1211-pregate-false-positives.md @@ -0,0 +1,8 @@ +**Two `gstack-pregate` preflight false positives fixed (GST-1211).** Both stopped /ship on real work in gbrain-evals. + +### Fixed +- **`strays`:** the scratch-name check matched three prefixes the docs never listed (`temp`, `debug-` and `probe`), so real files like `probe.py` and `temporal-graph-shipped.test.ts` failed preflight. It now matches only the documented names: `zz-*`, `tmp*`, `scratch*`, plus editor leftovers (`*.orig`, `*.rej`, `*.swp`, `*.swo`, `*.bak`, `*.tmp`, `*~`, `.DS_Store`, `Thumbs.db`), which docs/pregate.md and docs/troubleshooting.md now list too. +- **`literals`:** a named literal the branch only adds, such as `limit: 300` in a new object, was read as an existing `limit: 10` elsewhere in the file changing to 300, and every test that mentioned 10 was flagged. The check now compares every value under a name, so an added literal isn't a change. Editing an existing value still fails, and so does changing one of two equal values under the same name, which the old last-value check could miss. + +### Tests +- `test/pregate-selection.test.ts`: `probe.py` and `temporal-*.test.ts` pass `strays` while `zz-notes.md`, `tmp-x.ts`, `scratch.py` and `util.js.orig` still fail; a new object literal beside `limit: 10` passes `literals` while editing that value still fails; the pure `changedLiterals` case keeps an exported constant change. diff --git a/docs/pregate.md b/docs/pregate.md index a6b692927..916503ca7 100644 --- a/docs/pregate.md +++ b/docs/pregate.md @@ -201,9 +201,12 @@ Two stages with separate clocks, both printed: file is read whole from disk. A file over the size limit, or whose full diff does not parse, is scanned on its added lines alone, without that context. - `strays`: a new file at the repository root, a new top-level directory, or a - scratch name (`zz-*`, `tmp*`, `scratch*`) anywhere. + scratch name (`zz-*`, `tmp*`, `scratch*`) or editor leftover (`*.orig`, + `*.rej`, `*.swp`, `*.swo`, `*.bak`, `*.tmp`, `*~`, `.DS_Store`, `Thumbs.db`) + anywhere. - `literals`: for every changed exported constant or named timeout/retry - literal, the whole test tree (nightly- and Heavy-only files included) is + literal (a named literal the branch only adds, such as `limit: 300` in a new + object, is not a change), the whole test tree (nightly- and Heavy-only files included) is searched for the old value in files that name the constant or, when the old value is longer than one character, its module (the module's file name not followed by another extension, or a path ending diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index f0aca0832..cd8570e90 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -3770,7 +3770,9 @@ anchored pattern and a reason ([docs/pregate.md](pregate.md#allowlist)); ### `PREGATE_STRAYS` **Meaning.** A file was added at the repository root, in a new top-level -directory, or under a scratch name (`zz-*`, `tmp*`, `scratch*`). +directory, or under a scratch name (`zz-*`, `tmp*`, `scratch*`) or an editor +leftover name (`*.orig`, `*.rej`, `*.swp`, `*.swo`, `*.bak`, `*.tmp`, `*~`, +`.DS_Store`, `Thumbs.db`). **Fix.** Delete or move it. An intentional root file goes in `strays.allow`. diff --git a/lib/pregate/checks/literals.ts b/lib/pregate/checks/literals.ts index b34311e09..46373f985 100644 --- a/lib/pregate/checks/literals.ts +++ b/lib/pregate/checks/literals.ts @@ -57,10 +57,10 @@ export function constantsIn(text: string): Map { return out; } -/** Named numeric call sites (`timeout: 5000`) keyed by name; the last value per name wins, which is enough to spot a change. */ -export function callLiteralsIn(text: string): Map { - const out = new Map(); - for (const m of text.matchAll(CALL_PATTERN)) out.set(m[1]!, m[2]!); +/** Named numeric call sites (`timeout: 5000`) keyed by name, every value in source order. */ +export function callLiteralsIn(text: string): Map { + const out = new Map(); + for (const m of text.matchAll(CALL_PATTERN)) out.set(m[1]!, [...(out.get(m[1]!) ?? []), m[2]!]); return out; } @@ -69,7 +69,13 @@ export function changedLiterals(file: string, oldText: string, newText: string): const oldC = constantsIn(oldText); const newC = constantsIn(newText); for (const [name, old] of oldC) { const now = newC.get(name); if (now !== undefined && now !== old) out.push({ name, old, new: now, file }); } const oldCall = callLiteralsIn(oldText); const newCall = callLiteralsIn(newText); - for (const [name, old] of oldCall) { const now = newCall.get(name); if (now !== undefined && now !== old && !oldC.has(name)) out.push({ name, old, new: now, file }); } + for (const [name, olds] of oldCall) { + if (oldC.has(name)) continue; + const added = [...(newCall.get(name) ?? [])]; + const removed = olds.filter(v => { const i = added.indexOf(v); if (i >= 0) added.splice(i, 1); return i < 0; }); + const seen = new Set(); + removed.forEach((old, i) => { if (i < added.length && !seen.has(old)) { seen.add(old); out.push({ name, old, new: added[i]!, file }); } }); + } return out; } diff --git a/lib/pregate/checks/strays.ts b/lib/pregate/checks/strays.ts index 4b5d5b572..fed27ae1d 100644 --- a/lib/pregate/checks/strays.ts +++ b/lib/pregate/checks/strays.ts @@ -12,7 +12,7 @@ import type { CheckDef, CheckResult, PregateContext } from '../types'; /** Conventional root files a repo adds without a declaration. */ export const ROOT_OK = /^(?:README|LICENSE|LICENCE|NOTICE|CHANGELOG|CONTRIBUTING|CODE_OF_CONDUCT|SECURITY|AUTHORS|VERSION|Makefile|Dockerfile|Procfile|Gemfile|Rakefile|Cargo\.(?:toml|lock)|go\.(?:mod|sum)|package(?:-lock)?\.json|bun\.lockb?|pnpm-lock\.yaml|yarn\.lock|tsconfig[\w.-]*\.json|bunfig\.toml|pyproject\.toml|pubspec\.(?:yaml|lock)|analysis_options\.yaml|dart_test\.yaml|requirements[\w.-]*\.txt|setup\.(?:py|cfg)|\.[\w.-]+|[\w.-]+\.config\.[cm]?[jt]s|[\w.-]+\.md)$/i; -export const STRAY_NAME = /^(?:zz-|tmp|temp|scratch|debug-|probe)|\.(?:orig|rej|swp|swo|bak|tmp)$|~$|^\.DS_Store$|^Thumbs\.db$/i; +export const STRAY_NAME = /^(?:zz-|tmp|scratch)|\.(?:orig|rej|swp|swo|bak|tmp)$|~$|^\.DS_Store$|^Thumbs\.db$/i; function run(ctx: PregateContext): CheckResult { const inputs = baseInputs(ctx); diff --git a/test/pregate-selection.test.ts b/test/pregate-selection.test.ts index 2069493f7..99618df1b 100644 --- a/test/pregate-selection.test.ts +++ b/test/pregate-selection.test.ts @@ -201,6 +201,13 @@ describe('literals (pure)', () => { { name: 'MAX', old: '3', new: '4', file: 'f.ts' }, { name: 'timeout', old: '5000', new: '9000', file: 'f.ts' }, ]); }); + + test('GST-1211: a key inside a newly added object literal is not a changed literal; an edited call-site value and an exported constant still are', () => { + const before = 'export const MAX = 3;\nconst page = { limit: 10 };\n'; + expect(changedLiterals('f.ts', before, `${before}const query = { selection: { order: 'asc', limit: 300 } };\n`)).toEqual([]); + expect(changedLiterals('f.ts', before, 'export const MAX = 3;\nconst page = { limit: 20 };\n')).toEqual([{ name: 'limit', old: '10', new: '20', file: 'f.ts' }]); + expect(changedLiterals('f.ts', before, 'export const MAX = 4;\nconst page = { limit: 10 };\nconst query = { limit: 300 };\n')).toEqual([{ name: 'MAX', old: '3', new: '4', file: 'f.ts' }]); + }); // Value: protects=a sibling file that shares the module's stem (`postgres-unit-arms.txt` beside `postgres-unit-arms.ts`) is not the // module, so a test that only names it is not a consumer, while an extensionless import, a JS/TS import extension, the module's own // file name and a path ending at the stem (then a quote, space, bracket, sentence end or `.d.ts`) still are, and neither `.snap` nor a JS twin of a @@ -519,6 +526,21 @@ describe('gstack-pregate (bin)', () => { expect(w.out).toMatch(/^preflight {2}strays {4}warn .*downgraded to warn by origin\/main@/m); }); + test('GST-1211: probe.py and temporal-*.test.ts are not scratch names; zz-*, tmp*, scratch* and editor leftovers still are', () => { + const repo = fixture({ pregate: { ...BASE_PREGATE, warn: [] } }); + commit(repo, { 'docs/comparison-systems/ext-temporal-graph/shipped-0.31/probe.py': 'print(1)\n', 'test/eval/temporal-graph-shipped.test.ts': 'export {};\n' }); + const ok = pregate(repo, ['--stage', 'preflight', '--only', 'strays']); + expect(ok.status).toBe(EXIT.ok); + expect(ok.out).toMatch(/^preflight {2}strays {4}pass /m); + commit(repo, { 'docs/zz-notes.md': '# n\n', 'lib/tmp-x.ts': 'export {};\n', 'test/scratch.py': 'pass\n', 'lib/util.js.orig': 'x\n' }); + const r = pregate(repo, ['--stage', 'preflight', '--only', 'strays']); + expect(r.status).toBe(EXIT.fail); + expect(r.out).toMatch(/^preflight {2}strays {4}fail .*4 stray file\(s\)/m); + for (const f of ['docs/zz-notes.md', 'lib/tmp-x.ts', 'test/scratch.py', 'lib/util.js.orig']) expect(r.out).toContain(`strays: ${f}: scratch name (${f.split('/').pop()})`); + expect(r.out).not.toContain('probe.py: scratch name'); + expect(r.out).not.toContain('temporal-graph-shipped.test.ts: scratch name'); + }); + test('p0-4-heavy-old-constant: a nightly-only test still asserting the old literal fails literals with file:line', () => { const repo = fixture(); commit(repo, { 'lib/retry.js': "exports.MAX_RETRIES = 3;\nexports.TIMEOUT_MS = 9000;\nexports.retry = (fn) => fn();\n" }); @@ -532,6 +554,18 @@ describe('gstack-pregate (bin)', () => { expect(pregate(repo, ['--stage', 'preflight', '--only', 'literals']).status).toBe(EXIT.ok); }); + test('GST-1211: a new object literal beside an existing `limit: 10` passes literals; editing that existing value still fails', () => { + const repo = fixture({ baseFiles: { 'lib/query.js': 'exports.page = () => ({ limit: 10 });\n', 'test/query.test.js': "const test = require('node:test');\nconst assert = require('node:assert');\nconst { page } = require('../lib/query.js');\ntest('pages by ten', () => assert.strictEqual(page().limit, 10));\n" } }); + commit(repo, { 'lib/query.js': "exports.page = () => ({ limit: 10 });\nexports.selection = { order: 'asc', limit: 300 };\n" }); + const ok = pregate(repo, ['--stage', 'preflight', '--only', 'literals']); + expect(ok.status).toBe(EXIT.ok); + expect(ok.out).not.toContain('test/query.test.js'); + commit(repo, { 'lib/query.js': "exports.page = () => ({ limit: 20 });\nexports.selection = { order: 'asc', limit: 300 };\n" }); + const r = pregate(repo, ['--stage', 'preflight', '--only', 'literals']); + expect(r.status).toBe(EXIT.fail); + expect(r.out).toContain('literals: test/query.test.js:4: limit still 10 (now 20 in lib/query.js)'); + }); + // Value: protects=a real stale one-character constant still fails literals end to end after the #3492 narrowing, and an incidental // digit in the same file is not what it reports; fails_when=the statement binding drops a named assertion; why_new=only // multi-character values had a bin-level control; seam=none; no_claim=two-character values