mirror of
https://github.com/garrytan/gstack.git
synced 2026-10-31 22:14:25 +01:00
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/<base>'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>
This commit is contained in:
1 parent
e97b857350
commit
f47aba6fe7
8 files changed
+71
-20
No files matched your search
+2
-1
@@ -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/<base>'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/<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.
|
||||
- **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.
|
||||
|
||||
|
||||
+13
-9
@@ -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 <n> [--repo <r>] | --body <file> [--base <ref>] [--head <sha>] [--pregate <pregate.json>] [--dry-run] [--out <dir>]
|
||||
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/<base>'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/<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.
|
||||
*/
|
||||
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');
|
||||
|
||||
+5
-1
@@ -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.
|
||||
|
||||
+7
-3
@@ -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/<base>'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/<base> 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
|
||||
|
||||
@@ -69,6 +69,8 @@ export interface PregateCheck {
|
||||
stage?: 'preflight' | 'tests'; inputs?: Record<string, string>; 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 } } } } } } } },
|
||||
|
||||
@@ -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 };
|
||||
+9
-2
@@ -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) });
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
Reference in new issue
Block a user