feat(pregate): pregate-literal markers waive one deliberate old-value line (#3490)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

changelog.d/gst-634-literals-waiver.md

@@ -0,0 +1,8 @@
+**A test can keep an old value on purpose without turning off `literals` (#3490).** When a minimum version goes up, the test that checks the old version is refused has to keep the old value, and pregate failed it with `PREGATE_LITERALS`. The only way out was `warn: ["literals"]`, which turns the check off for every constant. Now that one line can carry `// pregate-literal: NAME=<old value> <reason>`.
+
+### Added
+- **`pregate-literal:` markers.** A marker on the flagged line, or on the comment line directly above it, waives that line when it names the changed constant and its old value and gives a reason. Every other line in the file is still checked, and text inside a string is not a marker. The policy has no waiver key, so neither a base nor a pushed `pregate.json` can widen it.
+- **Waivers are listed.** `literals` prints each waived line as `waived: <file>:<line>: <NAME> <old> (<reason>)` and adds `marker added on this branch` when the merge-base file lacks the marker, so a reviewer sees every new waiver.
+
+### Changed
+- **docs/pregate.md** and **docs/troubleshooting.md** (`PREGATE_LITERALS`) describe the marker.

docs/pregate.md

@@ -201,6 +201,17 @@ Two stages with separate clocks, both printed:
   elsewhere in the file is not a consumer. A call-site name such as `attempts`
   or `limit` is an ordinary word, so for it the file must also name the changed
   module.
+  A test that keeps the old value on purpose, such as the refused input after a
+  minimum version goes up, marks that one line with a comment on it or on the
+  comment line directly above it:
+  `// pregate-literal: MIN_RUNTIME='1.4.0' refused input below the minimum`
+  (`#` and `/*` comments work too). The marker waives only that line, and only
+  when it names the changed constant and its old value and gives a reason; text
+  inside a string is not a marker. Every other line in the file is still
+  checked. The check lists each waived line as
+  `waived: <file>:<line>: <NAME> <old> (<reason>)`, adding `marker added on
+  this branch` when the merge-base file lacks the marker. The policy has no
+  waiver key, so neither a base nor a pushed `pregate.json` can widen it.
 
 - `timing`: added or changed test lines that compare an elapsed wall-clock or
   CPU time against a bound, or sleep and then assert, print

docs/troubleshooting.md

@@ -3335,7 +3335,12 @@ directory, or under a scratch name (`zz-*`, `tmp*`, `scratch*`).
 appears with its old value in a test file the branch did not update — nightly-
 and Heavy-only files included.
 
-**Fix.** Update every listed `file:line`, or keep the old value.
+**Fix.** Update every listed `file:line`, or keep the old value. When a test
+keeps the old value on purpose (the refused input after a minimum version goes
+up), mark that line: `// pregate-literal: NAME=<old value> <reason>` on the line
+or on the comment line directly above it. The marker waives only that line, and
+the check lists it as `waived:`. Don't use `warn: ["literals"]` for this, since
+that turns the check off for every constant.
 
 <a id="pregate-no-lane"></a>
 ### `PREGATE_NO_LANE`

lib/pregate/checks/literals.ts

@@ -14,6 +14,23 @@ import { isTestFile, listRepoFiles, scansLiterals } from '../import-graph';
 import type { CheckDef, CheckResult, PregateContext } from '../types';
 
 export interface ChangedLiteral { name: string; old: string; new: string; file: string }
+/** A consumer line a `pregate-literal:` marker waived; `added` when the merge-base file lacks the marker. */
+export interface LiteralWaiver { at: string; reason: string; added: boolean }
+
+const MARKER = /pregate-literal:[ \t]*([A-Za-z_]\w*)=(\S+)[ \t]+(\S.*?)[ \t]*(?:\*\/[ \t]*)?$/;
+
+/** The marker in this line's trailing comment (`//`, `#` or `/*` outside a string), or null. */
+function markerIn(line: string): { marker: string; name: string; value: string; reason: string } | null {
+  const at = line.indexOf('pregate-literal:');
+  if (at < 0) return null;
+  const before = line.slice(0, at);
+  const open = /(?:\/\/|#|\/\*)[ \t]*$/.exec(before);
+  if (!open) return null;
+  const code = before.slice(0, open.index);
+  if (['\'', '"', '`'].some(q => code.split(q).length % 2 === 0)) return null;
+  const m = MARKER.exec(line.slice(at));
+  return m ? { marker: line.slice(at).trim(), name: m[1]!, value: m[2]!, reason: m[3]! } : null;
+}
 
 const CONST_PATTERNS: RegExp[] = [
   /^\s*export\s+(?:const|let|var)\s+([A-Z][A-Z0-9_]{1,})\s*(?::\s*[^=]+?)?\s*=\s*(.+?);?\s*$/,
@@ -79,8 +96,12 @@ function unquote(v: string): string { return /^['"`]/.test(v) ? v.slice(1, -1) :
  * has no new-value row: the branch extended the table. A table it did not
  * extend, a file that already held a new-value row, and an unreadable base
  * still count, and the scan goes on past skipped rows to the first real consumer.
+ *
+ * A consumer line is waived by a `pregate-literal: NAME=<old value> <reason>` comment on that line or
+ * on the comment line directly above it, naming this constant and its old value (#3490: the refused
+ * input after a minimum-version bump). Each waiver goes to `waived`; the scan goes on to the next line.
  */
-export function consumersOf(repoRoot: string, testFiles: readonly string[], change: ChangedLiteral, baseText?: (rel: string) => string | null | undefined): string[] {
+export function consumersOf(repoRoot: string, testFiles: readonly string[], change: ChangedLiteral, baseText?: (rel: string) => string | null | undefined, waived?: LiteralWaiver[]): string[] {
   const value = unquote(change.old);
   const valueRe = /^-?\d/.test(value) ? new RegExp(`(?<![\\w.])${escape(value)}(?![\\w.])`) : new RegExp(`['"\`]${escape(value)}['"\`]`);
   const row = (raw: string) => new RegExp(`^[ \\t]*\\[[ \\t]*${/^['"`]/.test(raw) ? `['"\`]${escape(unquote(raw))}['"\`]` : escape(raw)}[ \\t]*,`, 'm');
@@ -123,7 +144,15 @@ export function consumersOf(repoRoot: string, testFiles: readonly string[], chan
       if (extended === undefined) { const base = baseText?.(rel); extended = base === null || (base !== undefined && !newRowRe.test(base)); }
       return extended;
     };
-    for (let i = 0; i < lines.length; i++) if (valueRe.test(lines[i]!) && (!nameRequired || oldRowRe.test(lines[i]!) || named(i)) && !skip(i)) { hits.push(`${rel}:${i + 1}`); break; }
+    const waiver = (i: number) => {
+      const m = markerIn(lines[i]!) ?? (i > 0 && comment(lines[i - 1]!) ? markerIn(lines[i - 1]!) : null);
+      if (!m || m.name !== change.name || unquote(m.value) !== value) return false;
+      const before = baseText?.(rel);
+      waived?.push({ at: `${rel}:${i + 1}`, reason: m.reason, added: typeof before !== 'string' || !before.includes(m.marker) });
+      return true;
+    };
+    const carries = (i: number) => { const m = markerIn(lines[i]!); return valueRe.test(m ? lines[i]!.slice(0, lines[i]!.indexOf('pregate-literal:')) : lines[i]!); };
+    for (let i = 0; i < lines.length; i++) if (carries(i) && (!nameRequired || oldRowRe.test(lines[i]!) || named(i)) && !skip(i) && !waiver(i)) { hits.push(`${rel}:${i + 1}`); break; }
   }
   return hits;
 }
@@ -142,19 +171,23 @@ function run(ctx: PregateContext): CheckResult {
   if (!changes.length) return { id: 'literals', stage: 'preflight', status: 'pass', inputs, detail: `no exported constant or named timeout literal changed in ${candidates.length} modified source file(s)` };
   const testFiles = listRepoFiles(ctx.repoRoot).filter(f => scansLiterals(f) && !f.startsWith('bin/'));
   const lines: string[] = [];
+  const notes: string[] = [];
   for (const c of changes) {
+    const waived: LiteralWaiver[] = [];
     const hits = consumersOf(ctx.repoRoot, testFiles, c, rel => {
       const from = ctx.touched.find(t => t.path === rel)?.renamedFrom ?? rel;
       const r = spawnSync('git', ['show', `${ctx.mergeBase}:${from}`], { cwd: ctx.repoRoot, encoding: 'utf8', timeout: 30_000, maxBuffer: 64 * 1024 * 1024, env: ctx.env });
       if (r.status === 0) return r.stdout;
       const ls = spawnSync('git', ['ls-tree', ctx.mergeBase, '--', from], { cwd: ctx.repoRoot, encoding: 'utf8', timeout: 30_000, env: ctx.env });
       return ls.status === 0 && !ls.stdout.trim() ? null : undefined;
-    });
+    }, waived);
+    for (const w of waived) notes.push(`waived: ${w.at}: ${c.name} ${c.old} (${w.reason}${w.added ? '; marker added on this branch' : ''})`);
     ctx.explain.push(`literals: ${c.file} ${c.name} ${c.old} → ${c.new}: ${hits.length} test file(s) still carry ${c.old}`);
     for (const h of hits) lines.push(`${h}: ${c.name} still ${c.old} (now ${c.new} in ${c.file})`);
   }
-  if (lines.length) return { id: 'literals', stage: 'preflight', status: 'fail', inputs, lines, code: 'PREGATE_LITERALS', detail: `${lines.length} test location(s) still carry an old literal`, fix: 'update each listed test to the new value (nightly- and Heavy-only files included), or keep the old constant' };
-  return { id: 'literals', stage: 'preflight', status: 'pass', inputs, detail: `${changes.length} changed literal(s), no test still carries an old value (${testFiles.length} test-tree files scanned)` };
+  const waivers = notes.length ? `; ${notes.length} line(s) waived by pregate-literal markers` : '';
+  if (lines.length) return { id: 'literals', stage: 'preflight', status: 'fail', inputs, lines: [...lines, ...notes], code: 'PREGATE_LITERALS', detail: `${lines.length} test location(s) still carry an old literal${waivers}`, fix: "update each listed test to the new value (nightly- and Heavy-only files included), keep the old constant, or, when the old value is the test's input on purpose, mark that line with `// pregate-literal: NAME=<old value> <reason>`" };
+  return { id: 'literals', stage: 'preflight', status: 'pass', inputs, ...(notes.length ? { lines: notes } : {}), detail: `${changes.length} changed literal(s), no test still carries an old value (${testFiles.length} test-tree files scanned)${waivers}` };
 }
 
 export const literalsCheck: CheckDef = { id: 'literals', stage: 'preflight', tier: 1, downgradable: true, needsRepoCommands: false, run };

test/pregate-selection.test.ts

@@ -14,7 +14,7 @@ import * as fs from 'node:fs';
 import * as os from 'node:os';
 import * as path from 'node:path';
 import { EXIT } from '../lib/headless-artifacts';
-import { changedLiterals, constantsIn, consumersOf } from '../lib/pregate/checks/literals';
+import { changedLiterals, constantsIn, consumersOf, type LiteralWaiver } from '../lib/pregate/checks/literals';
 import { parseSummary, planRuns } from '../lib/pregate/checks/lanes';
 import { DEFAULT_CONFIG, validateConfig } from '../lib/pregate/config';
 import { isDocsEmptyLane } from '../lib/pregate/docs-lane';
@@ -263,6 +263,43 @@ describe('literals (pure)', () => {
     expect(consumersOf(root, Object.keys(files), { name: 'attempts', old: '1', new: '3', file: 'lanes/review-noloss/judge-run.ts' })).toEqual(['tests/judge-run.test.ts:3']);
     fs.rmSync(root, { recursive: true, force: true });
   });
+  // Value: protects=a test that keeps an old value on purpose (the refused input after a minimum-version bump) can waive that one line
+  //   with a `pregate-literal: NAME=value reason` comment, while every other old-value line, a marker for another constant or value,
+  //   a marker with no reason and marker text inside a string still count; fails_when=the waiver widens past its line or a stray
+  //   marker clears a real consumer (#3490, gbrain#7690); why_new=the only escape was warn: ["literals"] for every constant;
+  //   seam=none; no_claim=the policy file (no key exists; see the config test)
+  test('consumersOf: a pregate-literal marker waives only its own line, for its own constant and old value, with a reason (#3490)', () => {
+    const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-literals-waiver-'));
+    const nl = String.fromCharCode(10);
+    const files: Record<string, string> = {
+      'test/runtime.test.ts': [
+        "import { MIN_RUNTIME, checkRuntime } from '../src/runtime';",
+        "expect(checkRuntime('1.4.0')).toBe('refused'); // pregate-literal: MIN_RUNTIME='1.4.0' refused input below the minimum",
+        "// pregate-literal: MIN_RUNTIME=1.4.0 the oldest runtime we still name in the error",
+        "expect(message('1.4.0')).toContain('upgrade');",
+        "expect(MIN_RUNTIME).toBe('1.4.0');",
+        ''].join(nl),
+      'test/other-name.test.ts': ["import { MIN_RUNTIME } from '../src/runtime';", "expect(checkRuntime('1.4.0')).toBe('refused'); // pregate-literal: MAX_RUNTIME='1.4.0' wrong constant", ''].join(nl),
+      'test/other-value.test.ts': ["import { MIN_RUNTIME } from '../src/runtime';", "expect(checkRuntime('1.4.0')).toBe('refused'); // pregate-literal: MIN_RUNTIME='1.3.9' wrong value", ''].join(nl),
+      'test/no-reason.test.ts': ["import { MIN_RUNTIME } from '../src/runtime';", "expect(checkRuntime('1.4.0')).toBe('refused'); // pregate-literal: MIN_RUNTIME='1.4.0'", ''].join(nl),
+      'test/in-string.test.ts': ["import { MIN_RUNTIME } from '../src/runtime';", "const src = \"expect(v('1.4.0')); // pregate-literal: MIN_RUNTIME='1.4.0' looks waived\"; check('1.4.0');", ''].join(nl),
+      'test/two-above.test.ts': ["import { MIN_RUNTIME } from '../src/runtime';", "// pregate-literal: MIN_RUNTIME='1.4.0' too far above", '', "expect(checkRuntime('1.4.0')).toBe('refused');", ''].join(nl),
+      'test/all-waived.test.ts': ["import { MIN_RUNTIME } from '../src/runtime';", "expect(checkRuntime('1.4.0')).toBe('refused'); # pregate-literal: MIN_RUNTIME='1.4.0' refused input", ''].join(nl),
+    };
+    for (const [rel, text] of Object.entries(files)) { fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); fs.writeFileSync(path.join(root, rel), text); }
+    const change = { name: 'MIN_RUNTIME', old: "'1.4.0'", new: "'1.4.4'", file: 'src/runtime.ts' };
+    const waived: LiteralWaiver[] = [];
+    const base = (rel: string) => rel === 'test/runtime.test.ts' ? files[rel]!.split(nl).slice(0, 2).join(nl) : null;
+    expect(consumersOf(root, Object.keys(files), change, base, waived)).toEqual([
+      'test/runtime.test.ts:5', 'test/other-name.test.ts:2', 'test/other-value.test.ts:2', 'test/no-reason.test.ts:2', 'test/in-string.test.ts:2', 'test/two-above.test.ts:4',
+    ]);
+    expect(waived).toEqual([
+      { at: 'test/runtime.test.ts:2', reason: 'refused input below the minimum', added: false },
+      { at: 'test/runtime.test.ts:4', reason: 'the oldest runtime we still name in the error', added: true },
+      { at: 'test/all-waived.test.ts:2', reason: 'refused input', added: true },
+    ]);
+    fs.rmSync(root, { recursive: true, force: true });
+  });
   test('consumersOf: a history row is skipped only when this branch added the new-value row to the same table; other old-value lines still count (GST-344)', () => {
     const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-literals-history-'));
     const nl = String.fromCharCode(10);
@@ -465,6 +502,19 @@ describe('gstack-pregate (bin)', () => {
     expect(r.err).toContain('(PREGATE_LITERALS)');
   });
 
+  // Value: protects=end to end, a marked refused-input line passes literals and the run names the waiver, flagging one this branch
+  //   added; a branch policy cannot add a literals waiver; fails_when=the check ignores markers, hides a waiver, or the policy grows a
+  //   literals key (#3490); why_new=no waiver existed; seam=none; no_claim=gstack-gate-diff tagging
+  test('#3490: a pregate-literal marker passes literals for its line and the run lists the waiver as added on this branch', () => {
+    const repo = fixture();
+    commit(repo, { 'test/nightly/retries.test.js': "const test = require('node:test');\nconst assert = require('node:assert');\nconst { MAX_RETRIES, retry } = require('../../lib/retry.js');\n// pregate-literal: MAX_RETRIES=3 the old cap is the input the guard must refuse\ntest('refuses 3', () => assert.throws(() => retry(() => 0, { MAX_RETRIES: 3 })));\n" });
+    commit(repo, { 'lib/retry.js': "exports.MAX_RETRIES = 4;\nexports.TIMEOUT_MS = 5000;\nexports.retry = (fn) => fn();\n", 'test/retry.test.js': "const test = require('node:test');\nconst assert = require('node:assert');\nconst { MAX_RETRIES } = require('../lib/retry.js');\ntest('retries four times', () => assert.strictEqual(MAX_RETRIES, 4));\n" });
+    const r = pregate(repo, ['--stage', 'preflight', '--only', 'literals']);
+    expect(r.status).toBe(EXIT.ok);
+    expect(r.out).toContain('literals: waived: test/nightly/retries.test.js:5: MAX_RETRIES 3 (the old cap is the input the guard must refuse; marker added on this branch)');
+    expect(validateConfig({ ...BASE_PREGATE, literals: { allow: [{ name: 'MAX_RETRIES', value: '3' }] } }).map(e => e.path)).toEqual(['literals']);
+  });
+
   test('GST-344: a history table the branch extends passes literals through the merge-base read; one it does not extend fails', () => {
     const history = (...ns: number[]) => ["const { MAX_RETRIES } = require('../lib/retry.js');", 'const versions = [', ...ns.map(n => `  [${n}, 'digest-${n}'],`), '];', ''].join('\n');
     const repo = fixture({ baseFiles: { 'test/history.test.js': history(2, 3) } });