mirror of
https://github.com/garrytan/gstack.git
synced 2026-10-31 22:14:25 +01:00
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>
This commit is contained in:
1 parent
f47aba6fe7
commit
bf9a417d54
10 files changed
+59
-29
No files matched your search
+2
-2
@@ -6,12 +6,12 @@
|
||||
|
||||
### Changed
|
||||
- **`gstack-ship-receipt carry`**: an output of an entry with a `command` in origin/<base>'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
|
||||
|
||||
@@ -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/<base>'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[] }
|
||||
|
||||
+6
-5
@@ -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.
|
||||
|
||||
+1
-1
@@ -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`) |
|
||||
|
||||
@@ -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
|
||||
|
||||
+18
-9
@@ -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(); }
|
||||
|
||||
@@ -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',
|
||||
|
||||
+1
-1
@@ -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. */
|
||||
|
||||
@@ -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']);
|
||||
|
||||
@@ -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 } });
|
||||
|
||||
Reference in new issue
Block a user