From bf9a417d542d976dfd9f36ec9e65e35cf790cc46 Mon Sep 17 00:00:00 2001 From: garrytan Date: Wed, 28 Oct 2026 23:54:37 +0000 Subject: [PATCH] fix: prove generated outputs in a second regen run; literal names only The regen verdict comes from the unmodified run again, so a generator that reads its own registered output (gen-skill-docs and qa/templates/functional-report-template.md) passes as before; the proof is a second run with the touched outputs deleted, and its failure only means not proven. The carry ignores a proven path with glob syntax, since it uses the paths as pathspec globs (pre-landing review, #3564 pass 2). Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- CHANGELOG.md | 4 ++-- bin/gstack-ship-receipt | 5 +++-- docs/pregate.md | 11 +++++----- docs/ship-policy.md | 2 +- docs/troubleshooting.md | 3 ++- lib/regen.ts | 27 ++++++++++++++++--------- lib/result-codes-release.ts | 2 +- lib/ship-receipt.ts | 2 +- test/gstack-regen.test.ts | 17 +++++++++++++++- test/ship-receipt-carry-rewrite.test.ts | 15 ++++++++------ 10 files changed, 59 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 77ba60dc3..e3e7b2ea3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,12 +6,12 @@ ### Changed - **`gstack-ship-receipt carry`**: an output of an entry with a `command` in origin/'s `.gstack/generated.json` counts as `generated`, not product, when the `--pregate` result for HEAD lists it under the `regen` check's new `proven` paths and that check ran the same base registry. Without that evidence, for a file that matches an output glob but that the command never writes, and for an entry with no command or one only the branch registers, the output stays product. -- **`gstack-pregate` `regen` check**: each touched output is deleted in the scratch worktree before its entry's command runs, and the ones the command recreates byte-identically are recorded as `proven`. Pass and fail are unchanged. +- **`gstack-pregate` `regen` check**: a fresh entry with touched outputs runs its command a second time in the scratch worktree with those outputs deleted, and records the ones it recreates byte-identically as `proven`. The verdict still comes from the first run, so a generator that reads its own output passes as before and proves nothing. - **Carry across rewritten history**: `write` records `head_tree` and `product_patch_id`, and each carry rebinds both and keeps earlier trees in `carry.trees`. A head that is no longer an ancestor carries only when its tree is one the receipt covers and its product patch-id against the current base matches; it records `carry.ancestry: rewritten` and needs no old commit, so a fresh clone carries too. A different tree is the new `RECEIPT_CARRY_TREE_MISMATCH`. The reviewed tree on a base that moved under it is `RECEIPT_CARRY_PRODUCT_CHANGED`. - **Docs**: docs/ship-policy.md#receipt-carry opens with a four-row table: merge with the product unchanged, merge that changes product, rebase onto a covered tree, and any other rebase. docs/captain.md and captain/templates/CREW.md say to merge main in and never rebase a reviewed branch, and the troubleshooting entries for the carry refusals match the new rules. ### Tests -- `test/ship-receipt-carry-rewrite.test.ts`: a rebase onto the covered tree carries from a fresh clone that never fetched the old head, a different tree refuses with `RECEIPT_CARRY_TREE_MISMATCH`, the reviewed tree on a moved base refuses, and a merge that changes only a registered output's diff carries with a regen-fresh pregate and refuses without one. +- `test/ship-receipt-carry-rewrite.test.ts`: a rebase onto the covered tree carries from a fresh clone that never fetched the old head, a different tree refuses with `RECEIPT_CARRY_TREE_MISMATCH`, the reviewed tree on a moved base refuses, and a merge that changes only a registered output's diff carries with a pregate whose regen check proved it and refuses without one or with a pregate from another registry, and a hand-written file under an output glob that the generator never writes stays product. - `test/gstack-ship-receipt.test.ts`: the written receipt's key order includes `head_tree` and `product_patch_id`. ## [1.91.299.0] - 2026-10-28 diff --git a/bin/gstack-ship-receipt b/bin/gstack-ship-receipt index 53391b011..d3e2c342b 100755 --- a/bin/gstack-ship-receipt +++ b/bin/gstack-ship-receipt @@ -389,7 +389,8 @@ function pregateFieldFor(repoRoot: string, file: string, head: string): { ok: tr * passed for HEAD proved (its regen check deleted each touched output and the * entry's command recreated it byte-identically), when that check ran * origin/'s `.gstack/generated.json` at the commit this carry reads and the - * path is an output of an entry with a `command` there. Anything else is product. + * path is an output of an entry with a `command` there. A proven name with glob + * syntax is product too: the carry uses these paths as pathspec globs. */ function verifiedOutputs(repoRoot: string, base: string, pregateFile: string | undefined): string[] { if (!pregateFile) return []; @@ -398,7 +399,7 @@ function verifiedOutputs(repoRoot: string, base: string, pregateFile: string | u const load = loadRegistryFromBase(repoRoot, base); if (isRegistryUnavailable(load) || load.errors.length || regen.inputs?.registry !== load.source.label) return []; const owned = load.registry.entries.filter(e => e.command).flatMap(e => e.outputs); - return regen.proven.filter(p => matchesAny(owned, p)); + return regen.proven.filter(p => !/[*?[\]\\]/.test(p) && matchesAny(owned, p)); } interface ProductRules { extra: { metadata: string[]; outputs: readonly string[] }; metadata: string[]; templates: readonly string[] } diff --git a/docs/pregate.md b/docs/pregate.md index 2677f75b7..11740fcea 100644 --- a/docs/pregate.md +++ b/docs/pregate.md @@ -146,11 +146,12 @@ Two stages with separate clocks, both printed: ~2.5 s for `regen` and under 200 ms for each of the others): - `regen`: every registry entry whose inputs or outputs are touched is fresh - (`gstack-regen check` in a scratch worktree). Each touched output is deleted - before its entry's command runs; the ones the command recreates - byte-identically are recorded as the check's `proven` paths, which - `gstack-ship-receipt carry` reads. An output it does not recreate is neither - stale nor proven. + (`gstack-regen check` in a scratch worktree). A fresh entry with touched + outputs then runs once more with those outputs deleted; the ones the command + recreates byte-identically are the check's `proven` paths, which + `gstack-ship-receipt carry` reads. The verdict comes from the first run: an + output the second run does not recreate, or a second run that fails, is + simply not proven. - `secrets`: `gstack-redact` over the branch's added lines (merge-base to the working tree, untracked files included). HIGH, or MEDIUM in the `secret` category, fails; other MEDIUM findings warn. Cannot be downgraded. diff --git a/docs/ship-policy.md b/docs/ship-policy.md index e767904bb..8e31b960e 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -896,7 +896,7 @@ receipt when the base moves: | What changed since /ship | The receipt | |---|---| -| a merge of the base, product diff unchanged (tests, release metadata, registered outputs the pregate regenerated fresh) | carries; no review reruns | +| a merge of the base, product diff unchanged (tests, release metadata, registered outputs the pregate's regen check proved) | carries; no review reruns | | a merge of the base whose resolution changed product code | a fresh /ship (`RECEIPT_CARRY_PRODUCT_CHANGED`) | | a rebase or force-push onto a tree the receipt covers, product diff against the new base unchanged | carries (`carry.ancestry: rewritten`) | | any other rebase or force-push | a fresh /ship (`RECEIPT_CARRY_TREE_MISMATCH`) | diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index e180fbf97..b82dd8f86 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -4726,7 +4726,8 @@ instead keeps every later carry available **Meaning.** Between the receipt's head and HEAD the product diff against the base changed: at least one path that is neither a test, release metadata, a -registered output the pregate regenerated fresh, nor part of the base merge has +registered output the pregate's regen check proved (listed under `proven`, +checked against the same base registry), nor part of the base merge has a different patch than the reviewed head had. After a rebase it means HEAD has the reviewed tree but the base moved under it, so the diff a reviewer would see is not the reviewed one. The reviewed verdict does not cover HEAD, so the diff --git a/lib/regen.ts b/lib/regen.ts index 6654c854e..4760e5573 100644 --- a/lib/regen.ts +++ b/lib/regen.ts @@ -164,7 +164,7 @@ export interface EntryResult { exit?: number | null; stderr?: string; ms: number; /** The exact command an author runs to refresh the entry. */ fix: string; - /** `check` with `prove`, on a fresh entry: the deleted outputs the command recreated byte-identically (an output it never writes is not proven). */ + /** `check` with `prove`, on a fresh entry: the deleted outputs the proof run recreated byte-identically (an output it never writes, or a proof run that failed, proves nothing). */ proven?: string[]; } @@ -216,7 +216,7 @@ export interface RunRegenOptions { only?: string[]; /** Restrict to entries whose inputs or outputs intersect these paths. */ touched?: string[]; - /** `check`: outputs to delete from the scratch before their entry's command runs; the ones it recreates byte-identically come back as `proven`. */ + /** `check`: outputs to prove on a fresh entry: a second run with each one deleted first, the ones it recreates byte-identically come back as `proven`. The verdict comes from the first run alone. */ prove?: readonly string[]; env?: NodeJS.ProcessEnv; timeoutMs?: number; } @@ -244,21 +244,30 @@ export function runRegen(o: RunRegenOptions): { results: EntryResult[]; wtree: s if (!e.command) { results.push({ id: e.id, command: null, status: 'listed', changed: [], outside: [], ms: 0, fix: fixFor(e) }); continue; } const cwd = o.mode === 'check' ? scratch!.dir : o.repoRoot; const before = o.mode === 'write' ? snapshotFiles(o.repoRoot) : null; - const probe = o.mode === 'check' ? (o.prove ?? []).filter(p => matchesAny(e.outputs, p) && fs.existsSync(path.join(cwd, p))) : []; - for (const p of probe) fs.rmSync(path.join(cwd, p), { force: true }); const run = runCommand(cwd, e.command, o.env ?? process.env, timeout); const after = before ? diffSnapshots(before, snapshotFiles(o.repoRoot)) : changedPaths(cwd); - const unwritten = probe.filter(p => !fs.existsSync(path.join(cwd, p))); - const changed = after.filter(p => matchesAny(e.outputs, p) && !unwritten.includes(p)); + const changed = after.filter(p => matchesAny(e.outputs, p)); const outside = after.filter(p => !matchesAny(e.outputs, p) && !matchesAny(o.registry.entries.flatMap(x => x.outputs), p)); const ms = Date.now() - started; if (run.exit !== 0) { results.push({ id: e.id, command: e.command, status: 'failed', changed, outside, exit: run.exit, stderr: run.stderr, ms, fix: fixFor(e) }); continue; } if (o.mode === 'check') { const differs = changed.length > 0 || outside.length > 0; - if (differs && install) results.push({ id: e.id, command: e.command, status: 'deps-missing', changed, outside, exit: 0, ms, fix: `run ${install}, then rerun the check` }); - else results.push({ id: e.id, command: e.command, status: differs ? 'stale' : 'fresh', changed, outside, exit: 0, ms, fix: fixFor(e), ...(probe.length && !differs ? { proven: probe.filter(p => !unwritten.includes(p)) } : {}) }); // Reset the scratch so the next entry's diff is its own. - git(cwd, ['checkout', '-q', '--', '.']); git(cwd, ['clean', '-fdq']); + const reset = () => { git(cwd, ['checkout', '-q', '--', '.']); git(cwd, ['clean', '-fdq']); }; + reset(); + if (differs && install) { results.push({ id: e.id, command: e.command, status: 'deps-missing', changed, outside, exit: 0, ms, fix: `run ${install}, then rerun the check` }); continue; } + // The proof is a second run apart from the verdict: each touched output deleted first, the ones recreated + // byte-identically are proven, and a failure (a generator that reads its own output) proves nothing. + const probe = differs ? [] : (o.prove ?? []).filter(p => matchesAny(e.outputs, p) && fs.existsSync(path.join(cwd, p))); + let proven: string[] | undefined; + if (probe.length) { + for (const p of probe) fs.rmSync(path.join(cwd, p), { force: true }); + const proof = runCommand(cwd, e.command, o.env ?? process.env, timeout); + const redo = changedPaths(cwd); + proven = proof.exit === 0 ? probe.filter(p => !redo.includes(p)) : []; + reset(); + } + results.push({ id: e.id, command: e.command, status: differs ? 'stale' : 'fresh', changed, outside, exit: 0, ms, fix: fixFor(e), ...(proven ? { proven } : {}) }); } else results.push({ id: e.id, command: e.command, status: changed.length ? 'written' : 'fresh', changed, outside, exit: 0, ms, fix: fixFor(e) }); } } finally { scratch?.remove(); } diff --git a/lib/result-codes-release.ts b/lib/result-codes-release.ts index 27cab8d9b..51eccaa22 100644 --- a/lib/result-codes-release.ts +++ b/lib/result-codes-release.ts @@ -262,7 +262,7 @@ export const RELEASE_RESULT_CODES = { RECEIPT_CARRY_PRODUCT_CHANGED: { anchor: 'receipt-carry-product-changed', summary: 'the product diff against the base changed between the receipt\'s head and HEAD (a product file differs beyond a base merge, a test-only fix, release metadata or a regenerated output), so the reviewed verdict does not cover HEAD', - fix: 'run /ship on the new head; the receipt carries only when every changed path is a test, release metadata, a registered output the pregate regenerated fresh, or the base merge, and the product patch-id is unchanged', + fix: 'run /ship on the new head; the receipt carries only when every changed path is a test, release metadata, a registered output the pregate regen check proved, or the base merge, and the product patch-id is unchanged', }, DOCS_AUDIT_BLOCKED: { anchor: 'docs-audit-blocked', diff --git a/lib/ship-receipt.ts b/lib/ship-receipt.ts index ba6a03f31..aa86abf11 100644 --- a/lib/ship-receipt.ts +++ b/lib/ship-receipt.ts @@ -150,7 +150,7 @@ export const CARRY_METADATA_GLOBS: readonly string[] = ['VERSION', 'CHANGELOG.md /** How many differing product paths a RECEIPT_CARRY_PRODUCT_CHANGED refusal names, and how far the per-path patch-id walk goes. */ export const CARRY_NAMED_PATHS = 20; -/** `generated`: an output of a base-registry entry with a command, which the pregate for HEAD regenerated fresh (its inputs are product). */ +/** `generated`: an output of a base-registry entry with a command, which the pregate for HEAD proved its command recreates (its inputs are product). */ export type CarryClass = 'product' | 'test' | 'metadata' | 'generated'; export interface CarryPlanInput { /** The reviewed head the receipt names. */ diff --git a/test/gstack-regen.test.ts b/test/gstack-regen.test.ts index f718d6ec9..3f36b5084 100644 --- a/test/gstack-regen.test.ts +++ b/test/gstack-regen.test.ts @@ -9,7 +9,7 @@ import { afterEach, describe, expect, test } from 'bun:test'; import * as fs from 'node:fs'; import * as path from 'node:path'; import { EXIT } from '../lib/headless-artifacts'; -import { missingDependencies, parseRegistry, selectEntries, validateRegistry } from '../lib/regen'; +import { missingDependencies, parseRegistry, runRegen, selectEntries, validateRegistry } from '../lib/regen'; import { BASE_REGISTRY, cleanup, commit, makeFixtureRepo, readFile, runBin, write, type FixtureRepo } from './helpers/pregate-fixture'; const repos: FixtureRepo[] = []; @@ -135,6 +135,21 @@ describe('gstack-regen (bin)', () => { expect(parsed.wtree).toMatch(/^[0-9a-f]{40}$/); }); + // Value: protects=the receipt carry's generated class (only outputs a generator recreates) and a generator that reads + // its own registered output (gen-skill-docs and qa/templates/functional-report-template.md); fails_when=the proof + // deletes outputs before the verdict run, or proves a path the command never wrote; seam=runRegen; no_claim=- + test('prove: a writer proves its touched output; a generator that reads its own output stays fresh and proves nothing', () => { + const registryOf = (repo: FixtureRepo) => parseRegistry(fs.readFileSync(path.join(repo.work, '.gstack/generated.json'), 'utf8')).registry; + const writer = fixture(); + commit(writer, { 'src/version.js': "module.exports = { VERSION: '1.1.0' };\n", 'generated/digest.md': '# digest v1.1.0\n', 'generated/notes.md': '# hand-written\n' }); + expect(runRegen({ repoRoot: writer.work, registry: registryOf(writer), mode: 'check', only: ['digest'], prove: ['generated/digest.md', 'generated/notes.md', 'src/version.js'] }).results) + .toMatchObject([{ id: 'digest', status: 'fresh', proven: ['generated/digest.md'] }]); + const selfRead = fixture({ registry: { entries: [{ id: 'self', command: 'node selfread.js', outputs: ['generated/**'], inputs: ['selfread.js'] }] }, baseFiles: { 'selfread.js': "const fs=require('fs');fs.writeFileSync('generated/digest.md',fs.readFileSync('generated/digest.md','utf8'));\n" } }); + commit(selfRead, { 'generated/digest.md': '# hand edit\n' }); + expect(runRegen({ repoRoot: selfRead.work, registry: registryOf(selfRead), mode: 'check', prove: ['generated/digest.md'] }).results) + .toMatchObject([{ id: 'self', status: 'fresh', proven: [] }]); + }); + test('a failing command is failed (exit 1, REGEN_COMMAND_FAILED, status incomplete); an output outside the declared globs is stale', () => { const repo = fixture({ registry: { entries: [{ id: 'boom', command: 'node -e "process.exit(7)"', outputs: ['generated/**'] }, { id: 'leak', command: 'node -e "require(\'fs\').writeFileSync(\'docs/extra.md\',\'x\')"', outputs: ['generated/**'] }] } }); const r = runBin(repo, 'gstack-regen', ['check', '--base', 'main', '--allow-repo-commands']); diff --git a/test/ship-receipt-carry-rewrite.test.ts b/test/ship-receipt-carry-rewrite.test.ts index 83009c4a0..c98a8ebf1 100644 --- a/test/ship-receipt-carry-rewrite.test.ts +++ b/test/ship-receipt-carry-rewrite.test.ts @@ -123,12 +123,15 @@ describe('carry across a base merge that changes a registered output', () => { const pregate = JSON.parse(fs.readFileSync(pregateJson, 'utf8')); const regen = pregate.checks.find((c: { id: string }) => c.id === 'regen'); expect(regen).toMatchObject({ status: 'pass', proven: ['generated/digest.md'] }); - const headRegistry = path.join(repo.root, 'pregate-head-registry.json'); - regen.inputs.registry = 'head (unreviewed)'; - fs.writeFileSync(headRegistry, JSON.stringify(pregate)); - const foreign = carry(repo, body, ['--pregate', headRegistry]); - expect(foreign.out).toContain('RECEIPT_CARRY_PRODUCT_CHANGED'); - expect(foreign.err).toContain('generated/digest.md'); + const tampered = path.join(repo.root, 'pregate-tampered.json'); + const label = regen.inputs.registry; + for (const [registry, proven] of [['head (unreviewed)', ['generated/digest.md']], [label, ['generated/*']]]) { + Object.assign(regen, { proven, inputs: { ...regen.inputs, registry } }); + fs.writeFileSync(tampered, JSON.stringify(pregate)); + const refused = carry(repo, body, ['--pregate', tampered]); + expect(refused.out).toContain('RECEIPT_CARRY_PRODUCT_CHANGED'); + expect(refused.err).toContain('generated/digest.md'); + } const out = carry(repo, body, ['--pregate', pregateJson]); expect(out.status).toBe(EXIT.ok); expect(receiptIn(body)).toMatchObject({ head: repo.git('rev-parse', 'HEAD'), carry: { from: reviewed } });