mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 16:34:00 +01:00
fix(ship-receipt): a docs audit after the review no longer forces another review (GST-1429) (#4034)
* fix(ship-receipt): a docs audit after the review no longer voids the review_exclude binding (GST-1429) Step 14.5's docs audit runs after the review. When it edits, adds or removes a file under review_exclude_paths, HEAD's selection digest moves and receipt write refused the reviewed digest. write --docs-post <audit-id>-post.json now binds the reviewed digest when it is HEAD's selection with the files under a pattern that changed since the snapshot's HEAD reset to it, each named by the snapshot with the content HEAD holds and excluded or kept only because code names it, and every other file selected alike. Anything else still refuses. The ship driver's Step 19 write omitted --review-exclude, so gbrain #8627 wrote recorded=none; it now passes it, and the refusal says when the flag was left out. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test(review-exclude): keep the GST-1429 in-process receipt read off the real state root The #8627-shape test called receiptReviewAtHead in-process with the runner's GSTACK_STATE_ROOT, so its gate scan wrote gate-diff and slug-cache entries under the real ~/.gstack and CI rejected free-suite shard 9 (FREE_SUBSHARD_REJECTED, run 42017628509). It now points GSTACK_STATE_ROOT at the fixture like the docs-only gate-tagged test does. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: pre-landing review fixes The docs-audit exception reset every changed file under review_exclude_paths to the post snapshot's HEAD, so a docs edit the review read while it was still uncommitted was reverted and write refused a normal /ship run. --docs-pre <Prepare snapshot> now narrows the reset to the files the audit changed, back to their Prepare content. Snapshot hashes of working-tree bytes also compare against HEAD's content as it checks out, so autocrlf or a clean filter no longer reads as an edit after the audit. Review-Finding: s11/outside-voice#1 reader=outside-voice Review-Finding: s11/outside-voice#2 reader=outside-voice Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * chore: changelog and TODOS gstack: ok 1.96.0.0 for=ship Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
Garry Tanandcapy-ai[bot] authored and GitHub committed
1 parent
b406820980
commit
467be16ab2
11 files changed
+220
-14
No files matched your search
+10
-3
@@ -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 <jso
|
||||
carries when neither the recorded selection nor HEAD's holds a file the PR changed.
|
||||
The check binds the digest to the base policy, not to a review: write does not read the review ledger, so the expected=
|
||||
digest a refusal prints passes too. After a refusal, rerun the review and pass the digest its verify prints.
|
||||
--docs-post <audit-id>-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 <Prepare snapshot> (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 <json|@file> (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 <classes> (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 <ledger> and write with the --review-exclude <d> 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 <audit-id>-post.json and --docs-pre <its Prepare snapshot>; otherwise rerun /ship's review on ${head.slice(0, 12)} (gstack-review-dispatch pass --base ${base}), run gstack-review-dispatch verify --ledger <ledger> and write with the --review-exclude <d> 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<typeof parseReviewTargeted> | undefined;
|
||||
if (flag(rest, '--review-targeted') !== undefined) {
|
||||
|
||||
@@ -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 <audit-id>-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)
|
||||
@@ -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 <audit-id>-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 <Prepare snapshot>` 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
|
||||
|
||||
@@ -3496,6 +3496,11 @@ the field without a review. Do not do that. Rerun
|
||||
run `gstack-review-dispatch verify --ledger <ledger>`, which prints
|
||||
`REVIEW_EXCLUDE_RECORD: digest=<d>` from the briefs the readers got, then
|
||||
`gstack-ship-receipt write --pr <n> --review-exclude <d> --body <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 <audit-id>-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
|
||||
|
||||
@@ -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<string> | 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<string> | 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, unknown> } | 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<string, unknown> };
|
||||
};
|
||||
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';
|
||||
|
||||
+2
-2
@@ -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<NEW_VERSION> `. `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 <top> --sections 'Why,Summary,Blast radius' --gate`; `gstack-issue-links`; `gstack-review-scope summary --since "$REVIEW_SINCE" --run <dir> && gstack-ship-receipt write --pr <n> --base <base> --gate @<f> --pregate <f> --issues @<f> --review-loop @<f> --docs @<f> [--review-targeted @<f>] --body <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<NEW_VERSION> ` 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 <top> --sections 'Why,Summary,Blast radius' --gate`; `gstack-issue-links`; `gstack-review-scope summary --since "$REVIEW_SINCE" --run <dir> && gstack-ship-receipt write --pr <n> --base <base> --gate @<f> --pregate <f> --issues @<f> --review-loop @<f> --docs @<f> [--review-exclude <d>] [--review-targeted @<f>] --body <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 <n>`: `GSTACK_RESULT: skill=ship status=complete head=<sha> run=<dir>`. Under a merge queue, `gstack-ship-queue ready --pr <n> [--screen|--no-screen]` at the pushed head; paste its output.
|
||||
|
||||
|
||||
+2
-2
@@ -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<NEW_VERSION> `. `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 <top> --sections 'Why,Summary,Blast radius' --gate`; `gstack-issue-links`; `gstack-review-scope summary --since "$REVIEW_SINCE" --run <dir> && gstack-ship-receipt write --pr <n> --base <base> --gate @<f> --pregate <f> --issues @<f> --review-loop @<f> --docs @<f> [--review-targeted @<f>] --body <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<NEW_VERSION> ` 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 <top> --sections 'Why,Summary,Blast radius' --gate`; `gstack-issue-links`; `gstack-review-scope summary --since "$REVIEW_SINCE" --run <dir> && gstack-ship-receipt write --pr <n> --base <base> --gate @<f> --pregate <f> --issues @<f> --review-loop @<f> --docs @<f> [--review-exclude <d>] [--review-targeted @<f>] --body <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 <n>`: `GSTACK_RESULT: skill=ship status=complete head=<sha> run=<dir>`. Under a merge queue, `gstack-ship-queue ready --pr <n> [--screen|--no-screen]` at the pushed head; paste its output.
|
||||
|
||||
|
||||
@@ -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 @<file>` (`{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=<d>`, add `--review-exclude <d>`;
|
||||
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=<d>`, add `--review-exclude <d>`
|
||||
and Step 14.5's `--docs-post <audit-id>-post.json` and `--docs-pre <its Prepare snapshot>`; 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
|
||||
|
||||
@@ -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 @<file>` (`{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=<d>`, add `--review-exclude <d>`;
|
||||
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=<d>`, add `--review-exclude <d>`
|
||||
and Step 14.5's `--docs-post <audit-id>-post.json` and `--docs-pre <its Prepare snapshot>`; 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
|
||||
|
||||
@@ -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<string, string | null>, after: Record<string, string | null> = {}, branch: Record<string, string> = BRANCH, opts: { dirty?: Record<string, string>; 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<typeof audited>, 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<typeof audited>, 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<typeof receiptReviewAtHead>['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', () => {
|
||||
|
||||
@@ -0,0 +1,4 @@
|
||||
{
|
||||
"bytes": 225,
|
||||
"why": "GST-1429: pr-body.md names --docs-post and --docs-pre for the docs-audit exclusion exception"
|
||||
}
|
||||
Reference in new issue
Block a user