diff --git a/bin/gstack-ship-receipt b/bin/gstack-ship-receipt index fc5b4dd36..bf70b39f3 100755 --- a/bin/gstack-ship-receipt +++ b/bin/gstack-ship-receipt @@ -51,7 +51,7 @@ import { import { checkDocsCaps, docsCapLine, docsExceptionLine, parseReceiptDocs } from '../lib/ship-receipt-docs'; import { comparePath, contextPacket, PACKET_SECTION_CAP, readScopedReview, touchedPaths } from '../lib/ship-patch'; import { planRecheck, readRecheck, recheckPacket } from '../lib/ship-recheck'; -import { carryExcludeBinds, deeper, depthBelow, exceptionHead, exceptionLoopProblem, exceptionProblems, parseReviewLoopException, parseReviewTargeted, plainDocsSince, platformReceiptDepth, receiptExclude, receiptReviewAtHead, receiptTargetedAtHead, reviewLoopExceptionLine, targetedClassesSince } from '../lib/ship-receipt-depth'; +import { carryExcludeBinds, deeper, depthBelow, exceptionHead, exceptionLoopProblem, exceptionProblems, parseReviewLoopException, parseReviewTargeted, plainDocsSince, platformReceiptDepth, receiptExclude, receiptReviewAtHead, receiptTargetedAtHead, reviewLoopExceptionLine, targetedClassesSince, writeExcludeBinds } from '../lib/ship-receipt-depth'; import { RISK_CLASSES, type RiskClass } from '../lib/review-risk'; import { checkReceiptProse, proseLine } from '../lib/ship-receipt-prose'; import { resolveStateRoot } from '../lib/state-root'; @@ -108,6 +108,11 @@ write flags: --pr --head --base --version --gated-tree --predecessor --gate -post.json (Step 14.5's post snapshot, the file finish takes): the reviewed --review-exclude digest also binds + when the only change to the selection since the review is files under review_exclude_paths that the snapshot names as uncommitted, + with the content HEAD holds, each excluded or kept only because code names it (GST-1429); prints REVIEW_EXCLUDE_DOCS_AUDIT: and + records HEAD's selection. Any other change, or an unreadable snapshot, refuses as above. --docs-pre (the same + audit's first snapshot) narrows the audit's files to those whose content it changed, so uncommitted docs the review read stay. --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 @@ -266,10 +271,12 @@ function cmdWrite(rest: string[], json: boolean): number { if (review.exclude.state === 'refused') { for (const l of review.exclude.lines) process.stderr.write(`${l}; nothing written\n`); return EXIT.refused; } const excludeNow = review.exclude.state === 'active' ? review.exclude.record : null; const excludeGiven = flag(rest, '--review-exclude') ?? 'none'; - if ((excludeNow?.digest ?? 'none') !== excludeGiven) { - process.stderr.write(`gstack-ship-receipt write: field=review_exclude expected=${excludeNow?.digest ?? 'none'} recorded=${excludeGiven} source=${excludeNow?.source ?? `origin/${base} (no review_exclude_paths)`}: the --review-exclude argument names another selection than ${base}'s review_exclude_paths gives ${head.slice(0, 12)} (write compares the argument, not the review ledger); fix: rerun /ship's review on ${head.slice(0, 12)} (gstack-review-dispatch pass --base ${base}), run gstack-review-dispatch verify --ledger and write with the --review-exclude its REVIEW_EXCLUDE_RECORD: line prints (none when it prints no such line); nothing written (RECEIPT_INVALID)\n`); + const excludeBound = writeExcludeBinds(repoRoot, base, headSha ?? head, review, excludeGiven, flag(rest, '--docs-post'), flag(rest, '--docs-pre')); + if (!excludeBound.ok) { + process.stderr.write(`gstack-ship-receipt write: field=review_exclude expected=${excludeNow?.digest ?? 'none'} recorded=${excludeGiven} source=${excludeNow?.source ?? `origin/${base} (no review_exclude_paths)`}: the --review-exclude argument names another selection than ${base}'s review_exclude_paths gives ${head.slice(0, 12)} (write compares the argument, not the review ledger)${flag(rest, '--review-exclude') === undefined ? '; no --review-exclude was given, so write compared none' : ''}${excludeBound.why ? `; the docs-audit exception does not apply: ${excludeBound.why}` : ''}; fix: when only Step 14.5's docs audit changed files under review_exclude_paths after the review, pass its --docs-post -post.json and --docs-pre ; otherwise rerun /ship's review on ${head.slice(0, 12)} (gstack-review-dispatch pass --base ${base}), run gstack-review-dispatch verify --ledger and write with the --review-exclude its REVIEW_EXCLUDE_RECORD: line prints (none when it prints no such line); nothing written (RECEIPT_INVALID)\n`); return EXIT.fail; } + if (excludeBound.line) process.stderr.write(excludeBound.line + '\n'); const product = reviewedProduct(repoRoot, base, head, pregateFile); let targeted: ReturnType | undefined; if (flag(rest, '--review-targeted') !== undefined) { diff --git a/changelog.d/capy-gst-1429-docs-audit-exclude.md b/changelog.d/capy-gst-1429-docs-audit-exclude.md new file mode 100644 index 000000000..2fa8c5290 --- /dev/null +++ b/changelog.d/capy-gst-1429-docs-audit-exclude.md @@ -0,0 +1 @@ +- **A docs audit after the review no longer forces another review.** /ship's Step 14.5 docs audit runs after the review, so when it added, edited or removed a file under `review_exclude_paths`, `gstack-ship-receipt write` refused the digest the reviewers read and the run had to review again. Write now takes the audit's post snapshot (`--docs-post -post.json`) and its Prepare snapshot (`--docs-pre`), and binds the reviewed digest when the audit's own files are the only change to the selection. It prints `REVIEW_EXCLUDE_DOCS_AUDIT:` when it does. Docs the review read while they were still uncommitted stay as they are, and files that git converts on checkout (autocrlf, filters) compare as committed. Any other change still refuses. The ship driver's Step 19 write now passes `--review-exclude`, and a refusal says when the flag was left out. (GST-1429) diff --git a/docs/ship-policy.md b/docs/ship-policy.md index 7c1f029be..1b657bfc1 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -1476,6 +1476,7 @@ recorded by accident, but it does not read the review ledger: a write given the the reviewers read that selection. The queue makes one exception (GST-1408): a receipt with no `review_exclude`, written before the base gained the key, still binds when no file the head changes matches any current pattern, excluded or kept, because its reviewers read the same full diff; a recorded digest that differs, a matched file and an unreadable base policy still refuse. Carry makes the same exception in both directions (GST-1415): when the digests differ but neither the receipt's recorded selection (no `review_exclude`, or one whose list hash is the empty selection's and whose digest is built from it) nor HEAD's holds a file the PR changed, the receipt carries and records HEAD's digest, so a pattern the base gains, adds, removes or reverts does not void it; a selection that holds a PR file, excluded or kept, 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. **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 a3e291dfa..09d928014 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -3496,6 +3496,11 @@ the field without a review. Do not do that. Rerun run `gstack-review-dispatch verify --ledger `, which prints `REVIEW_EXCLUDE_RECORD: digest=` from the briefs the readers got, then `gstack-ship-receipt write --pr --review-exclude --body `. +`recorded=none` with a `no --review-exclude was given` note means the write left the flag out: pass the digest +`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). The queue's check skips a receipt with `recorded=none` (written before the base gained `review_exclude_paths`) when no file the head changes matches any current pattern, so diff --git a/lib/ship-receipt-depth.ts b/lib/ship-receipt-depth.ts index 50645976f..e84b47c7c 100644 --- a/lib/ship-receipt-depth.ts +++ b/lib/ship-receipt-depth.ts @@ -18,11 +18,13 @@ * lib/ship-receipt.ts so that module stays under the size ratchet. */ import { spawnSync } from 'node:child_process'; +import { createHash, randomBytes } from 'node:crypto'; +import * as fs from 'node:fs'; import * as path from 'node:path'; import { baseContainsHead } from './default-base'; import type { ShipReceipt } from './headless-artifacts'; import { collectCommit } from './review-collect'; -import { recordSelectsNoFile, reviewExclude, type Exclusion } from './review-exclude'; +import { recordSelectsNoFile, reviewExclude, type ActiveExclusion, type Exclusion } from './review-exclude'; import { isUnavailable } from './ship-policy'; import { loadPolicyAt } from './ship-policy-at'; import { classifyPath, depthOfCommitInput, gateScanNeeded, platformFits, scanNumbers, type Depth } from './review-depth'; @@ -98,7 +100,7 @@ export function receiptDepthAtHead(repoRoot: string, base: string, binDir: strin * receiptDepthAtHead plus the exclusion the head's review read under: the depth classifies excluded paths as docs (GST-508). * `declared` (write --declare-risk, GST-1129) joins the detected risk classes and only raises; the record then keeps both lists. */ -export function receiptReviewAtHead(repoRoot: string, base: string, binDir: string, head: string, declared: readonly RiskClass[] = []): { depth: ReceiptDepth; exclude: Exclusion } { +export function receiptReviewAtHead(repoRoot: string, base: string, binDir: string, head: string, declared: readonly RiskClass[] = []): { depth: ReceiptDepth; exclude: Exclusion; tagged: ReadonlySet | null } { const branch = base.replace(/^origin\//, ''); const baseSha = baseCommit(repoRoot, `origin/${branch}`); const baseRev = baseSha.ok ? baseSha.out : branch; @@ -118,7 +120,7 @@ export function receiptReviewAtHead(repoRoot: string, base: string, binDir: stri } const exclude = exclusionAt(repoRoot, ref, sha, head, c, paths, 'gstack-ship-receipt'); const d = depthOfCommitInput({ ...c, scope, gateTagged: tagged, ...riskPathsOf(review), declared, ...(exclude.state === 'active' ? { excluded: exclude.excluded } : {}) }); - return { depth: { depth: d.depth, class: d.class, runtime_lines: d.runtimeLines, rules: d.rules, base: baseRev, registry: c.registry.sha, gate_scan: { tagged, head }, reasons: d.reasons.slice(0, 12), risk: d.risk, mode: modeFor(d.depth, review), ...(d.declared.length ? { declared: d.declared, detected: d.detected } : {}) }, exclude }; + return { depth: { depth: d.depth, class: d.class, runtime_lines: d.runtimeLines, rules: d.rules, base: baseRev, registry: c.registry.sha, gate_scan: { tagged, head }, reasons: d.reasons.slice(0, 12), risk: d.risk, mode: modeFor(d.depth, review), ...(d.declared.length ? { declared: d.declared, detected: d.detected } : {}) }, exclude, tagged: paths }; } /** @@ -297,6 +299,96 @@ export function carryExcludeBinds(x: Exclusion, r: ShipReceipt): boolean { const e = r.review_exclude; return (x.state === 'active' ? x.record.digest : 'none') === (e?.digest ?? 'none') || (selectsNoFile(x) && (!e || recordSelectsNoFile(e))); } +/** How an exclusion treats one path: excluded, the reason it is kept, or absent from the selection. */ +const treatment = (x: ActiveExclusion, p: string) => (x.excluded.has(p) ? 'excluded' : x.kept.find(k => k.path === p)?.reason ?? 'absent'); +/** + * Write's exclusion check: the --review-exclude digest the reviewers read against HEAD's. /ship's Step 14.5 docs audit runs after + * the review, so a docs file it adds, edits or removes under a pattern can change HEAD's selection (GST-1429). With `docsPost` + * (the audit's post snapshot, gstack-docs-candidate) the reviewed digest still binds when every file under a pattern that changed + * since the snapshot's HEAD was uncommitted in the snapshot with the content HEAD holds, the reviewed digest is HEAD's selection + * 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`. + */ +export function writeExcludeBinds(root: string, base: string, head: string, review: { exclude: Exclusion; tagged: ReadonlySet | null }, given: string, docsPost: string | undefined, docsPre?: string): { ok: true; line?: string } | { ok: false; why?: string } { + const x = review.exclude; + if ((x.state === 'active' ? x.record.digest : 'none') === given) return { ok: true }; + 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 }; + 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 }; + }; + 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)`); + 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(); + 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) => { + const id = blob(rev, p); + if (id === want) return true; + if (id === null || typeof want !== 'string') return false; + const out = spawnSync('git', ['cat-file', '--filters', `${rev}:${p}`], { cwd: root, timeout: 60_000, maxBuffer: 256 * 1024 * 1024 }); + return out.status === 0 && createHash('sha1').update(`blob ${out.stdout.length}\0`).update(out.stdout).digest('hex') === want; + }; + const named = new Set([...post.staged, ...post.unstaged, ...post.untracked]); + for (const p of changed) { + if (!named.has(p)) return no(`${p} changed after the snapshot's HEAD ${short(at)}, and the snapshot does not name it as an uncommitted audit file`); + 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 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 branch = base.replace(/^origin\//, ''); + const baseSha = baseCommit(root, `origin/${branch}`); + const ref = baseSha.ok ? `origin/${branch}` : branch; + const sha = baseSha.ok ? baseSha.out : (git(root, ['rev-parse', '--verify', '--quiet', `${branch}^{commit}`]).out || null); + const before = exclusionAt(root, ref, sha, reset, collectCommit(root, sha ?? branch, 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`); + } + 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)` }; +} export function excludeMismatch(x: Exclusion, r: ShipReceipt, head: string, base: string): string | null { const expected = x.state === 'active' ? x.record.digest : 'none'; const recorded = r.review_exclude?.digest ?? 'none'; diff --git a/ship/DRIVER.md b/ship/DRIVER.md index aa12f35e4..4dfaa7598 100644 --- a/ship/DRIVER.md +++ b/ship/DRIVER.md @@ -115,8 +115,8 @@ repo gates run beside CI. Unattended, push defers the pre-push offer; else its S ## 8. Publish: Steps 18, 19, 19-C, 20, 21, 22 -- Step 18: the open-PR lookup; a title file starting `v `. `UNATTENDED_SHIP:` with no open PR at HEAD takes Step 18-H. -- Step 19 body: the receipt block, then the sections `sections/pr-body.md` lists (Pre-Landing Review: `REVIEW_LOOP:`, `Gate edits:`, deferred, notes), led under `CAPY_PR: on` (Step 19-C) by What changed, Why, Blast radius and Approvals (`gstack-approvals render`). `gstack-prose-lint --sections 'Why,Summary,Blast radius' --gate`; `gstack-issue-links`; `gstack-review-scope summary --since "$REVIEW_SINCE" --run && gstack-ship-receipt write --pr --base --gate @ --pregate --issues @ --review-loop @ --docs @ [--review-targeted @] --body `; `gstack-post pr-body` and `pr-title` (new PR: host PR tool; draft while a gate finding is `open`). `SHIP_RECEIPT:` opens your report. +- Step 18: the open-PR lookup; a `v ` title file. `UNATTENDED_SHIP:` with no open PR at HEAD takes Step 18-H. +- Step 19 body: the receipt block, then sections `sections/pr-body.md` lists (Pre-Landing Review: `REVIEW_LOOP:`, `Gate edits:`, deferred, notes), led under `CAPY_PR: on` (Step 19-C) by What changed, Why, Blast radius and Approvals (`gstack-approvals render`). `gstack-prose-lint --sections 'Why,Summary,Blast radius' --gate`; `gstack-issue-links`; `gstack-review-scope summary --since "$REVIEW_SINCE" --run && gstack-ship-receipt write --pr --base --gate @ --pregate --issues @ --review-loop @ --docs @ [--review-exclude ] [--review-targeted @] --body `; `gstack-post pr-body` and `pr-title` (new PR: host PR tool; draft while a gate finding is `open`). `SHIP_RECEIPT:` opens your report. - Step 20: a metrics row via `gstack-review-log`. - Step 22: `gstack-ship-budget close --reading R --credits C` (report `BUDGET_FINAL:`, `BUDGET_TIMELINE:` (tokens, credits), any `METER:` line), then `gstack-ship-receipt result --pr `: `GSTACK_RESULT: skill=ship status=complete head= run=`. Under a merge queue, `gstack-ship-queue ready --pr [--screen|--no-screen]` at the pushed head; paste its output. diff --git a/ship/DRIVER.md.tmpl b/ship/DRIVER.md.tmpl index ca14f3bc4..8974cb376 100644 --- a/ship/DRIVER.md.tmpl +++ b/ship/DRIVER.md.tmpl @@ -113,8 +113,8 @@ repo gates run beside CI. Unattended, push defers the pre-push offer; else its S ## 8. Publish: Steps 18, 19, 19-C, 20, 21, 22 -- Step 18: the open-PR lookup; a title file starting `v `. `UNATTENDED_SHIP:` with no open PR at HEAD takes Step 18-H. -- Step 19 body: the receipt block, then the sections `sections/pr-body.md` lists (Pre-Landing Review: `REVIEW_LOOP:`, `Gate edits:`, deferred, notes), led under `CAPY_PR: on` (Step 19-C) by What changed, Why, Blast radius and Approvals (`gstack-approvals render`). `gstack-prose-lint --sections 'Why,Summary,Blast radius' --gate`; `gstack-issue-links`; `gstack-review-scope summary --since "$REVIEW_SINCE" --run && gstack-ship-receipt write --pr --base --gate @ --pregate --issues @ --review-loop @ --docs @ [--review-targeted @] --body `; `gstack-post pr-body` and `pr-title` (new PR: host PR tool; draft while a gate finding is `open`). `SHIP_RECEIPT:` opens your report. +- Step 18: the open-PR lookup; a `v ` title file. `UNATTENDED_SHIP:` with no open PR at HEAD takes Step 18-H. +- Step 19 body: the receipt block, then sections `sections/pr-body.md` lists (Pre-Landing Review: `REVIEW_LOOP:`, `Gate edits:`, deferred, notes), led under `CAPY_PR: on` (Step 19-C) by What changed, Why, Blast radius and Approvals (`gstack-approvals render`). `gstack-prose-lint --sections 'Why,Summary,Blast radius' --gate`; `gstack-issue-links`; `gstack-review-scope summary --since "$REVIEW_SINCE" --run && gstack-ship-receipt write --pr --base --gate @ --pregate --issues @ --review-loop @ --docs @ [--review-exclude ] [--review-targeted @] --body `; `gstack-post pr-body` and `pr-title` (new PR: host PR tool; draft while a gate finding is `open`). `SHIP_RECEIPT:` opens your report. - Step 20: a metrics row via `gstack-review-log`. - Step 22: `gstack-ship-budget close --reading R --credits C` (report `BUDGET_FINAL:`, `BUDGET_TIMELINE:` (tokens, credits), any `METER:` line), then `gstack-ship-receipt result --pr `: `GSTACK_RESULT: skill=ship status=complete head= run=`. Under a merge queue, `gstack-ship-queue ready --pr [--screen|--no-screen]` at the pushed head; paste its output. diff --git a/ship/sections/pr-body.md b/ship/sections/pr-body.md index 850570bdb..cf0ac1330 100644 --- a/ship/sections/pr-body.md +++ b/ship/sections/pr-body.md @@ -232,8 +232,10 @@ or a body missing the exception line, is `DOCS_AUDIT_BLOCKED` with nothing writt When the owner accepted a stop after a fix-check edit, add `--review-loop-exception @` (`{accepted_by, date, via, reason, fix_check: {head, findings}}`) and put its printed `Review-loop exception:` line in the body; never a prose line under `REVIEW_LOOP:`. -When `gstack-review-dispatch verify` printed `REVIEW_EXCLUDE_RECORD: digest=`, add `--review-exclude `; -a digest that no longer matches HEAD is `RECEIPT_INVALID field=review_exclude`: rerun the review, not the write. +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 +`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:`). The summary reads this branch's review log for the invocation's passes (its `REVIEW_LOOP:` line is the one the Pre-Landing Review section carries). Here every diff --git a/ship/sections/pr-body.md.tmpl b/ship/sections/pr-body.md.tmpl index b7cbe34e6..c5d88ca1f 100644 --- a/ship/sections/pr-body.md.tmpl +++ b/ship/sections/pr-body.md.tmpl @@ -215,8 +215,10 @@ or a body missing the exception line, is `DOCS_AUDIT_BLOCKED` with nothing writt When the owner accepted a stop after a fix-check edit, add `--review-loop-exception @` (`{accepted_by, date, via, reason, fix_check: {head, findings}}`) and put its printed `Review-loop exception:` line in the body; never a prose line under `REVIEW_LOOP:`. -When `gstack-review-dispatch verify` printed `REVIEW_EXCLUDE_RECORD: digest=`, add `--review-exclude `; -a digest that no longer matches HEAD is `RECEIPT_INVALID field=review_exclude`: rerun the review, not the write. +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 +`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:`). The summary reads this branch's review log for the invocation's passes (its `REVIEW_LOOP:` line is the one the Pre-Landing Review section carries). Here every diff --git a/test/review-exclude.test.ts b/test/review-exclude.test.ts index 34a363d8a..3514042c9 100644 --- a/test/review-exclude.test.ts +++ b/test/review-exclude.test.ts @@ -493,6 +493,98 @@ describe('the receipt binds the selection the reviewers read', () => { } }, 240_000); }); + + // Value: protects=a receipt write after /ship's Step 14.5 docs audit binds the digest the reviewers read when the only change to + // the selection since the review is files under review_exclude_paths that the audit's post snapshot names (GST-1429: gbrain + // #8627's audit added a key-files entry, a docs file code names, and write refused with RECEIPT_INVALID field=review_exclude, + // forcing a third review pass), a docs edit the review read uncommitted stays when --docs-pre names the Prepare snapshot and + // checkout line endings compare as committed (the #4034 review), while a selection change from anything else, an audit file the exclusion reads as code, an + // edit after the snapshot and a missing snapshot still refuse; fails_when=write accepts a changed selection without the + // snapshot, or the exception spreads to files the snapshot does not name or the exclusion does not hide or keep as docs; + // why_new=GST-1429; seam=gstack-docs-candidate snapshot, then gstack-ship-receipt write --review-exclude --docs-post; + // no_claim=what the docs audit itself may edit (ship/sections/documentation.md) + describe('a docs audit after the review (GST-1429)', () => { + const BRANCH = { 'lib/reader.js': "exports.p = 'docs/parity.md';\n", 'docs/parity.md': '# parity\n', 'docs/old.md': '# old\n' }; + /** A branch reviewed at its head with `dirty` uncommitted (the plan's digest is what verify prints), then audited: the Prepare snapshot is saved, `audit` edits the tree, the post snapshot is saved, `after` edits more, and everything is committed. */ + const audited = (audit: Record, after: Record = {}, branch: Record = BRANCH, opts: { dirty?: Record; autocrlf?: boolean } = {}) => { + const repo = makeFixtureRepo({ prefix: 'gstack-exclude-audit-', baseFiles: { '.gstack/ship-policy.json': policyText(['docs/**', 'test/fixtures/goldens/**']), 'docs/key-files.md': '# key files\n', 'lib/index.js': "exports.k = 'docs/key-files.md';\n" } }); + repos.push(repo); + commit(repo, branch); + if (opts.autocrlf) repo.git('config', 'core.autocrlf', 'true'); + for (const [rel, text] of Object.entries(opts.dirty ?? {})) write(repo.work, rel, text); + const reviewed = JSON.parse(plan(repo, ['--json']).out).review_exclude.digest as string; + const snapshot = (out: string) => { + const snap = runBin(repo, 'gstack-docs-candidate', ['snapshot', '--out', out, '--audit-id', 'a1', '--mode', 'edit', '--base', repo.git('rev-parse', 'origin/main'), '--docs', 'docs']); + expect({ status: snap.status, err: snap.err }).toMatchObject({ status: 0 }); + }; + const pre = path.join(repo.root, 'a1.json'); + snapshot(pre); + for (const [rel, text] of Object.entries(audit)) { if (text === null) fs.rmSync(path.join(repo.work, rel), { force: true }); else write(repo.work, rel, text); } + const post = path.join(repo.root, 'a1-post.json'); + snapshot(post); + commit(repo, after, 'docs audit and release'); + return { repo, reviewed, pre, post, body: path.join(repo.root, 'body.md') }; + }; + const writeWith = (a: ReturnType, post: string | null = a.post, pre: string | null = null) => + writeReceipt(a.repo, a.body, ['--review-exclude', a.reviewed, ...(post === null ? [] : ['--docs-post', post]), ...(pre === null ? [] : ['--docs-pre', 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(`field=review_exclude expected=`); + expect(w.err).toContain(`recorded=${a.reviewed} `); + expect(w.err).toContain(why); + }; + + test('the #8627 shape: an audit edit to a docs file code names binds the reviewed digest with the post snapshot only', () => { + const a = audited({ 'docs/key-files.md': '# key files\n- lib/reader.js: the parity guard\n' }, {}, { 'lib/reader.js': BRANCH['lib/reader.js'], 'docs/parity.md': BRANCH['docs/parity.md'] }); + const prior = process.env.GSTACK_STATE_ROOT; + process.env.GSTACK_STATE_ROOT = path.join(a.repo.root, 'receipt-state'); + let head: ReturnType['exclude']; + try { head = receiptReviewAtHead(a.repo.work, 'main', path.join(import.meta.dir, '..', 'bin'), a.repo.git('rev-parse', 'HEAD')).exclude; } + finally { if (prior === undefined) delete process.env.GSTACK_STATE_ROOT; else process.env.GSTACK_STATE_ROOT = prior; } + expect(head.state === 'active' ? head.kept.map(k => [k.path, k.reason]) : head.state).toEqual([['docs/key-files.md', 'named-by-code'], ['docs/parity.md', 'named-by-code']]); + expect(head.state === 'active' && head.record.digest).not.toBe(a.reviewed); + refusedWith(writeWith(a, null), a, '(write compares the argument, not the review ledger)'); + 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(receiptIn(a.body).review_exclude!.digest).toBe(head.state === 'active' ? head.record.digest : 'none'); + }, 240_000); + + test('an audit that adds an excluded docs file and removes one the branch added binds', () => { + const a = audited({ 'docs/new.md': '# new\n', 'docs/old.md': null }); + 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=docs/new.md,docs/old.md'); + }, 240_000); + + test('a selection change outside the audit, or an audit file the exclusion reads as code, still refuses', () => { + const code = audited({ 'docs/new.md': '# new\n' }, { 'lib/old.js': "exports.o = 'docs/old.md';\n" }); + refusedWith(writeWith(code), code, 'is not the selection HEAD has with the files under review_exclude_paths that changed since the snapshot reset'); + const outside = audited({ 'docs/new.md': '# new\n', 'lib/reader.js': "exports.p = 'docs/parity.md';\nexports.o = 'docs/old.md';\n" }); + refusedWith(writeWith(outside), outside, 'is not the selection HEAD has with the files under review_exclude_paths that changed since the snapshot reset'); + const tool = audited({ 'docs/tool.js': 'exports.t = 1;\n' }); + refusedWith(writeWith(tool), tool, 'docs/tool.js is code at HEAD'); + }, 240_000); + + test('an edit after the snapshot, a file the snapshot does not name and a missing snapshot still refuse', () => { + const late = audited({ 'docs/new.md': '# new\n' }, { 'docs/new.md': '# new, edited after the snapshot\n' }); + refusedWith(writeWith(late), late, 'docs/new.md at HEAD is not the content the snapshot recorded'); + const unnamed = audited({}, { 'docs/new.md': '# new\n' }); + refusedWith(writeWith(unnamed), unnamed, 'docs/new.md changed after the snapshot'); + refusedWith(writeWith(unnamed, path.join(unnamed.repo.root, 'missing-post.json')), unnamed, 'missing-post.json: cannot be read'); + }, 240_000); + + test('a docs edit the review read uncommitted stays with the Prepare snapshot, and checkout line endings compare as committed', () => { + const dirty = audited({ 'docs/new.md': '# new\n' }, {}, BRANCH, { dirty: { 'docs/key-files.md': '# key files\n- lib/reader.js: the parity guard\n' } }); + refusedWith(writeWith(dirty), dirty, 'is not the selection HEAD has with the files under review_exclude_paths that changed since the snapshot reset'); + const kept = writeWith(dirty, dirty.post, dirty.pre); + expect({ status: kept.status, err: kept.status === EXIT.ok ? '' : kept.err }).toEqual({ status: EXIT.ok, err: '' }); + expect(kept.err + kept.out).toContain('files=docs/new.md '); + const crlf = audited({ 'docs/new.md': '# new\r\n' }, {}, BRANCH, { autocrlf: true }); + const w = writeWith(crlf, crlf.post, crlf.pre); + expect({ status: w.status, err: w.status === EXIT.ok ? '' : w.err }).toEqual({ status: EXIT.ok, err: '' }); + }, 240_000); + }); }); describe('a branch cannot hide code under a base pattern', () => { diff --git a/test/size-ledger/ship/capy-gst-1429-docs-audit-exclude.json b/test/size-ledger/ship/capy-gst-1429-docs-audit-exclude.json new file mode 100644 index 000000000..c1c568604 --- /dev/null +++ b/test/size-ledger/ship/capy-gst-1429-docs-audit-exclude.json @@ -0,0 +1,4 @@ +{ + "bytes": 225, + "why": "GST-1429: pr-body.md names --docs-post and --docs-pre for the docs-audit exclusion exception" +}