diff --git a/scripts/bump-harness-version.ts b/scripts/bump-harness-version.ts index 4ce1ef8ac..d047fabbc 100644 --- a/scripts/bump-harness-version.ts +++ b/scripts/bump-harness-version.ts @@ -41,7 +41,11 @@ function readPins(root: string): Record { for (const rel of (fs.readdirSync(dir, { recursive: true }) as string[]).map(f => f.split(path.sep).join('/')).sort()) { if (fs.statSync(path.join(dir, rel)).isDirectory()) continue; if (!rel.endsWith('.json')) throw new Error(`${PINS_DIR}/${rel}: every harness pin is .json`); - files[rel.slice(0, -'.json'.length)] = JSON.parse(fs.readFileSync(path.join(dir, rel), 'utf8')); + try { + files[rel.slice(0, -'.json'.length)] = JSON.parse(fs.readFileSync(path.join(dir, rel), 'utf8')); + } catch (e) { + throw new Error(`${PINS_DIR}/${rel}: ${(e as Error).message}`); + } } return files; } @@ -101,9 +105,8 @@ export function decide(manifest: HarnessManifest, files: TreeEntry[], decision: : manifest.files[entry.file]!])) }; } -/** Write the manifest head and one pin file per harness file, rewriting only pins whose bytes change and deleting dropped ones. */ +/** Write one pin file per harness file (rewriting only pins whose bytes change, deleting dropped ones), then the manifest head, so a failed pin write never moves the version. */ export function writeManifest(root: string, manifest: HarnessManifest): void { - fs.writeFileSync(path.join(root, MANIFEST), `${JSON.stringify({ version: manifest.version, patterns: manifest.patterns }, null, 2)}\n`); for (const file of Object.keys(readPins(root))) if (!manifest.files[file]) fs.rmSync(pinPath(root, file)); for (const [file, entry] of Object.entries(manifest.files)) { const target = pinPath(root, file), text = `${JSON.stringify(entry)}\n`; @@ -111,6 +114,7 @@ export function writeManifest(root: string, manifest: HarnessManifest): void { fs.mkdirSync(path.dirname(target), { recursive: true }); fs.writeFileSync(target, text); } + fs.writeFileSync(path.join(root, MANIFEST), `${JSON.stringify({ version: manifest.version, patterns: manifest.patterns }, null, 2)}\n`); } if (import.meta.main) { diff --git a/test/harness-version.test.ts b/test/harness-version.test.ts index d2be99295..5ccd013cd 100644 --- a/test/harness-version.test.ts +++ b/test/harness-version.test.ts @@ -18,7 +18,7 @@ import * as fs from 'node:fs'; import * as os from 'node:os'; import { LIVE_REGISTRY, type Registry } from '../scripts/eval-flake-rank'; import { matchGlob } from './helpers/test-selection'; -import { mergePinEdits } from './helpers/pin-merge-fixture'; +import { mergePinEdits, type PinMerge } from './helpers/pin-merge-fixture'; const ROOT = path.resolve(import.meta.dir, '..'); @@ -75,6 +75,9 @@ describe('HARNESS_VERSION guard', () => { fs.writeFileSync(path.join(dir, PINS_DIR, 'z.ts.txt'), 'stray\n'); expect(() => readManifest(dir)).toThrow('every harness pin is .json'); fs.rmSync(path.join(dir, PINS_DIR, 'z.ts.txt')); + fs.writeFileSync(path.join(dir, PINS_DIR, 'z.ts.json'), '<<<<<<< ours\n'); + expect(() => readManifest(dir)).toThrow(`${PINS_DIR}/z.ts.json: `); + fs.writeFileSync(path.join(dir, PINS_DIR, 'z.ts.json'), '{"blob":"z","since":1}\n'); fs.writeFileSync(path.join(dir, MANIFEST), JSON.stringify({ version: 3, patterns: [], files: {} })); expect(() => readManifest(dir)).toThrow('holds only version and patterns'); } finally { @@ -113,9 +116,12 @@ describe('HARNESS_VERSION guard', () => { writeManifest(root, decide(manifest, files, 'non-behavioral', reason)); }; const floor = 'test/helpers/pty/runners/floor.ts', observation = 'test/helpers/pty/runners/observation.ts'; - const clean = mergePinEdits(copyManifest, record(floor, 'a'.repeat(40), 'floor runner logging only'), record(observation, 'b'.repeat(40), 'observation runner comment only')); - const conflict = mergePinEdits(copyManifest, record(floor, 'a'.repeat(40), 'floor runner logging only'), record(floor, 'c'.repeat(40), 'floor runner rename only')); + const merges: PinMerge[] = []; try { + const clean = mergePinEdits(copyManifest, record(floor, 'a'.repeat(40), 'floor runner logging only'), record(observation, 'b'.repeat(40), 'observation runner comment only')); + merges.push(clean); + const conflict = mergePinEdits(copyManifest, record(floor, 'a'.repeat(40), 'floor runner logging only'), record(floor, 'c'.repeat(40), 'floor runner rename only')); + merges.push(conflict); expect(clean.output).not.toContain('CONFLICT'); expect(clean.clean).toBe(true); expect(JSON.parse(clean.read(`${PINS_DIR}/${floor}.json`))).toEqual({ blob: 'a'.repeat(40), since: live.version, nonBehavioral: 'floor runner logging only' }); @@ -124,7 +130,7 @@ describe('HARNESS_VERSION guard', () => { expect(conflict.clean).toBe(false); expect(conflict.output).toContain(`CONFLICT (content): Merge conflict in ${PINS_DIR}/${floor}.json`); } finally { - for (const merge of [clean, conflict]) fs.rmSync(merge.root, { recursive: true, force: true }); + for (const merge of merges) fs.rmSync(merge.root, { recursive: true, force: true }); } }); }); diff --git a/test/helpers/pin-merge-fixture.ts b/test/helpers/pin-merge-fixture.ts index 3db410a56..8189b7280 100644 --- a/test/helpers/pin-merge-fixture.ts +++ b/test/helpers/pin-merge-fixture.ts @@ -21,15 +21,21 @@ export function mergePinEdits(seed: (root: string) => void, a: (root: string) => if (result.status !== 0) throw new Error(`git ${args.join(' ')} failed (${result.status}): ${result.stderr}`); return result.stdout; }; - must('init', '-q', '-b', 'base'); const commit = (edit: (root: string) => void, message: string) => { edit(root); must('add', '-A'); must('commit', '-q', '--no-verify', '-m', message); }; - commit(seed, 'seed'); - for (const [branch, edit] of [['a', a], ['b', b]] as const) { - must('checkout', '-q', '-b', branch, 'base'); - commit(edit, branch); + let merged: ReturnType; + try { + must('init', '-q', '-b', 'base'); + commit(seed, 'seed'); + for (const [branch, edit] of [['a', a], ['b', b]] as const) { + must('checkout', '-q', '-b', branch, 'base'); + commit(edit, branch); + } + merged = git('merge-tree', '--write-tree', 'a', 'b'); + if (merged.status !== 0 && merged.status !== 1) throw new Error(`git merge-tree failed (${merged.status}): ${merged.stderr}`); + } catch (e) { + fs.rmSync(root, { recursive: true, force: true }); + throw e; } - const merged = git('merge-tree', '--write-tree', 'a', 'b'); - if (merged.status !== 0 && merged.status !== 1) throw new Error(`git merge-tree failed (${merged.status}): ${merged.stderr}`); const tree = merged.stdout.split('\n')[0]!; return { root, clean: merged.status === 0, output: merged.stdout, read: rel => must('show', `${tree}:${rel}`) }; } diff --git a/test/review-packets-removed.test.ts b/test/review-packets-removed.test.ts index c3cdc081b..2c520f690 100644 --- a/test/review-packets-removed.test.ts +++ b/test/review-packets-removed.test.ts @@ -11,7 +11,7 @@ import * as fs from 'fs'; import * as os from 'os'; import * as path from 'path'; import { sectionTexts } from '../lib/review-brief'; -import { mergePinEdits } from './helpers/pin-merge-fixture'; +import { mergePinEdits, type PinMerge } from './helpers/pin-merge-fixture'; const ROOT = path.join(import.meta.dir, '..'); const REMOVED = [ @@ -69,7 +69,11 @@ function readPinDir(dir: string): Record { for (const rel of (fs.readdirSync(dir, { recursive: true }) as string[]).map(f => f.split(path.sep).join('/')).sort()) { if (fs.statSync(path.join(dir, rel)).isDirectory()) continue; if (!rel.endsWith('.json')) throw new Error(`${dir}/${rel}: every pin file is .json`); - out[rel.slice(0, -'.json'.length)] = JSON.parse(fs.readFileSync(path.join(dir, rel), 'utf8')); + try { + out[rel.slice(0, -'.json'.length)] = JSON.parse(fs.readFileSync(path.join(dir, rel), 'utf8')); + } catch (e) { + throw new Error(`${dir}/${rel}: ${(e as Error).message}`); + } } return out; } @@ -97,6 +101,24 @@ function readPins(pins: string) { } type Pins = ReturnType; +/** Every source a reviewer brief, checklist or depth rule comes from: deleting one of these pins fails, never drops the check. */ +const MUST_PIN = [ + 'lib/review-depth.ts', + 'lib/review-risk.ts', + 'review/checklist.md', + 'review/design-checklist.md', + 'review/gate-integrity.md', + 'review/sections/adversarial.md', + 'review/sections/review-army.md', + 'scripts/resolvers/fix-check.ts', + 'ship/sections/adversarial.md', + 'ship/sections/review-army.md', +]; + +/** MUST_PIN entries and specialist checklists under root with no pin. */ +const unpinned = (pins: Pins, root: string) => [...MUST_PIN, ...fs.readdirSync(path.join(root, 'review/specialists')).filter(f => f.endsWith('.md')).map(f => `review/specialists/${f}`)] + .filter(f => !Object.hasOwn(pins.files, f)); + /** Pinned files whose LF-normalized bytes under root no longer hash to their pin. */ const stalePins = (pins: Pins, root: string) => Object.entries(pins.files).filter(([file, pin]) => { const text = fs.readFileSync(path.join(root, file), 'utf8').replace(/\r\n/g, '\n'); @@ -105,7 +127,7 @@ const stalePins = (pins: Pins, root: string) => Object.entries(pins.files).filte /** Pins naming no replay in the store, and replays whose justification names no no-loss replay or re-score. */ const unadmitted = (pins: Pins) => [ - ...Object.entries(pins.files).filter(([, pin]) => !(pin.replay in pins.replays)).map(([file, pin]) => `${file} names unknown replay ${pin.replay}`), + ...Object.entries(pins.files).filter(([, pin]) => !Object.hasOwn(pins.replays, pin.replay)).map(([file, pin]) => `${file} names unknown replay ${pin.replay}`), ...Object.entries(pins.replays).filter(([, what]) => !/no-loss|re-score/.test(what)).map(([replay]) => `replay ${replay} is not a no-loss replay or re-score`), ]; @@ -140,7 +162,11 @@ describe('reviewer briefs stay at their no-loss-tested bytes (GSTA-167)', () => test('the pins cover every specialist checklist', () => { const specialists = fs.readdirSync(path.join(ROOT, 'review/specialists')).filter(f => f.endsWith('.md')).map(f => `review/specialists/${f}`); - expect(specialists.filter(f => !(f in PINNED.files))).toEqual([]); + expect(specialists.filter(f => !Object.hasOwn(PINNED.files, f))).toEqual([]); + }); + + test('every source a reviewer brief, checklist or depth rule comes from has a pin (GST-1084)', () => { + expect(unpinned(PINNED, ROOT)).toEqual([]); }); // Value: protects=the one-file-per-pin layout keeps every check the in-file table had; fails_when=a stale sha, an unknown or @@ -160,13 +186,23 @@ describe('reviewer briefs stay at their no-loss-tested bytes (GSTA-167)', () => const pin = (file: string, value: unknown) => fs.writeFileSync(path.join(pins, 'files', `${file}.json`), `${JSON.stringify(value)}\n`); pin('review/design-checklist.md', { sha: PINNED.files['review/design-checklist.md']!.sha, replay: 'GST-0000' }); + pin('lib/review-risk.ts', { sha: PINNED.files['lib/review-risk.ts']!.sha, replay: 'toString' }); fs.writeFileSync(path.join(pins, 'replays', 'GST-0001.json'), `${JSON.stringify({ replay: 'GST-0001', justification: 'reviewed by hand' })}\n`); - expect(unadmitted(readPins(pins))).toEqual(['review/design-checklist.md names unknown replay GST-0000', 'replay GST-0001 is not a no-loss replay or re-score']); + expect(unadmitted(readPins(pins))).toEqual(['lib/review-risk.ts names unknown replay toString', 'review/design-checklist.md names unknown replay GST-0000', + 'replay GST-0001 is not a no-loss replay or re-score']); + + expect(unpinned(readPins(pins), ROOT)).toEqual([]); + fs.rmSync(path.join(pins, 'files', 'lib/review-risk.ts.json')); + fs.rmSync(path.join(pins, 'files', 'review/specialists/testing.md.json')); + expect(unpinned(readPins(pins), ROOT)).toEqual(['lib/review-risk.ts', 'review/specialists/testing.md']); pin('review/gate-integrity.md', { ...PINNED.files['review/gate-integrity.md'], note: 'extra' }); fs.writeFileSync(path.join(pins, 'replays', 'GST-0002.json'), `${JSON.stringify({ replay: 'GST-0003', justification: 'a no-loss replay' })}\n`); expect(readPins(pins).malformed).toEqual(['replays/GST-0002.json (replay GST-0003 belongs in replays/GST-0003.json)', 'files/review/gate-integrity.md.json (expected exactly sha, replay)']); + fs.writeFileSync(path.join(pins, 'files', 'review/gate-integrity.md.json'), '<<<<<<< a\n{}\n'); + expect(() => readPins(pins)).toThrow('files/review/gate-integrity.md.json: '); + fs.rmSync(path.join(pins, 'files', 'review/gate-integrity.md.json')); fs.writeFileSync(path.join(pins, 'brief-bodies', 'specialist.txt'), 'stray\n'); expect(() => readPins(pins)).toThrow('every pin file is .json'); } finally { @@ -184,9 +220,12 @@ describe('reviewer briefs stay at their no-loss-tested bytes (GSTA-167)', () => }; const seed = (root: string) => fs.cpSync(PINS, root, { recursive: true }); const army = 'ship/sections/review-army.md', adversarial = 'ship/sections/adversarial.md'; - const clean = mergePinEdits(seed, repin(army, 'a'.repeat(64), 'GST-9001+GST-967'), repin(adversarial, 'b'.repeat(64), 'GST-9002')); - const conflict = mergePinEdits(seed, repin(army, 'a'.repeat(64), 'GST-9001'), repin(army, 'c'.repeat(64), 'GST-9003')); + const merges: PinMerge[] = []; try { + const clean = mergePinEdits(seed, repin(army, 'a'.repeat(64), 'GST-9001+GST-967'), repin(adversarial, 'b'.repeat(64), 'GST-9002')); + merges.push(clean); + const conflict = mergePinEdits(seed, repin(army, 'a'.repeat(64), 'GST-9001'), repin(army, 'c'.repeat(64), 'GST-9003')); + merges.push(conflict); expect(clean.output).not.toContain('CONFLICT'); expect(clean.clean).toBe(true); expect(JSON.parse(clean.read(`files/${army}.json`))).toEqual({ sha: 'a'.repeat(64), replay: 'GST-9001+GST-967' }); @@ -196,7 +235,7 @@ describe('reviewer briefs stay at their no-loss-tested bytes (GSTA-167)', () => expect(conflict.clean).toBe(false); expect(conflict.output).toContain(`CONFLICT (content): Merge conflict in files/${army}.json`); } finally { - for (const merge of [clean, conflict]) fs.rmSync(merge.root, { recursive: true, force: true }); + for (const merge of merges) fs.rmSync(merge.root, { recursive: true, force: true }); } }); });