diff --git a/bin/gstack-docs-candidate b/bin/gstack-docs-candidate index 3a012abad..05917afe4 100755 --- a/bin/gstack-docs-candidate +++ b/bin/gstack-docs-candidate @@ -5,6 +5,7 @@ * Usage (run inside the repository): * gstack-docs-candidate snapshot --out --audit-id --mode edit|read-only * --base [--select ]... [--docs [,...]]... [--exception ] + * [--run ] [--pre [--carry ]] * gstack-docs-candidate compare * gstack-docs-candidate attempt --state * gstack-docs-candidate [snapshot|compare] --help @@ -24,6 +25,15 @@ * Step 16 can tell an accepted risk from a skipped audit. A --base that is HEAD * or already contains it while the branch has commits its real base lacks is * refused (BASE_CONTAINS_HEAD, lib/default-base.ts): it would select nothing. + * --run names the /ship invocation (its record's path) and is saved as run_id. + * --pre marks a post snapshot: it must be this audit's + * (same audit id, HEAD and run) and the record saves `edited`, each path whose + * content the audit changed with its content before (the Prepare hash, else + * HEAD's blob, null when absent) and after. --carry (with --pre and --run) saves that audit and the ones it carried as + * `earlier`, so receipt write resets every file this run's audits edited + * (GST-1616); a snapshot of another run, one without `edited`, or one whose + * HEAD is not an ancestor of HEAD is refused. * * compare re-reads the same state and prints one JSON object: whether HEAD or * the index changed, which recorded paths' content changed or disappeared, and @@ -118,7 +128,7 @@ function one(args: string[], name: string): string { return values[0]!; } -const USAGE = `usage: gstack-docs-candidate snapshot --out --audit-id --mode edit|read-only --base [--select ]... [--docs [,...]]... [--exception ] +const USAGE = `usage: gstack-docs-candidate snapshot --out --audit-id --mode edit|read-only --base [--select ]... [--docs [,...]]... [--exception ] [--run ] [--pre [--carry ]] gstack-docs-candidate compare gstack-docs-candidate attempt --state `; @@ -132,6 +142,47 @@ function acceptedException(file: string): NonNullable return { ...parsed.docs.exception, scope: parsed.docs.scope }; } +/** One value for an optional flag given at most once. */ +function optional(args: string[], name: string): string | undefined { + const values = flag(args, name); + if (values.length > 1) throw new Error(`${name} is allowed once`); + if (values[0] !== undefined && (!values[0].trim() || /[\x00-\x1f\x7f]/.test(values[0]))) throw new Error(`${name} must be a non-empty one-line value`); + return values[0]; +} + +function readRecord(file: string, name: string): Record { + let record: unknown; + try { record = JSON.parse(fs.readFileSync(file, 'utf8')); } catch (error) { throw new Error(`${name} ${file} is not readable JSON: ${(error as Error).message.split('\n')[0]}`); } + if (!record || typeof record !== 'object' || Array.isArray(record) || typeof (record as any).head !== 'string' || !(record as any).hashes || typeof (record as any).hashes !== 'object') throw new Error(`${name} ${file} is not a gstack-docs-candidate snapshot`); + return record as Record; +} + +type Edited = Record; + +/** The post snapshot's own audit: each path whose content differs from this audit's Prepare snapshot, before (its Prepare hash, else HEAD's blob) and after. */ +function auditEdits(pre: Record, file: string, post: { audit_id: string; run_id?: string; head: string; hashes: Record }): { edited: Edited } { + if (pre.audit_id !== post.audit_id || pre.head !== post.head || pre.run_id !== post.run_id) throw new Error(`--pre ${file} is not this audit's Prepare snapshot (its audit id, HEAD or run differs)`); + const paths = [...new Set([...Object.keys(pre.hashes), ...Object.keys(post.hashes)])].sort(); + const now = hashes(paths.filter(p => !(p in post.hashes))); + const blob = (p: string) => spawnSync('git', ['rev-parse', '--verify', '--quiet', `${pre.head}:${p}`], { encoding: 'utf8', timeout: 60_000 }).stdout?.trim() || null; + const edited: Edited = {}; + for (const p of paths) { + const before = p in pre.hashes ? pre.hashes[p] : blob(p); + const after = p in post.hashes ? post.hashes[p]! : now[p]!; + if (before !== after) edited[p] = { before, after }; + } + return { edited }; +} + +/** An earlier audit of this run and the audits it carried, oldest first; refuses another run's snapshot, one without `edited` and one whose HEAD is not an ancestor of HEAD. */ +function carried(prior: Record, file: string, post: { audit_id: string; run_id?: string; head: string }): Array<{ run_id: string; audit_id: string; head: string; edited: Edited }> { + if (prior.run_id !== post.run_id) throw new Error(`--carry ${file} is audit ${prior.audit_id} of another /ship run (${prior.run_id ?? 'no --run'}), not ${post.run_id}: a post snapshot carries only this run's earlier audits`); + if (!prior.edited || typeof prior.edited !== 'object' || Array.isArray(prior.edited)) throw new Error(`--carry ${file} names no edited files: take each post snapshot with --pre `); + if (prior.audit_id === post.audit_id) throw new Error(`--carry ${file} is this audit (${post.audit_id}), not an earlier one`); + if (spawnSync('git', ['merge-base', '--is-ancestor', String(prior.head), post.head], { timeout: 60_000 }).status !== 0) throw new Error(`--carry ${file}: its HEAD ${String(prior.head).slice(0, 12)} is not an ancestor of HEAD`); + return [...(Array.isArray(prior.earlier) ? prior.earlier : []), { run_id: prior.run_id, audit_id: prior.audit_id, head: prior.head, edited: prior.edited }]; +} + /** Each field compare needs that the record lacks or types wrongly; empty for a valid record. */ function recordProblems(record: any): string[] { if (!record || typeof record !== 'object' || Array.isArray(record)) return ['not a JSON object']; @@ -151,7 +202,7 @@ function main(argv: string[]): void { return; } if (command === 'snapshot') { - const known = new Set(['--out', '--audit-id', '--mode', '--base', '--select', '--docs', '--exception']); + const known = new Set(['--out', '--audit-id', '--mode', '--base', '--select', '--docs', '--exception', '--run', '--pre', '--carry']); for (let i = 0; i < args.length; i += 2) if (!known.has(args[i]!)) throw new Error(`unknown argument: ${args[i]}`); const out = path.resolve(one(args, '--out')); const top = fs.realpathSync(git(['rev-parse', '--show-toplevel']).trim()); @@ -163,6 +214,7 @@ function main(argv: string[]): void { if (self) throw new Error(resultLine('snapshot', 'BASE_CONTAINS_HEAD', self)); const exceptionFile = flag(args, '--exception')[0]; const exception = exceptionFile === undefined ? undefined : acceptedException(exceptionFile); + const [preFile, carryFile] = ['--pre', '--carry'].map(name => { const v = optional(args, name); return v === undefined ? undefined : path.resolve(v); }); process.chdir(top); const now = state(base); const listed = nul(git(['ls-files', '-z', '--cached', '--others', '--exclude-standard'])); @@ -170,11 +222,18 @@ function main(argv: string[]): void { : [...new Set([...now.committed, ...now.staged, ...now.unstaged, ...now.untracked])].sort(); const docs = expand(listed, docsPaths(listed, flag(args, '--docs'))); const tracked = [...new Set([...selected, ...docs, ...now.staged, ...now.unstaged, ...now.untracked])].sort(); - const record = { schema_version: 1, audit_id: one(args, '--audit-id'), mode, base, ...now, selected, docs, hashes: hashes(tracked), ...(exception ? { exception } : {}) }; - fs.writeFileSync(out, JSON.stringify(record, null, 2) + '\n', { mode: 0o600 }); + const auditId = one(args, '--audit-id'); + const run = optional(args, '--run'); + const record = { schema_version: 1, audit_id: auditId, ...(run === undefined ? {} : { run_id: run }), mode, base, ...now, selected, docs, hashes: hashes(tracked), ...(exception ? { exception } : {}) }; + if (carryFile !== undefined && (preFile === undefined || run === undefined)) throw new Error('--carry needs --pre and --run: only a post snapshot of a named /ship run carries an earlier audit'); + const audit = preFile === undefined ? {} : auditEdits(readRecord(preFile, '--pre'), preFile, record); + const earlier = carryFile === undefined ? {} : { earlier: carried(readRecord(carryFile, '--carry'), carryFile, record) }; + fs.writeFileSync(out, JSON.stringify({ ...record, ...audit, ...earlier }, null, 2) + '\n', { mode: 0o600 }); console.log(`CANDIDATE: ${out}`); console.log(`base=${base} head=${now.head} branch=${now.branch || '(detached)'} mode=${mode}`); if (exception) console.log(`EXCEPTION: accepted by ${exception.accepted_by} ${exception.date} via ${exception.via}`); + if ('edited' in audit) console.log(`edited: ${Object.keys(audit.edited).map(p => JSON.stringify(p)).join(' ') || '(none)'}`); + if ('earlier' in earlier) console.log(`earlier: ${earlier.earlier.map(e => e.audit_id).join(' ')}`); console.log(`committed=${now.committed.length} staged=${now.staged.length} unstaged=${now.unstaged.length} untracked=${now.untracked.length} selected=${selected.length} docs=${docs.length} hashed=${tracked.length}`); for (const [label, list] of [['committed', now.committed], ['staged', now.staged], ['unstaged', now.unstaged], ['untracked', now.untracked], ['docs', docs]] as const) { if (list.length) console.log(`${label}: ${list.map(p => JSON.stringify(p)).join(' ')}`); diff --git a/bin/gstack-ship-receipt b/bin/gstack-ship-receipt index b9b956945..7eb64624c 100755 --- a/bin/gstack-ship-receipt +++ b/bin/gstack-ship-receipt @@ -118,6 +118,9 @@ write flags: --pr --head --base --version --gated-tree --predecessor --gate (the same audit's first snapshot) narrows the audit's files to those whose content it changed, so uncommitted docs the review read stay. + When a run audits twice, the last audit's post snapshot carries the earlier audit (gstack-docs-candidate snapshot --run --pre + --carry): files the earlier audit edited reset too, to their content before it, when HEAD holds what it left; a carried audit of + another run refuses (GST-1616). --no-stop (a JSON array of {class, reason, evidence}: each routine stop the run recorded and went on past, class one of scoped-fix-check, test-timeout-fix, pregate-flake-rerun, stamp-only-carry, autoplan-regate); recorded as no_stop. --declare-risk (the same list gstack-review-plan --declare-risk took): joins review_depth's risk and only raises diff --git a/changelog.d/capy-gst-1616-two-docs-audits.md b/changelog.d/capy-gst-1616-two-docs-audits.md new file mode 100644 index 000000000..fe9505d13 --- /dev/null +++ b/changelog.d/capy-gst-1616-two-docs-audits.md @@ -0,0 +1 @@ +- **A /ship run that audits its docs twice keeps the review it already has.** When Step 14.5 ran again after a base merge and restamp, `gstack-ship-receipt write` reset only the last audit's files, so a file the first audit edited and committed stayed in the `review_exclude_paths` selection and write refused the reviewed digest (gbrain #8678); the only way out was HEAD's digest, which skips review of the delta. Each docs snapshot now names its /ship run (`--run`), each post snapshot records the files its audit edited (`--pre `), and a re-audit's post snapshot carries the earlier audit (`--carry`). Write resets every file this run's audits edited to its content before them and binds the reviewed digest. A file no audit edited, an edit after an audit, and a carried audit from another run still refuse. (GST-1616) diff --git a/docs/ship-policy.md b/docs/ship-policy.md index 6b0da9c8f..211f2ec20 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -1516,7 +1516,7 @@ recorded by accident, but it does not read the review ledger: a write given the `expected=` digest passes without a review, and only the review run shows that the reviewers read that selection. Write, carry and the queue make one exception (GST-1408, GST-1415, GST-1434, GST-1446): when the digests differ but the recorded selection and HEAD's give the PR's files the same selection, meaning the same applied patterns, the same excluded files and the same kept files with their reasons, write records HEAD's digest, carry carries the receipt and records HEAD's digest, and the queue (`ready`, train build, land) binds it. So a pattern the base gains, adds, removes or reverts that changes nothing about the PR's files does not void the review. No recorded selection (`none`) counts as the empty one. A receipt's record counts only when its digest is built from its list hash. Write gets only a digest, so it rebuilds the recorded selection: the digest binds when the `review_exclude_paths` the base held at some commit that changed the policy file, combined with HEAD's list hash, gives that digest. It prints `REVIEW_EXCLUDE_POLICY_MOVED:`. A digest no such policy gives, an omitted `--review-exclude`, a selection that differs over the PR's files and an unreadable base policy still refuse. -Write makes one exception for Step 14.5's docs audit, which runs after the review (GST-1429): with `--docs-post -post.json` it accepts the reviewed digest when that digest is HEAD's selection with every file under a pattern that changed since the snapshot's HEAD reset to it, each such file is uncommitted in the snapshot with the content HEAD holds, each is excluded or kept only because code names it, and every other file is excluded or kept alike on both sides. With `--docs-pre ` only the files whose content changed between the audit's two snapshots reset, to their Prepare content, so uncommitted docs the reviewers read stay as they are; content compares as git stores it or as it checks out (autocrlf, filters). It prints `REVIEW_EXCLUDE_DOCS_AUDIT:` and records HEAD's digest. Any other change to the selection, an audit file the exclusion keeps for another reason, an edit after the snapshot and a missing or unreadable snapshot still refuse. +Write makes one exception for Step 14.5's docs audit, which runs after the review (GST-1429): with `--docs-post -post.json` it accepts the reviewed digest when that digest is HEAD's selection with every file under a pattern that changed since the snapshot's HEAD reset to it, each such file is uncommitted in the snapshot with the content HEAD holds, each is excluded or kept only because code names it, and every other file is excluded or kept alike on both sides. With `--docs-pre ` only the files whose content changed between the audit's two snapshots reset, to their Prepare content, so uncommitted docs the reviewers read stay as they are; content compares as git stores it or as it checks out (autocrlf, filters). It prints `REVIEW_EXCLUDE_DOCS_AUDIT:` and records HEAD's digest. Any other change to the selection, an audit file the exclusion keeps for another reason, an edit after the snapshot and a missing or unreadable snapshot still refuse. When one `/ship` run audits twice (a re-audit after a base merge and restamp), every snapshot names the run (`--run `), each post snapshot names its Prepare snapshot (`--pre`, recording the files the audit edited), and the re-audit's post snapshot carries the earlier one (`--carry `; GST-1616). Write then also resets each file an earlier audit of the run edited, to its content before that audit, when HEAD (or the re-audit's Prepare snapshot) holds what the audit left; a file no audit edited still refuses, and a carried audit of another run is refused by the snapshot and by write. **Adopting it.** (1) Open a PR on the base that adds the patterns and `min_gstack_version` at least `REVIEW_EXCLUDE_KEY_VERSION`, and ship it like any diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index fc0ea85e7..6398c7f93 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -3560,7 +3560,9 @@ run `gstack-review-dispatch verify --ledger `, which prints `verify` printed. When the only change since the review is Step 14.5's docs audit editing, adding or removing files under a pattern, pass that run's `--docs-post -post.json` and its Prepare snapshot as `--docs-pre` as well instead of rerunning the review; the refusal's `the docs-audit exception does not apply:` clause says why when it cannot -([ship-policy.md](ship-policy.md#review-exclude-paths), GST-1429). +([ship-policy.md](ship-policy.md#review-exclude-paths), GST-1429). When the run audited twice (a re-audit after a base merge), +pass the last audit's pair; its post snapshot must carry the earlier audit (`--carry `, GST-1616), or the +earlier audit's files stay in the selection and write refuses as before. Write, carry and the queue bind a digest that differs (`recorded=none`, written before the base gained `review_exclude_paths`, or a digest from an older policy) when the recorded selection and diff --git a/lib/ship-receipt-depth.ts b/lib/ship-receipt-depth.ts index fa105169a..dc7c32e31 100644 --- a/lib/ship-receipt-depth.ts +++ b/lib/ship-receipt-depth.ts @@ -34,7 +34,7 @@ import { reviewPolicy, type ReviewPolicy } from './ship-policy-review'; import { TARGETED_CLASSES, classList, targetedClasses, type TargetedClass } from './review-targeted'; import { runScriptSync } from './script-invocation'; import { bindReceipt, type ReceiptBindingCode } from './ship-receipt'; -import { docsExceptionLine, parseReceiptDocs } from './ship-receipt-docs'; +import { carriedAudits, docsExceptionLine, parseReceiptDocs, type CarriedAudit } from './ship-receipt-docs'; export type ReceiptDepth = NonNullable; export type ReviewLoopException = NonNullable; @@ -327,8 +327,14 @@ const treatment = (x: ActiveExclusion, p: string) => (x.excluded.has(p) ? 'exclu * with the audit's files reset to their content before the audit, each of them is excluded or kept only because code names it, * and every other file is selected alike on both sides. The audit's files are those whose content differs between `docsPre` * (the Prepare snapshot) and the post snapshot, reset to the Prepare content, so uncommitted docs the reviewers read stay; without - * it every such file is an audit file, reset to the snapshot's HEAD. Content compares as git stores it or as it checks out - * (autocrlf, filters). Anything else, an unreadable snapshot included, refuses with `why`. + * it every such file is an audit file, reset to the snapshot's HEAD. A post snapshot that carries this /ship run's earlier audits + * (`earlier`, all of its run_id; GST-1616) also resets each file one of them edited that changed since its HEAD or that this audit edited again, to its content + * before the first of them, when HEAD (or the latest audit's Prepare snapshot, for a file it edited again) holds what the last of + * them left. When the whole carried list does not rebuild the reviewed selection, write retries without the oldest carried audits, + * one at a time down to none (a review that ran between two audits already read the earlier one's edits); only that digest + * mismatch retries, so an integrity refusal (an edit after an audit, a file changed between audits, a foreign HEAD) ends it, and + * the refusal names the whole list's reason. Content compares as git stores it or as it checks out (autocrlf, filters). Anything else, an unreadable + * snapshot included, refuses with `why`. */ export function writeExcludeBinds(root: string, base: string, head: string, review: { exclude: Exclusion; tagged: ReadonlySet | null }, flagged: string | undefined, docsPost: string | undefined, docsPre?: string): { ok: true; line?: string } | { ok: false; why?: string } { const x = review.exclude; @@ -339,23 +345,26 @@ export function writeExcludeBinds(root: string, base: string, head: string, revi if (docsPost === undefined || x.state !== 'active') return { ok: false }; const no = (why: string) => ({ ok: false as const, why: `--docs-post ${docsPost}: ${why}` }); const list = (v: unknown): v is string[] => Array.isArray(v) && v.every(p => typeof p === 'string'); - const snapshot = (file: string): { head: string; audit_id?: unknown; staged: string[]; unstaged: string[]; untracked: string[]; hashes: Record } | string => { - let s: { head?: unknown; audit_id?: unknown; staged?: unknown; unstaged?: unknown; untracked?: unknown; hashes?: unknown }; + const snapshot = (file: string): { head: string; audit_id?: unknown; run_id?: unknown; earlier?: unknown; staged: string[]; unstaged: string[]; untracked: string[]; hashes: Record } | string => { + let s: { head?: unknown; audit_id?: unknown; run_id?: unknown; earlier?: unknown; staged?: unknown; unstaged?: unknown; untracked?: unknown; hashes?: unknown }; try { s = JSON.parse(fs.readFileSync(file, 'utf8')); } catch (e) { return `cannot be read (${(e as Error).message.split('\n')[0]})`; } const h = s?.hashes; if (typeof s?.head !== 'string' || !/^[0-9a-f]{40,64}$/.test(s.head) || !list(s.staged) || !list(s.unstaged) || !list(s.untracked) || !h || typeof h !== 'object' || Array.isArray(h)) return 'is not a gstack-docs-candidate snapshot (head, staged, unstaged, untracked, hashes)'; - return { head: s.head, audit_id: s.audit_id, staged: s.staged, unstaged: s.unstaged, untracked: s.untracked, hashes: h as Record }; + return { head: s.head, audit_id: s.audit_id, run_id: s.run_id, earlier: s.earlier, staged: s.staged, unstaged: s.unstaged, untracked: s.untracked, hashes: h as Record }; }; const post = snapshot(docsPost); if (typeof post === 'string') return no(post); const pre = docsPre === undefined ? null : snapshot(docsPre); if (typeof pre === 'string') return no(`--docs-pre ${docsPre} ${pre}`); - if (pre && (pre.head !== post.head || pre.audit_id !== post.audit_id)) return no(`--docs-pre ${docsPre} is not this audit's Prepare snapshot (its HEAD or audit_id differs)`); + if (pre && (pre.head !== post.head || pre.audit_id !== post.audit_id || pre.run_id !== post.run_id)) return no(`--docs-pre ${docsPre} is not this audit's Prepare snapshot (its HEAD, audit_id or run_id differs)`); + const earlier = carriedAudits(post); + if (typeof earlier === 'string') return no(earlier); + if (earlier.length && !pre) return no(`it carries audit ${earlier.map(e => e.audit_id).join(',')}, so write needs --docs-pre `); const at = post.head; if (!git(root, ['merge-base', '--is-ancestor', at, head]).ok) return no(`its HEAD ${short(at)} is not ${short(head)} or an ancestor of it`); - const diff = spawnSync('git', ['diff', '--name-only', '-z', '--no-renames', at, head, '--', ...x.record.patterns.map(p => `:(top,glob)${p}`)], { cwd: root, encoding: 'utf8', timeout: 60_000 }); - if (diff.status !== 0) return no(`git could not list the files under review_exclude_paths that changed since ${short(at)}`); - const changed = diff.stdout.split('\0').filter(Boolean).sort(); + const since = (rev: string) => { const d = spawnSync('git', ['diff', '--name-only', '-z', '--no-renames', rev, head, '--', ...x.record.patterns.map(p => `:(top,glob)${p}`)], { cwd: root, encoding: 'utf8', timeout: 60_000 }); return d.status === 0 ? d.stdout.split('\0').filter(Boolean).sort() : null; }; + const changed = since(at); + if (!changed) return no(`git could not list the files under review_exclude_paths that changed since ${short(at)}`); if (!changed.length) return no(`no file under review_exclude_paths changed since its HEAD ${short(at)}, so the docs audit did not change the selection`); const blob = (rev: string, p: string) => { const b = git(root, ['rev-parse', '--verify', '--quiet', `${rev}:${p}`]); return b.ok ? b.out : null; }; const holds = (rev: string, p: string, want: unknown) => { @@ -371,41 +380,68 @@ export function writeExcludeBinds(root: string, base: string, head: string, revi if (!holds(head, p, post.hashes[p] ?? null)) return no(`${p} at HEAD is not the content the snapshot recorded (an edit after the audit)`); } const auditFiles = pre ? changed.filter(p => !(p in pre.hashes) || pre.hashes[p] !== post.hashes[p]) : changed; - if (!auditFiles.length) return no(`--docs-pre ${docsPre}: the audit changed none of ${changed.join(',')}, so it did not change the selection`); - const gitDir = git(root, ['rev-parse', '--absolute-git-dir']).out; - const index = path.join(gitDir, `gstack-receipt-${process.pid}-${randomBytes(4).toString('hex')}.index`); - const env = { ...process.env, GIT_INDEX_FILE: index, GIT_AUTHOR_NAME: 'gstack-ship-receipt', GIT_AUTHOR_EMAIL: 'ship-receipt@gstack.local', GIT_COMMITTER_NAME: 'gstack-ship-receipt', GIT_COMMITTER_EMAIL: 'ship-receipt@gstack.local', GIT_AUTHOR_DATE: '2000-01-01T00:00:00Z', GIT_COMMITTER_DATE: '2000-01-01T00:00:00Z' }; - const run = (args: string[], input?: string) => { const r = spawnSync('git', args, { cwd: root, encoding: 'utf8', env, input, timeout: 60_000 }); return { ok: r.status === 0, out: (r.stdout ?? '').trim() }; }; - const treeEntry = (rev: string, p: string) => run(['ls-tree', '-z', '--full-tree', rev, '--', p]).out.split('\0').find(l => l.endsWith(`\t${p}`)); - let reset: string; - try { - const entries: string[] = []; - for (const p of auditFiles) { - const want = pre ? pre.hashes[p] : undefined; - const atEntry = treeEntry(at, p); - if (want === undefined || (atEntry && holds(at, p, want))) entries.push(atEntry ? `${atEntry}\0` : `0 ${'0'.repeat(at.length)}\t${p}\0`); - else if (want === null) entries.push(`0 ${'0'.repeat(at.length)}\t${p}\0`); - else if (typeof want === 'string' && run(['cat-file', '-e', `${want}^{blob}`]).ok) entries.push(`${(treeEntry(head, p) ?? atEntry ?? '100644').split(' ')[0]} blob ${want}\t${p}\0`); - else return no(`${p}'s content before the audit (${short(String(want))} in --docs-pre ${docsPre}) is not in git's object store, so the selection the reviewers read cannot be rebuilt`); + const bindAudits = (carried: CarriedAudit[]): { ok: true; line: string } | { ok: false; why: string; retry?: true } => { + const resets = new Map(auditFiles.map(p => [p, { rev: at, want: pre ? pre.hashes[p] : undefined, from: `--docs-pre ${docsPre}` }])); + const left = new Map(); + for (const e of carried) { + const moved = git(root, ['merge-base', '--is-ancestor', e.head, at]).ok ? since(e.head) : null; + if (!moved) return no(`carried audit ${e.audit_id}'s HEAD ${short(e.head)} is not an ancestor of ${short(at)}, or git could not diff it`); + for (const [p, { before, after }] of Object.entries(e.edited)) { + if (!moved.includes(p) && !auditFiles.includes(p)) continue; + const prior = left.get(p); + if (prior && prior.after !== before) return no(`${p} changed between audit ${prior.audit} and audit ${e.audit_id}, outside both`); + if (!prior) resets.set(p, { rev: e.head, want: before, from: `audit ${e.audit_id}'s post snapshot` }); + left.set(p, { after, audit: e.audit_id }); + } } - const tree = run(['read-tree', head]).ok && run(['update-index', '-z', '--index-info'], entries.join('')).ok ? run(['write-tree']) : { ok: false, out: '' }; - const c = tree.ok ? run(['commit-tree', '--no-gpg-sign', tree.out, '-p', head, '-m', 'gstack-ship-receipt: HEAD before the docs audit']) : tree; - if (!c.ok) return no(`git could not build HEAD with ${auditFiles.join(',')} reset to before the audit`); - reset = c.out; - } finally { fs.rmSync(index, { force: true }); } - const { ref, sha } = writeBase(root, base); - const before = exclusionAt(root, ref, sha, reset, collectCommit(root, sha ?? ref, reset), review.tagged, 'gstack-ship-receipt'); - if (before.state !== 'active') return no(`the selection with ${auditFiles.join(',')} reset to before the audit cannot be read`); - if (before.record.digest !== given) return no(`the reviewed digest ${short(given)} is not the selection HEAD has with the files under review_exclude_paths that changed since the snapshot reset to ${pre ? 'their --docs-pre content' : short(at)} (${short(before.record.digest)}), so more than the docs audit changed what the reviewers read`); - for (const p of auditFiles) for (const [side, s] of [['the review', before], ['HEAD', x]] as const) { - const t = treatment(s, p); - if (!['absent', 'excluded', 'named-by-code'].includes(t)) return no(`${p} is ${t} at ${side}: the docs audit binds only docs the exclusion hides or keeps because code names them`); + for (const [p, { after, audit }] of left) { + const kept = auditFiles.includes(p) ? (p in pre!.hashes ? pre!.hashes[p] === after : holds(at, p, after)) : holds(head, p, after); + if (!kept) return no(`${p} at ${auditFiles.includes(p) ? `this audit's Prepare snapshot` : 'HEAD'} is not the content audit ${audit} recorded (an edit after that audit)`); + } + const files = [...resets.keys()].sort(); + if (!files.length) return no(`--docs-pre ${docsPre}: the audit changed none of ${changed.join(',')}, so it did not change the selection`); + const gitDir = git(root, ['rev-parse', '--absolute-git-dir']).out; + const index = path.join(gitDir, `gstack-receipt-${process.pid}-${randomBytes(4).toString('hex')}.index`); + const env = { ...process.env, GIT_INDEX_FILE: index, GIT_AUTHOR_NAME: 'gstack-ship-receipt', GIT_AUTHOR_EMAIL: 'ship-receipt@gstack.local', GIT_COMMITTER_NAME: 'gstack-ship-receipt', GIT_COMMITTER_EMAIL: 'ship-receipt@gstack.local', GIT_AUTHOR_DATE: '2000-01-01T00:00:00Z', GIT_COMMITTER_DATE: '2000-01-01T00:00:00Z' }; + const run = (args: string[], input?: string) => { const r = spawnSync('git', args, { cwd: root, encoding: 'utf8', env, input, timeout: 60_000 }); return { ok: r.status === 0, out: (r.stdout ?? '').trim() }; }; + const treeEntry = (rev: string, p: string) => run(['ls-tree', '-z', '--full-tree', rev, '--', p]).out.split('\0').find(l => l.endsWith(`\t${p}`)); + let reset: string; + try { + const entries: string[] = []; + for (const [p, { rev, want, from }] of resets) { + const atEntry = treeEntry(rev, p); + if (want === undefined || (atEntry && holds(rev, p, want))) entries.push(atEntry ? `${atEntry}\0` : `0 ${'0'.repeat(rev.length)}\t${p}\0`); + else if (want === null) entries.push(`0 ${'0'.repeat(rev.length)}\t${p}\0`); + else if (typeof want === 'string' && run(['cat-file', '-e', `${want}^{blob}`]).ok) entries.push(`${(treeEntry(head, p) ?? atEntry ?? '100644').split(' ')[0]} blob ${want}\t${p}\0`); + else return no(`${p}'s content before the audit (${short(String(want))} in ${from}) is not in git's object store, so the selection the reviewers read cannot be rebuilt`); + } + const tree = run(['read-tree', head]).ok && run(['update-index', '-z', '--index-info'], entries.join('')).ok ? run(['write-tree']) : { ok: false, out: '' }; + const c = tree.ok ? run(['commit-tree', '--no-gpg-sign', tree.out, '-p', head, '-m', 'gstack-ship-receipt: HEAD before the docs audit']) : tree; + if (!c.ok) return no(`git could not build HEAD with ${files.join(',')} reset to before the audit`); + reset = c.out; + } finally { fs.rmSync(index, { force: true }); } + const { ref, sha } = writeBase(root, base); + const before = exclusionAt(root, ref, sha, reset, collectCommit(root, sha ?? ref, reset), review.tagged, 'gstack-ship-receipt'); + if (before.state !== 'active') return no(`the selection with ${files.join(',')} reset to before the audit cannot be read`); + if (before.record.digest !== given) return { retry: true, ...no(`the reviewed digest ${short(given)} is not the selection HEAD has with the files under review_exclude_paths that changed since the snapshot reset to ${pre ? 'their --docs-pre content' : short(at)}${left.size ? ` and the files audit ${carried.map(e => e.audit_id).join(',')} edited reset to before that audit` : ''} (${short(before.record.digest)}), so more than the docs audit changed what the reviewers read`) }; + for (const p of files) for (const [side, s] of [['the review', before], ['HEAD', x]] as const) { + const t = treatment(s, p); + if (!['absent', 'excluded', 'named-by-code'].includes(t)) return no(`${p} is ${t} at ${side}: the docs audit binds only docs the exclusion hides or keeps because code names them`); + } + const audit = new Set(files); + for (const p of new Set([...before.excluded, ...x.excluded, ...before.kept.map(k => k.path), ...x.kept.map(k => k.path)])) { + if (!audit.has(p) && treatment(before, p) !== treatment(x, p)) return no(`${p} is ${treatment(before, p)} at the review and ${treatment(x, p)} at HEAD`); + } + return { ok: true, line: `REVIEW_EXCLUDE_DOCS_AUDIT: reviewed=${short(given)} head=${short(x.record.digest)} snapshot=${short(at)} files=${files.join(',')}${carried.length ? ` audits=${[...carried.map(e => e.audit_id), post.audit_id].join(',')}` : ''} (the selection changed only by this /ship run's Step 14.5 docs audits' files under review_exclude_paths; the receipt records HEAD's)` }; + }; + let first: { ok: false; why: string } | undefined; + for (let k = 0; k <= earlier.length; k++) { + const r = bindAudits(earlier.slice(k)); + if (r.ok) return r; + first ??= r; + if (!r.retry) return first; } - const audit = new Set(auditFiles); - for (const p of new Set([...before.excluded, ...x.excluded, ...before.kept.map(k => k.path), ...x.kept.map(k => k.path)])) { - if (!audit.has(p) && treatment(before, p) !== treatment(x, p)) return no(`${p} is ${treatment(before, p)} at the review and ${treatment(x, p)} at HEAD`); - } - return { ok: true, line: `REVIEW_EXCLUDE_DOCS_AUDIT: reviewed=${short(given)} head=${short(x.record.digest)} snapshot=${short(at)} files=${auditFiles.join(',')} (the selection changed only by the Step 14.5 docs audit's files under review_exclude_paths; the receipt records HEAD's)` }; + return first!; } /** The queue's refusal when carryExcludeBinds does not bind the receipt at `head`; null when it does. */ export function excludeMismatch(x: Exclusion, r: ShipReceipt, head: string, base: string): string | null { diff --git a/lib/ship-receipt-docs.ts b/lib/ship-receipt-docs.ts index ceb64c1f6..06f0cef7f 100644 --- a/lib/ship-receipt-docs.ts +++ b/lib/ship-receipt-docs.ts @@ -47,6 +47,27 @@ export function docsExceptionLine(docs: ReceiptDocs): string | null { return `Documentation exception: accepted by ${e.accepted_by} ${e.date} via ${e.via}: ${e.reason}`; } +/** One earlier Step 14.5 audit of the same /ship run that a post snapshot carries (gstack-docs-candidate snapshot --carry, GST-1616). */ +export interface CarriedAudit { run_id: string; audit_id: string; head: string; edited: Record } + +/** The earlier audits a post snapshot carries, oldest first, each of the snapshot's own run; a string says why they cannot be read. */ +export function carriedAudits(post: { run_id?: unknown; audit_id?: unknown; earlier?: unknown }): CarriedAudit[] | string { + if (post.earlier === undefined) return []; + if (!Array.isArray(post.earlier)) return 'its earlier field is not a list of audits'; + if (!text(post.run_id)) return 'it carries earlier audits but names no run_id'; + const sha = (v: unknown) => typeof v === 'string' && /^[0-9a-f]{40,64}$/.test(v); + const seen = new Set([post.audit_id]); + for (const e of post.earlier as Array>) { + const edited = e?.edited; + if (!e || typeof e !== 'object' || !text(e.audit_id) || !sha(e.head) || !edited || typeof edited !== 'object' || Array.isArray(edited) + || !Object.values(edited).every((v: any) => v && typeof v === 'object' && (v.before === null || sha(v.before)) && (v.after === null || sha(v.after)))) return 'it carries an earlier audit that is not { run_id, audit_id, head, edited }'; + if (e.run_id !== post.run_id) return `it carries audit ${e.audit_id} of another /ship run (${String(e.run_id)}), not ${post.run_id}`; + if (seen.has(e.audit_id)) return `it carries audit ${e.audit_id} twice`; + seen.add(e.audit_id); + } + return post.earlier as CarriedAudit[]; +} + // --------------------------------------------------------------------------- // doc caps: `docs.caps` in .gstack/ship-policy.json // --------------------------------------------------------------------------- diff --git a/ship/sections/documentation.md b/ship/sections/documentation.md index 70dea163c..5688b719f 100644 --- a/ship/sections/documentation.md +++ b/ship/sections/documentation.md @@ -30,7 +30,7 @@ Reentry never resets the count or authorizes a launch. Read-only audits must not fetch/merge. 3. Discover docs roots/authored templates per audit-scope, pause other writers, and save the candidate outside the product tree in one call: - `~/.claude/skills/gstack/bin/gstack-docs-candidate snapshot --out --audit-id --mode --base --docs ...` + `~/.claude/skills/gstack/bin/gstack-docs-candidate snapshot --out --audit-id --mode --base --docs ... --run ` (`--select ` narrows release paths). It records HEAD, branch, index, path lists and hashes of release paths, generated outputs and docs, NUL-safely. Read only new files' content; the helper hashes the rest. Fill the prompt placeholders with literal candidate values. @@ -97,8 +97,9 @@ It reads `docs.caps` from `origin/`'s `.gstack/ship-policy.json`, never th branch's copy (a PR cannot raise its own cap), prints one `DOCS_CAP: / ok|over` line per cap (`DOCS_CAP: none declared` without any) and exits 1 on any `over` or absent file: a blocked audit, repaired by trimming that file, never by raising the cap. Then save for Step 16 only post-child hashes (rerun the Prepare `snapshot` -with only `--out -post.json` changed; Step 16 `compare`s it; never type -hashes), status, the child's final JSON line as `-result.json`, and the section copied unchanged as the sole content of a private +with only `--out -post.json` changed and `--pre ` added, +plus `--carry ` on a re-audit so receipt write +resets both audits' files; Step 16 `compare`s it; never type hashes), status, the child's final JSON line as `-result.json`, and the section copied unchanged as the sole content of a private `-documentation.md`. That file is the section's single source; reports and Step 19 insert it by command (`cat`) where they can, never retyped or edited. Save records once; cite files by path, never copying their content. diff --git a/ship/sections/documentation.md.tmpl b/ship/sections/documentation.md.tmpl index eb77f9f03..cf7baabbb 100644 --- a/ship/sections/documentation.md.tmpl +++ b/ship/sections/documentation.md.tmpl @@ -28,7 +28,7 @@ Reentry never resets the count or authorizes a launch. Read-only audits must not fetch/merge. 3. Discover docs roots/authored templates per audit-scope, pause other writers, and save the candidate outside the product tree in one call: - `~/.claude/skills/gstack/bin/gstack-docs-candidate snapshot --out --audit-id --mode --base --docs ...` + `~/.claude/skills/gstack/bin/gstack-docs-candidate snapshot --out --audit-id --mode --base --docs ... --run ` (`--select ` narrows release paths). It records HEAD, branch, index, path lists and hashes of release paths, generated outputs and docs, NUL-safely. Read only new files' content; the helper hashes the rest. Fill the prompt placeholders with literal candidate values. @@ -95,8 +95,9 @@ It reads `docs.caps` from `origin/`'s `.gstack/ship-policy.json`, never th branch's copy (a PR cannot raise its own cap), prints one `DOCS_CAP: / ok|over` line per cap (`DOCS_CAP: none declared` without any) and exits 1 on any `over` or absent file: a blocked audit, repaired by trimming that file, never by raising the cap. Then save for Step 16 only post-child hashes (rerun the Prepare `snapshot` -with only `--out -post.json` changed; Step 16 `compare`s it; never type -hashes), status, the child's final JSON line as `-result.json`, and the section copied unchanged as the sole content of a private +with only `--out -post.json` changed and `--pre ` added, +plus `--carry ` on a re-audit so receipt write +resets both audits' files; Step 16 `compare`s it; never type hashes), status, the child's final JSON line as `-result.json`, and the section copied unchanged as the sole content of a private `-documentation.md`. That file is the section's single source; reports and Step 19 insert it by command (`cat`) where they can, never retyped or edited. Save records once; cite files by path, never copying their content. diff --git a/ship/sections/pr-body.md b/ship/sections/pr-body.md index 80bc6c4d3..fcb16d98b 100644 --- a/ship/sections/pr-body.md +++ b/ship/sections/pr-body.md @@ -241,7 +241,7 @@ A finding the owner rules a disclosed note instead of a fix is closed only by th `gstack-approvals rule`, recorded outside this run, never by this run or a review-log field (docs/ship-policy.md#ruled-notes); the receipt lists each one as `ruled_notes`. When `gstack-review-dispatch verify` printed `REVIEW_EXCLUDE_RECORD: digest=`, add `--review-exclude ` -and Step 14.5's `--docs-post -post.json` and `--docs-pre `; a digest that no longer matches HEAD is +and the last Step 14.5 audit's `--docs-post -post.json` and `--docs-pre `; a digest that no longer matches HEAD is `RECEIPT_INVALID field=review_exclude`: rerun the review, not the write, unless only the docs audit's files under `review_exclude_paths` changed it, which `--docs-post` binds (`REVIEW_EXCLUDE_DOCS_AUDIT:`). diff --git a/ship/sections/pr-body.md.tmpl b/ship/sections/pr-body.md.tmpl index 1eb834284..f6853c840 100644 --- a/ship/sections/pr-body.md.tmpl +++ b/ship/sections/pr-body.md.tmpl @@ -224,7 +224,7 @@ A finding the owner rules a disclosed note instead of a fix is closed only by th `gstack-approvals rule`, recorded outside this run, never by this run or a review-log field (docs/ship-policy.md#ruled-notes); the receipt lists each one as `ruled_notes`. When `gstack-review-dispatch verify` printed `REVIEW_EXCLUDE_RECORD: digest=`, add `--review-exclude ` -and Step 14.5's `--docs-post -post.json` and `--docs-pre `; a digest that no longer matches HEAD is +and the last Step 14.5 audit's `--docs-post -post.json` and `--docs-pre `; a digest that no longer matches HEAD is `RECEIPT_INVALID field=review_exclude`: rerun the review, not the write, unless only the docs audit's files under `review_exclude_paths` changed it, which `--docs-post` binds (`REVIEW_EXCLUDE_DOCS_AUDIT:`). diff --git a/test/gstack-docs-candidate.test.ts b/test/gstack-docs-candidate.test.ts index c9ed7b4bf..cd5629e81 100644 --- a/test/gstack-docs-candidate.test.ts +++ b/test/gstack-docs-candidate.test.ts @@ -171,11 +171,70 @@ describe('gstack-docs-candidate', () => { } }); + // Value: protects=a post snapshot records the files its audit edited (before and after) and carries only this run's earlier + // audits, so receipt write can reset every file one /ship run's audits edited (GST-1616); fails_when=--pre records untouched + // files or misses an added one, or --carry accepts a snapshot without --pre/--run or without edited; why_new=GST-1616; + // seam=gstack-docs-candidate snapshot; no_claim=the receipt binding (test/review-exclude.test.ts) + test('--pre records the audit\'s edits; --carry needs --pre, --run and an earlier post snapshot that recorded them', () => { + const { dir, root } = repo(); + const args = (id: string, out: string, ...extra: string[]) => ['snapshot', '--out', path.join(root, out), '--audit-id', id, '--mode', 'edit', '--base', 'main', '--docs', 'handbook', '--run', 'r1', ...extra]; + expect(run(dir, ...args('a1', 'a1.json')).status).toBe(0); + fs.writeFileSync(path.join(dir, 'handbook', 'new.md'), '# New\n'); + expect(run(dir, ...args('a1', 'a1-plain.json')).status).toBe(0); + const post = run(dir, ...args('a1', 'a1-post.json', '--pre', path.join(root, 'a1.json'))); + expect(post.status, post.stderr).toBe(0); + expect(JSON.parse(fs.readFileSync(path.join(root, 'a1-post.json'), 'utf8'))).toMatchObject({ run_id: 'r1', edited: { 'handbook/new.md': { before: null, after: git(dir, 'hash-object', 'handbook/new.md') } } }); + expect(Object.keys(JSON.parse(fs.readFileSync(path.join(root, 'a1-post.json'), 'utf8')).edited)).toEqual(['handbook/new.md']); + const refused = (extra: string[], why: string) => { + const r = run(dir, ...args('a2', 'a2-post.json', ...extra)); + expect({ status: r.status, written: fs.existsSync(path.join(root, 'a2-post.json')) }).toEqual({ status: 1, written: false }); + expect(r.stderr).toContain(why); + }; + expect(run(dir, ...args('a2', 'a2.json')).status).toBe(0); + refused(['--carry', path.join(root, 'a1-post.json')], '--carry needs --pre and --run'); + refused(['--pre', path.join(root, 'a2.json'), '--carry', path.join(root, 'a1-plain.json')], 'names no edited files'); + // Value: protects=snapshot refuses a carry of this same audit, a carry whose HEAD is not an ancestor, another audit's --pre and a + // repeated or empty --run/--pre/--carry; fails_when=one of those guards in carried(), auditEdits() or optional() is dropped; + // why_new=only the missing --pre/--run and missing edited refusals were covered; seam=none; no_claim=the receipt binding + const orphan = git(dir, 'commit-tree', '-m', 'orphan', git(dir, 'hash-object', '-t', 'tree', '-w', '/dev/null')); + const forged = path.join(root, 'a1-orphan.json'); + fs.writeFileSync(forged, JSON.stringify({ ...JSON.parse(fs.readFileSync(path.join(root, 'a1-post.json'), 'utf8')), head: orphan })); + refused(['--pre', path.join(root, 'a2.json'), '--carry', forged], 'is not an ancestor of HEAD'); + refused(['--pre', path.join(root, 'a1.json')], 'is not this audit\'s Prepare snapshot'); + refused(['--run', 'r1'], '--run is allowed once'); + refused(['--pre', ' '], '--pre must be a non-empty one-line value'); + refused(['--pre', path.join(root, 'a2.json'), '--carry', path.join(root, 'a1-post.json'), '--carry', path.join(root, 'a1-post.json')], '--carry is allowed once'); + const self = run(dir, ...args('a1', 'a1-self.json', '--pre', path.join(root, 'a1.json'), '--carry', path.join(root, 'a1-post.json'))); + expect({ status: self.status, written: fs.existsSync(path.join(root, 'a1-self.json')) }).toEqual({ status: 1, written: false }); + expect(self.stderr).toContain('is this audit (a1), not an earlier one'); + }); + + // Value: protects=a third audit's post snapshot carries the whole chain oldest first (the carried snapshot's earlier[] prepended); + // fails_when=carried() keeps only the directly carried audit or reorders the chain; why_new=no test carried more than one audit; + // seam=none; no_claim=the receipt reset over three audits (test/review-exclude.test.ts) + test('--carry of a post snapshot that itself carried an audit records the whole chain, oldest first', () => { + const { dir, root } = repo(); + const audit = (id: string, file: string, carry?: string) => { + const args = ['snapshot', '--audit-id', id, '--mode', 'edit', '--base', 'main', '--docs', 'handbook', '--run', 'r1']; + expect(run(dir, ...args, '--out', path.join(root, `${id}.json`)).status).toBe(0); + fs.writeFileSync(path.join(dir, 'handbook', file), `# ${id}\n`); + const post = run(dir, ...args, '--out', path.join(root, `${id}-post.json`), '--pre', path.join(root, `${id}.json`), ...(carry ? ['--carry', path.join(root, carry)] : [])); + expect(post.status, post.stderr).toBe(0); + return JSON.parse(fs.readFileSync(path.join(root, `${id}-post.json`), 'utf8')); + }; + audit('a1', 'one.md'); + audit('a2', 'two.md', 'a1-post.json'); + const third = audit('a3', 'three.md', 'a2-post.json'); + expect(third.earlier.map((e: { audit_id: string; run_id: string }) => [e.audit_id, e.run_id])).toEqual([['a1', 'r1'], ['a2', 'r1']]); + expect(Object.keys(third.earlier[0].edited)).toEqual(['handbook/one.md']); + expect(Object.keys(third.earlier[1].edited)).toEqual(['handbook/two.md']); + }); + test('/ship records post-child hashes with the helper, so Step 16 compares instead of retyping', () => { // ci-37198445662 gate-census-2: ship-docsync-completion/-store parents spent 45-78 s of model time // retyping hash maps, child output and records into several artifacts after the child returned. const gate = fs.readFileSync(path.resolve(import.meta.dir, '..', 'ship/sections/documentation.md.tmpl'), 'utf8').replace(/\s+/g, ' '); - expect(gate).toContain('post-child hashes (rerun the Prepare `snapshot` with only `--out -post.json` changed; Step 16 `compare`s it; never type hashes)'); + expect(gate).toContain('post-child hashes (rerun the Prepare `snapshot` with only `--out -post.json` changed and `--pre ` added, plus `--carry ` on a re-audit so receipt write resets both audits\' files; Step 16 `compare`s it; never type hashes)'); expect(gate).toContain('Save records once; cite files by path, never copying their content.'); const { dir, root, record } = repo(); const args = ['--audit-id', 'a1', '--mode', 'edit', '--base', 'main', '--docs', 'handbook']; diff --git a/test/helpers/docsync-fault-eval.ts b/test/helpers/docsync-fault-eval.ts index a2f204328..aee4731b7 100644 --- a/test/helpers/docsync-fault-eval.ts +++ b/test/helpers/docsync-fault-eval.ts @@ -104,7 +104,7 @@ bun ${actorFile} stop ${stateFile} task_id=RETURNED_TASK_ID ${docsActorCanRepair(scenario) ? `bun ${actorFile} repair ${stateFile}\nThis launcher supports one concrete marking/transport repair, only after a dispatched child is confirmed settled: it corrects the launcher's spawned-marker setup and its child transport, the launch faults behind a child that returns without the spawned marker or returns no result within the Collect deadline. That isolated repair is authorized, not approval for document risks.` : 'This launcher has no repair operation or alternate launcher. Missing assets/marking, launch failures and invalid completions cannot be repaired here. Do not invoke repair or invent an inline fallback.'} bun ${actorFile} publish ${stateFile} audit_id=ID report=${report} -prepare is a serialization convenience, not an audit or validation: after inspecting the required inputs and deciding an attempt is allowed, supply a fresh id of 1–80 letters/digits/underscores/hyphens, beginning with a letter or digit. It runs the installed gstack-docs-candidate snapshot (the section's Prepare helper) for the fixture's selection, so the new candidate JSON is the record gstack-docs-candidate compare reads, and copies the exact installed section's child prompt with literal substitutions and the observation interface to a new prompt file. It returns their paths and snapshot_command, the exact snapshot command it ran: where the section reruns the Prepare snapshot, run that command with only --out changed. Read these artifacts; use the returned paths unchanged in dispatch. prepare neither launches a child nor resets/increments the attempt count, repairs content, compares snapshots or accepts an audit. Saved files are never overwritten. Use the single batched inspect transport call (declared above) to read committed, staged, unstaged and new content in one response instead of one command per file. +prepare is a serialization convenience, not an audit or validation: after inspecting the required inputs and deciding an attempt is allowed, supply a fresh id of 1–80 letters/digits/underscores/hyphens, beginning with a letter or digit. It runs the installed gstack-docs-candidate snapshot (the section's Prepare helper) for the fixture's selection, so the new candidate JSON is the record gstack-docs-candidate compare reads, and copies the exact installed section's child prompt with literal substitutions and the observation interface to a new prompt file. It returns their paths and snapshot_command, the exact snapshot command it ran: where the section reruns the Prepare snapshot, run that command with only --out changed and the flags the section adds. Read these artifacts; use the returned paths unchanged in dispatch. prepare neither launches a child nor resets/increments the attempt count, repairs content, compares snapshots or accepts an audit. Saved files are never overwritten. Use the single batched inspect transport call (declared above) to read committed, staged, unstaged and new content in one response instead of one command per file. inspect takes no arguments beyond the state path shown above and is a batched read-only observation: in one JSON response it returns the current base, head, branch and index in the candidate's own representation, the committed (base→HEAD), staged and unstaged diffs, the NUL-safe tracked-and-new path inventory, and per file its bytes plus hash (its git blob id, the representation of the candidate's hashes), with a tracked-but-deleted file reported as exists:false. It returns no verdict, acceptance, snapshot refresh, attempt, count change or publication, never exposes private transport state or precomputed gate answers, and grants no repair, risk exception, new attempt or missing-asset bypass; you still parse the returned data and apply every gate yourself. It is a real observation boundary: an independent editor may change inputs exactly at inspect time, as during any repository read, so an inspect after the child can legitimately reveal a changed input that invalidates a returned audit. ${seed ? 'inspect is the authoritative repository observation in this phase: its result already contains what git status, git rev-parse, git diff, git ls-files or cat of product files would return, so do not run those reads. One inspect after reading attempt 1\'s output serves its freshness check and any repeated Prepare. After a later child returns, one inspect serves its ownership and freshness checks and the pre-publication recheck when only private artifacts were written since. For those checks compare base, head, index and each files[path].hash with the saved candidate\'s base, head, index and hashes[path], and read content or diffs only for paths whose hash differs. inspect does not substitute for reading the phase and documentation section.' : 'Read the actual phase, the installed documentation section and the child assets directly; inspect does not substitute for those reads.'} diff --git a/test/helpers/docsync-observer.ts b/test/helpers/docsync-observer.ts index e3deb5850..019913f1d 100644 --- a/test/helpers/docsync-observer.ts +++ b/test/helpers/docsync-observer.ts @@ -418,9 +418,9 @@ function literalDocsCommandAllowed(text: string, fixture: ReturnType i % 2 === 0); - const out = rest.slice(1).filter((_, i) => i % 2 === 1)[flags.indexOf('--out')]; - return flags.every(flag => ['--out', '--audit-id', '--mode', '--base', '--select', '--docs'].includes(flag)) && - flags.filter(flag => flag === '--out').length === 1 && privateJson(out); + const values = rest.slice(1).filter((_, i) => i % 2 === 1); + return flags.every(flag => ['--out', '--audit-id', '--mode', '--base', '--select', '--docs', '--run', '--pre', '--carry'].includes(flag)) && + flags.filter(flag => flag === '--out').length === 1 && flags.every((flag, i) => !['--out', '--pre', '--carry'].includes(flag) || privateJson(values[i])); } const marker = 'GSTACK_SESSION_KIND=spawned'; const start = path.join(fixture.skills, 'bin/gstack-skill-start').split(path.sep).join('/'); diff --git a/test/review-exclude.test.ts b/test/review-exclude.test.ts index c030ee2e2..ccf92c229 100644 --- a/test/review-exclude.test.ts +++ b/test/review-exclude.test.ts @@ -724,6 +724,134 @@ describe('the receipt binds the selection the reviewers read', () => { expect({ status: w.status, err: w.status === EXIT.ok ? '' : w.err }).toEqual({ status: EXIT.ok, err: '' }); }, 240_000); }); + + // Value: protects=one /ship run whose Step 14.5 ran twice (an audit, then a base merge and restamp, then the re-audit) binds the + // digest the reviewers read when every change to the selection since the review is a file one of the run's audits edited + // (GST-1616: gbrain #8678 refused RECEIPT_INVALID field=review_exclude because the first audit's committed file stayed in the + // selection, and the only way out was HEAD's digest, which skips review of the delta), while a non-audit edit to an excluded + // file and another run's audit list still refuse; fails_when=write resets only the latest audit's files, takes a carried list + // from another run, resets an earlier audit's file that something else edited later, or drops that audit on a retry so an + // unreviewed edit to a file code names binds (the #4071 outside voice); why_new=GST-1616; + // seam=gstack-docs-candidate snapshot --run --pre --carry, then gstack-ship-receipt write --review-exclude --docs-post --docs-pre; + // no_claim=how many audits one run may launch (ship/sections/documentation.md) + describe('two docs audits in one /ship run (GST-1616)', () => { + const RUN = '/records/ship-8678.md'; + const BRANCH = { 'lib/reader.js': "exports.p = 'docs/parity.md';\n", 'docs/parity.md': '# parity\n' }; + /** The #8678 shape: reviewed; audit a1 edits docs/guide.md (excluded, untouched by the branch) and is committed; main moves and is merged; the release is restamped with `between`; audit a3 adds docs/new.md, its post snapshot carries a1's unless `carry` is false, and it is committed. `a1`/`a3` replace the audits' edits (null deletes) and `rereview` re-takes the reviewed digest after the restamp. */ + const twoAudits = (opts: { between?: Record; carry?: boolean; a1?: Record; a3?: Record; rereview?: boolean } = {}) => { + const repo = makeFixtureRepo({ prefix: 'gstack-exclude-audits-', baseFiles: { '.gstack/ship-policy.json': policyText(['docs/**', 'test/fixtures/goldens/**']), 'docs/guide.md': '# guide\n', 'lib/index.js': "exports.k = 'docs/key-files.md';\n" } }); + repos.push(repo); + commit(repo, BRANCH); + const digest = () => JSON.parse(plan(repo, ['--json']).out).review_exclude.digest as string; + let reviewed = digest(); + const apply = (files: Record) => { for (const [p, c] of Object.entries(files)) if (c === null) fs.rmSync(path.join(repo.work, p)); else write(repo.work, p, c); }; + const snapshot = (id: string, out: string, extra: string[] = [], run = RUN) => { + const file = path.join(repo.root, out); + return { file, ...runBin(repo, 'gstack-docs-candidate', ['snapshot', '--out', file, '--audit-id', id, '--mode', 'edit', '--base', repo.git('rev-parse', 'origin/main'), '--docs', 'docs', '--run', run, ...extra]) }; + }; + const saved = (s: ReturnType) => { expect({ status: s.status, err: s.err }).toMatchObject({ status: 0 }); return s.file; }; + const pre1 = saved(snapshot('a1', 'a1.json')); + apply(opts.a1 ?? { 'docs/guide.md': '# guide\n\nUpdated by the docs audit.\n' }); + const post1 = saved(snapshot('a1', 'a1-post.json', ['--pre', pre1])); + commit(repo, {}, 'docs audit a1'); + onBase(repo, { 'notes/base.md': '# base\n' }); + commit(repo, { VERSION: '1.0.1\n', ...opts.between }, 'restamp'); + if (opts.rereview) reviewed = digest(); + const pre = saved(snapshot('a3', 'a3.json')); + apply(opts.a3 ?? { 'docs/new.md': '# new\n' }); + const post = saved(snapshot('a3', 'a3-post.json', ['--pre', pre, ...(opts.carry === false ? [] : ['--carry', post1])])); + commit(repo, {}, 'docs audit a3'); + return { repo, reviewed, pre, post, post1, snapshot, body: path.join(repo.root, 'body.md') }; + }; + const writeWith = (a: ReturnType, post = a.post) => + writeReceipt(a.repo, a.body, ['--review-exclude', a.reviewed, '--docs-post', post, '--docs-pre', a.pre, ...targetedAtHead(a.repo)]); + const refusedWith = (w: { status: number | null; err: string }, a: ReturnType, why: string) => { + expect({ status: w.status, written: fs.existsSync(a.body) }).toEqual({ status: EXIT.fail, written: false }); + expect(w.err).toContain(`recorded=${a.reviewed} `); + expect(w.err).toContain(why); + }; + + test('the #8678 shape: a re-audit after a base merge and restamp binds the reviewed digest with both audits\' files reset', () => { + const lone = twoAudits({ carry: false }); + refusedWith(writeWith(lone), lone, 'is not the selection HEAD has with the files under review_exclude_paths'); + const a = twoAudits(); + const w = writeWith(a); + expect({ status: w.status, err: w.status === EXIT.ok ? '' : w.err }).toEqual({ status: EXIT.ok, err: '' }); + expect(w.err + w.out).toContain(`REVIEW_EXCLUDE_DOCS_AUDIT: reviewed=${a.reviewed.slice(0, 12)} `); + expect(w.err + w.out).toContain('files=docs/guide.md,docs/new.md '); + expect(receiptIn(a.body).review_exclude!.digest).not.toBe(a.reviewed); + }, 240_000); + + test('a file no audit edited, or an edit after an audit to the file it edited, still refuses', () => { + const stray = twoAudits({ between: { 'docs/stray.md': '# stray\n' } }); + refusedWith(writeWith(stray), stray, 'is not the selection HEAD has with the files under review_exclude_paths'); + const later = twoAudits({ between: { 'docs/guide.md': '# guide\n\nEdited after the docs audit.\n' } }); + refusedWith(writeWith(later), later, 'docs/guide.md at HEAD is not the content audit a1 recorded'); + const kept = twoAudits({ a1: { 'docs/parity.md': '# parity\naudited\n' }, between: { 'docs/parity.md': '# parity\nan unreviewed edit\n' } }); + refusedWith(writeWith(kept), kept, 'docs/parity.md at HEAD is not the content audit a1 recorded'); + }, 360_000); + + test('another run\'s audit list is refused, at the snapshot and at write', () => { + const a = twoAudits(); + const otherPre = a.snapshot('a3', 'a3-other-pre.json', [], '/records/another-run.md'); + expect({ status: otherPre.status, err: otherPre.err }).toMatchObject({ status: 0 }); + const other = a.snapshot('a3', 'a3-other.json', ['--pre', otherPre.file, '--carry', a.post1], '/records/another-run.md'); + expect({ status: other.status, written: fs.existsSync(other.file) }).toEqual({ status: 1, written: false }); + expect(other.err).toContain('another /ship run'); + const forged = path.join(a.repo.root, 'a3-forged.json'); + const post = JSON.parse(fs.readFileSync(a.post, 'utf8')); + post.earlier[0].run_id = '/records/another-run.md'; + fs.writeFileSync(forged, JSON.stringify(post)); + refusedWith(writeWith(a, forged), a, 'audit a1 of another /ship run'); + }, 240_000); + + // Value: protects=write refuses a carried list it cannot trust: no --docs-pre, a --docs-pre of another run, a malformed, duplicated + // or run-less earlier, a carried HEAD off the snapshot's history, or a file changed between two audits outside both; + // fails_when=a guard in writeExcludeBinds or carriedAudits is dropped and write binds the reviewed digest anyway; + // why_new=only the success path, stray/later edits and another run's entry were covered; seam=none; no_claim=snapshot-side refusals + test('write refuses a carried audit list it cannot trust', () => { + const a = twoAudits(); + refusedWith(writeReceipt(a.repo, a.body, ['--review-exclude', a.reviewed, '--docs-post', a.post, ...targetedAtHead(a.repo)]), a, 'carries audit a1, so write needs --docs-pre'); + const forge = (name: string, edit: (j: any) => void) => { const j = JSON.parse(fs.readFileSync(name === 'pre' ? a.pre : a.post, 'utf8')); edit(j); const f = path.join(a.repo.root, `forged-${name}-${Math.random().toString(36).slice(2)}.json`); fs.writeFileSync(f, JSON.stringify(j)); return f; }; + const otherPre = forge('pre', j => { j.run_id = '/records/another-run.md'; }); + refusedWith(writeReceipt(a.repo, a.body, ['--review-exclude', a.reviewed, '--docs-post', a.post, '--docs-pre', otherPre, ...targetedAtHead(a.repo)]), a, 'its HEAD, audit_id or run_id differs'); + const orphan = a.repo.git('commit-tree', '-m', 'orphan', a.repo.git('hash-object', '-t', 'tree', '-w', '/dev/null')); + const rows: Array<[(j: any) => void, string]> = [ + [j => { j.earlier = j.earlier[0]; }, 'its earlier field is not a list of audits'], + [j => { j.earlier[0].edited = []; }, 'is not { run_id, audit_id, head, edited }'], + [j => { j.earlier.push(j.earlier[0]); }, 'carries audit a1 twice'], + [j => { j.earlier[0].head = orphan; }, `carried audit a1's HEAD ${orphan.slice(0, 12)} is not an ancestor`], + [j => { j.earlier.push({ ...j.earlier[0], audit_id: 'a2', edited: { 'docs/guide.md': { before: null, after: null } } }); }, 'docs/guide.md changed between audit a1 and audit a2, outside both'], + ]; + for (const [edit, why] of rows) refusedWith(writeWith(a, forge('post', edit)), a, why); + const runless = (j: any) => { delete j.run_id; }; + refusedWith(writeReceipt(a.repo, a.body, ['--review-exclude', a.reviewed, '--docs-post', forge('post', runless), '--docs-pre', forge('pre', runless), ...targetedAtHead(a.repo)]), a, 'names no run_id'); + }, 240_000); + + // Value: protects=write binds an honest re-audit that undid, or re-edited, the carried audit's file, and one whose review ran + // after the carried audit; fails_when=the moved skip drops a file the re-audit reverted, the reset target or the Prepare-side + // check for a twice-edited file regresses, or write insists on the carried audit when the review already read it; + // why_new=the other GST-1616 rows edit disjoint files and review before both audits; seam=none; no_claim=a live /ship run + test('write binds a re-audit that reverts or re-edits the carried audit\'s file, and a review between the two audits', () => { + const binds = (a: ReturnType, files: string, audits: string) => { + const w = writeWith(a); + expect({ status: w.status, err: w.status === EXIT.ok ? '' : w.err }).toEqual({ status: EXIT.ok, err: '' }); + expect(w.err + w.out).toContain(`files=${files}${audits} (`); + }; + binds(twoAudits({ a1: { 'docs/x.md': '# x\n' }, a3: { 'docs/x.md': null, 'docs/new.md': '# new\n' } }), 'docs/new.md,docs/x.md', ' audits=a1,a3'); + binds(twoAudits({ a1: { 'docs/guide.md': null }, a3: { 'docs/guide.md': '# guide\n', 'docs/new.md': '# new\n' } }), 'docs/guide.md,docs/new.md', ' audits=a1,a3'); + const again = twoAudits({ a3: { 'docs/guide.md': '# guide\n\nEdited again by the re-audit.\n', 'docs/new.md': '# new\n' } }); + binds(again, 'docs/guide.md,docs/new.md', ' audits=a1,a3'); + const between = path.join(again.repo.root, 'between.md'); + fs.writeFileSync(between, '# guide\n\nEdited between the audits.\n'); + const pre = JSON.parse(fs.readFileSync(again.pre, 'utf8')); + pre.hashes['docs/guide.md'] = again.repo.git('hash-object', '-w', between); + fs.writeFileSync(again.pre, JSON.stringify(pre)); + fs.rmSync(again.body, { force: true }); + refusedWith(writeWith(again), again, "docs/guide.md at this audit's Prepare snapshot is not the content audit a1 recorded"); + binds(twoAudits({ a1: { 'docs/x.md': '# x\n' }, rereview: true }), 'docs/new.md', ''); + }, 480_000); + }); }); describe('a branch cannot hide code under a base pattern', () => { diff --git a/test/size-ledger/ship/capy-gst-1616-two-docs-audits.json b/test/size-ledger/ship/capy-gst-1616-two-docs-audits.json new file mode 100644 index 000000000..8f797a4b1 --- /dev/null +++ b/test/size-ledger/ship/capy-gst-1616-two-docs-audits.json @@ -0,0 +1,4 @@ +{ + "bytes": 206, + "why": "GST-1616: Step 14.5's post snapshot names its Prepare snapshot and carries a re-audit's earlier audit; Step 19 passes the last audit's pair" +}