From f47aba6fe73a2be2d1303525d2fffaf9f6144de2 Mon Sep 17 00:00:00 2001 From: garrytan Date: Wed, 28 Oct 2026 23:37:40 +0000 Subject: [PATCH] fix: carry sets aside only outputs the regen check proved it recreates The regen check deletes each touched output in its scratch worktree before the command runs and records the ones recreated byte-identically as proven. gstack-ship-receipt carry exempts only those paths, and only when the check ran origin/'s registry at the commit the carry reads. A file under an output glob that the generator never writes stays product (pre-landing review, #3564 pass 1). Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- CHANGELOG.md | 3 ++- bin/gstack-ship-receipt | 22 ++++++++++-------- docs/pregate.md | 6 ++++- docs/ship-policy.md | 10 ++++++--- lib/headless-artifacts.ts | 4 +++- lib/pregate/checks/regen.ts | 5 +++-- lib/regen.ts | 11 +++++++-- test/ship-receipt-carry-rewrite.test.ts | 30 ++++++++++++++++++++++++- 8 files changed, 71 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f9e7f26e2..77ba60dc3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,8 @@ **A /ship receipt now survives a base merge that only regenerates outputs, and a rebase onto the tree it reviewed.** On gstack-evals, two full /ship reruns came from a merge of main that changed a generated file's diff (U1 paid about 28M tokens for its rerun), and one came from a rebase whose tree was byte-identical to the reviewed one (about 60M). Each re-reviewed code the receipt already covered. For crews the rule is now a single line. Merge main into a reviewed branch, and never rebase it. ### 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 shows the `regen` check passing with those entries fresh. Without that evidence, and for an entry with no command or one only the branch registers, the output stays product. +- **`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. - **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. diff --git a/bin/gstack-ship-receipt b/bin/gstack-ship-receipt index e6d89eaf9..53391b011 100755 --- a/bin/gstack-ship-receipt +++ b/bin/gstack-ship-receipt @@ -63,7 +63,7 @@ const HELP = `gstack-ship-receipt — the \`\`\`gstack-ship-receipt\`\`\` block the P7 adoption number over merged PRs (deduplicated by run and thread) carry --pr [--repo ] | --body [--base ] [--head ] [--pregate ] [--dry-run] [--out ] carry the receipt to HEAD when only a base merge, tests, release metadata or registered - outputs the --pregate regen check proved fresh changed since its head (product patch-id + outputs the --pregate regen check proved (recreated them) changed since its head (product patch-id unchanged), or after a rebase onto a tree the receipt covers with the recorded product patch-id: rewrites the receipt with a \`carry\` record, posts the body (gh pr edit) and prints GSTACK_RESULT: skill=ship status=complete; RECEIPT_CARRY_PRODUCT_CHANGED / @@ -385,18 +385,20 @@ function pregateFieldFor(repoRoot: string, file: string, head: string): { ok: tr } /** - * Generated outputs the carry may set aside as metadata: the outputs of every - * entry with a `command` in origin/'s `.gstack/generated.json`, and only - * when the pregate passed for HEAD shows the regen check regenerating them - * fresh. Without that evidence an output is product like any other file. + * Generated outputs the carry may set aside as metadata: the paths the pregate + * 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. */ function verifiedOutputs(repoRoot: string, base: string, pregateFile: string | undefined): string[] { if (!pregateFile) return []; const regen = readPregateFile(pregateFile).file?.checks.find(c => c.id === 'regen'); - if (!regen || regen.status !== 'pass' || !/\bfresh\b/.test(regen.detail ?? '')) return []; + if (!regen || regen.status !== 'pass' || !regen.proven?.length) return []; const load = loadRegistryFromBase(repoRoot, base); - if (isRegistryUnavailable(load) || load.errors.length) return []; - return load.registry.entries.filter(e => e.command).flatMap(e => e.outputs); + 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)); } interface ProductRules { extra: { metadata: string[]; outputs: readonly string[] }; metadata: string[]; templates: readonly string[] } @@ -445,7 +447,9 @@ function productPatchAgainstBase(repoRoot: string, rules: ProductRules, baseRef: * reran a full /ship on identical product code after a base merge or a * test-only fix; the carry compares the product diff against the base at the * receipt's head and at HEAD by patch-id, classifies every changed path, and - * records the range-diff as evidence in the receipt it rewrites. + * records the range-diff as evidence in the receipt it rewrites. After a rebase + * or force-push it carries only onto a tree the receipt covers whose product + * patch-id against the current base is the recorded one (planTreeCarry). */ function cmdCarry(rest: string[], json: boolean): number { const bodyFile = flag(rest, '--body'); diff --git a/docs/pregate.md b/docs/pregate.md index a42b37831..2677f75b7 100644 --- a/docs/pregate.md +++ b/docs/pregate.md @@ -146,7 +146,11 @@ 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). + (`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. - `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 8890a2f4a..e767904bb 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -933,9 +933,13 @@ edit). A logic change in a stamped source file is product. Lockfiles are product unless the policy names them. An output of an entry with a `command` in origin/'s `.gstack/generated.json` is `generated`, not product, only when the -`--pregate` result passed for HEAD shows the `regen` check passing with those -entries fresh; without that evidence, or for an entry with no command or one -only the branch registers, the output is product (U1 on gstack-evals paid a +`--pregate` result passed for HEAD lists it under the `regen` check's +`proven` paths (the check deleted the touched output in its scratch worktree +and the command recreated it byte-identically) and that check ran the same +registry, origin/ at the commit the carry reads. A file that matches an +output glob but that the command never writes is not proven. Without that +evidence, or for an entry with no command or one only the branch registers, +the output is product (U1 on gstack-evals paid a full rerun because a merge changed its generated index's diff). A base merge contributes no product change of its own. A receipt carried twice keeps the reviewed head as `carry.from` and adds up the merges and paths. The carried receipt records diff --git a/lib/headless-artifacts.ts b/lib/headless-artifacts.ts index 2c37e3dcc..a42338611 100644 --- a/lib/headless-artifacts.ts +++ b/lib/headless-artifacts.ts @@ -69,6 +69,8 @@ export interface PregateCheck { stage?: 'preflight' | 'tests'; inputs?: Record; fix?: string; code?: string; lines?: string[]; // Lane D (GSTA-112): a non-pass names its cause; an environment cause carries its kind and evidence. cause?: 'diff' | 'environment'; environment?: { kind: string; evidence: string; proof?: 'launch' | 'output' }; + /** `regen`: the touched outputs the generator recreated byte-identically from a deleted copy (what a receipt carry may set aside as generated). */ + proven?: string[]; } /** A remote_gate receipt accepted from a stamp-only predecessor tree (lib/pregate/remote-receipt.ts): the gated tree, the tree it was accepted for, the policy, and every differing path with its stamp rule. */ export interface PregateReceiptSubstitution { gated_tree: string; accepted_for: string; policy: string; files: Array<{ path: string; rule: string }> } @@ -205,7 +207,7 @@ export const ARTIFACT_SCHEMAS = { type: 'object', required: ['schema_version', 'tree', 'checks'], properties: { schema_version: { const: 1 }, run: str, tree: str, stage1_ms: num, stage2_ms: num, - checks: { type: 'array', items: { type: 'object', required: ['id', 'status'], properties: { id: str, status: { enum: ['pass', 'fail', 'warn', 'incomplete', 'requires-remote', 'not_installed'] }, ms: num, detail: str, stage: { enum: ['preflight', 'tests'] }, inputs: { type: 'object' }, fix: str, code: str, lines: strList, cause: { enum: ['diff', 'environment'] }, environment: { type: 'object', required: ['kind', 'evidence'], properties: { kind: str, evidence: str } } } } }, + checks: { type: 'array', items: { type: 'object', required: ['id', 'status'], properties: { id: str, status: { enum: ['pass', 'fail', 'warn', 'incomplete', 'requires-remote', 'not_installed'] }, ms: num, detail: str, stage: { enum: ['preflight', 'tests'] }, inputs: { type: 'object' }, fix: str, code: str, lines: strList, cause: { enum: ['diff', 'environment'] }, environment: { type: 'object', required: ['kind', 'evidence'], properties: { kind: str, evidence: str } }, proven: strList } } }, head: str, base: str, generated_at: str, policy: str, policy_digest: str, policy_user_path: str, registry: str, verdict: str, scratch: str, requires_remote: { type: 'array', items: { type: 'object', required: ['lane', 'platform', 'tests', 'receipt'], properties: { lane: str, platform: str, workflow: str, job: str, tests: strList, receipt: nullableStr, cleared_by: str, command: str, substitution: { type: 'object', required: ['gated_tree', 'accepted_for', 'policy', 'files'], properties: { gated_tree: str, accepted_for: str, policy: str, files: { type: 'array', items: { type: 'object', required: ['path', 'rule'], properties: { path: str, rule: str } } } } } } } }, diff --git a/lib/pregate/checks/regen.ts b/lib/pregate/checks/regen.ts index dd63dd003..4aef9c75e 100644 --- a/lib/pregate/checks/regen.ts +++ b/lib/pregate/checks/regen.ts @@ -23,7 +23,7 @@ function run(ctx: PregateContext): CheckResult { fix: 'rerun with --allow-repo-commands (what /ship passes)', lines: commands.map(c => `COMMAND: ${c.id}=${JSON.stringify(c.command)} outputs=${c.outputs.join(',')}`), }; } - const { results } = runRegen({ repoRoot: ctx.repoRoot, registry: ctx.registry, mode: 'check', only: selected.map(e => e.id), env: ctx.env, timeoutMs: ctx.timeoutMs }); + const { results } = runRegen({ repoRoot: ctx.repoRoot, registry: ctx.registry, mode: 'check', only: selected.map(e => e.id), prove: ctx.touched.map(t => t.path), env: ctx.env, timeoutMs: ctx.timeoutMs }); const lines = results.map(renderEntry); const stale = results.filter(r => r.status === 'stale'); const failed = results.filter(r => r.status === 'failed'); @@ -32,7 +32,8 @@ function run(ctx: PregateContext): CheckResult { if (depsMissing.length) return { id: 'regen', stage: 'preflight', status: 'incomplete', inputs, lines, code: 'REGEN_DEPS_MISSING', detail: `dependencies missing; ${depsMissing[0]!.fix} (${depsMissing.length} entr${depsMissing.length === 1 ? 'y' : 'ies'} unverified: ${depsMissing.map(d => d.id).join(', ')})`, fix: depsMissing[0]!.fix }; if (failed.length) return { id: 'regen', stage: 'preflight', status: 'incomplete', inputs, lines, code: 'REGEN_COMMAND_FAILED', detail: `${failed.length} regen command(s) failed: ${failed.map(f => f.id).join(', ')}`, fix: failed.map(f => f.fix).join('; ') }; if (stale.length) return { id: 'regen', stage: 'preflight', status: 'fail', inputs, lines, code: 'REGEN_STALE', detail: `${stale.length} stale: ${stale.flatMap(s => s.changed.concat(s.outside)).join(' ')}`, fix: stale.map(s => s.fix).join('; ') }; - return { id: 'regen', stage: 'preflight', status: 'pass', inputs, lines, detail: `${results.length} entr${results.length === 1 ? 'y' : 'ies'} fresh${results.some(r => r.status === 'listed') ? ' (hand-regenerated outputs listed)' : ''}` }; + const proven = results.flatMap(r => r.proven ?? []).sort(); + return { id: 'regen', stage: 'preflight', status: 'pass', inputs, lines, ...(proven.length ? { proven } : {}), detail: `${results.length} entr${results.length === 1 ? 'y' : 'ies'} fresh${results.some(r => r.status === 'listed') ? ' (hand-regenerated outputs listed)' : ''}` }; } export const regenCheck: CheckDef = { id: 'regen', stage: 'preflight', tier: 1, downgradable: true, needsRepoCommands: true, run }; diff --git a/lib/regen.ts b/lib/regen.ts index 2a8538647..6654c854e 100644 --- a/lib/regen.ts +++ b/lib/regen.ts @@ -164,6 +164,8 @@ 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). */ + proven?: string[]; } /** Working tree vs the seeded index in a scratch worktree: modified tracked files plus new untracked ones. */ @@ -214,6 +216,8 @@ 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`. */ + prove?: readonly string[]; env?: NodeJS.ProcessEnv; timeoutMs?: number; } @@ -240,16 +244,19 @@ 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 changed = after.filter(p => matchesAny(e.outputs, p)); + const unwritten = probe.filter(p => !fs.existsSync(path.join(cwd, p))); + const changed = after.filter(p => matchesAny(e.outputs, p) && !unwritten.includes(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) }); + 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']); } else results.push({ id: e.id, command: e.command, status: changed.length ? 'written' : 'fresh', changed, outside, exit: 0, ms, fix: fixFor(e) }); diff --git a/test/ship-receipt-carry-rewrite.test.ts b/test/ship-receipt-carry-rewrite.test.ts index 41c038d1c..83009c4a0 100644 --- a/test/ship-receipt-carry-rewrite.test.ts +++ b/test/ship-receipt-carry-rewrite.test.ts @@ -120,9 +120,37 @@ describe('carry across a base merge that changes a registered output', () => { expect(runBin(repo, 'gstack-pregate', ['--base', 'main', '--allow-repo-commands']).status).toBe(EXIT.ok); const pregateJson = path.join(repo.work, '.gstack/tmp/pregate.json'); - expect(JSON.parse(fs.readFileSync(pregateJson, 'utf8')).checks.find((c: { id: string }) => c.id === 'regen')).toMatchObject({ status: 'pass' }); + 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 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 } }); }, 120_000); + + // Value: protects=a hand-written file under an output glob is reviewed code (red team, #3564 pass 1); fails_when=a + // regen pass that never wrote the path sets it aside as generated; seam=none; no_claim=- + test('a file that matches an output glob but that the generator never writes stays product', () => { + const repo = makeFixtureRepo(); repos.push(repo); + const reviewed = commit(repo, { 'lib/util.js': "exports.add = (a, b) => a + b + 0;\n" }); + const body = path.join(repo.root, 'body.md'); + expect(writeReceipt(repo, body).status).toBe(EXIT.ok); + commit(repo, { 'generated/extra.md': '# hand-written after review\n' }); + expect(runBin(repo, 'gstack-pregate', ['--base', 'main', '--allow-repo-commands']).status).toBe(EXIT.ok); + const pregateJson = path.join(repo.work, '.gstack/tmp/pregate.json'); + const regen = JSON.parse(fs.readFileSync(pregateJson, 'utf8')).checks.find((c: { id: string }) => c.id === 'regen'); + expect(regen.status).toBe('pass'); + expect(regen.proven ?? []).not.toContain('generated/extra.md'); + const out = carry(repo, body, ['--pregate', pregateJson]); + expect(out.status).toBe(EXIT.refused); + expect(out.out).toContain('RECEIPT_CARRY_PRODUCT_CHANGED'); + expect(out.err).toContain('generated/extra.md'); + expect(receiptIn(body).head).toBe(reviewed); + }, 120_000); });