diff --git a/.gstack/ship-policy.json b/.gstack/ship-policy.json index d188eced9..d50acbc38 100644 --- a/.gstack/ship-policy.json +++ b/.gstack/ship-policy.json @@ -14,5 +14,5 @@ "pr_count": { "default_pr_count": 1, "split_requires_owner": true }, "maintainers": ["garrytan"], "train": { "gate_workflow": "evals.yml", "free_workflow": "free-tests.yml", "full_gate_check": "gstack/full-gate", "record_artifact": "gstack-train-record", "max_size": 3, "fill_hours": 1, "screen_selection": "gstack-paid", "trial_reservation_usd": 1.5 }, - "min_gstack_version": "1.91.146.0" + "min_gstack_version": "1.91.299.0" } diff --git a/CHANGELOG.md b/CHANGELOG.md index f9e7f26e2..295d897c3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,21 @@ ## [Unreleased] +**/ship now reviews in proportion to what the diff can break: light, standard or full depth.** A diff with 50 or fewer runtime lines gets one batched specialist plus the native adversarial pass, a larger runtime diff gets the class's specialists scoped to their hunks, and gate code or any classifier error gets the full review as before. The receipt records the depth and the queue refuses a receipt below what the head needs. Before this, a 12-line fix paid for the same reviewer fan-out as a change to the merge gate. The backtest that would have let light depth drop the adversarial pass was inconclusive (9 light-eligible PRs, 5 with attributable findings, below the 15 the plan required), so light keeps it. + +### Added +- **Review depth** (`lib/review-depth.ts`, `lib/review-collect.ts`, `gstack-review-plan`): `REVIEW_DEPTH:` (depth, runtime lines, the rules hash, reasons), `GATE_SCAN:` bound to HEAD, one `DEPTH_ERROR:` per input that fails closed to full (with the input, the cause and the repair), and per-reviewer `DIFF_CMD:` pathspecs. Runtime lines leave out docs, tests and outputs registered with a command in the base branch's `.gstack/generated.json`, read at the base commit. `--depth-floor standard|full` only raises the depth. A fixed blocker or a CRITICAL raises light to standard for the rest of the invocation. +- **Receipt fields**: `gstack_version` and `review_depth` (`{ depth, class, runtime_lines, rules, base, registry, gate_scan: { tagged, head }, reasons }`), computed at the checked-out head. `write --head` must be HEAD. `review_loop_exception` (`write --review-loop-exception`) records an owner-accepted stop after a fix-check edit with the fix-check's head and findings, and the PR body repeats its line word for word (Fixes #3559). +- **Queue checks** (`gstack-ship-queue ready`, `train build`): the depth the head needs is recomputed from git and a lower receipt is `QUEUE_RECEIPT_DEPTH_RAISED`, with the receipt's depth, the required depth, reasons and inputs on a `REVIEW_DEPTH:` line. A malformed owner exception, or a body that dropped its line, is `QUEUE_RECEIPT_EXCEPTION_INVALID`. A receipt with neither new field predates depth receipts and is accepted as legacy. + +### Changed +- **Carry and depth**: a carry recomputes the depth at the new head, rebinds the gate scan, and refuses when the new head needs more (a test change that skips a test is gate class). +- **`.gstack/ship-policy.json`**: `min_gstack_version` rises from 1.91.146.0 to 1.91.299.0. + +### Tests +- `test/ship-receipt-depth.test.ts`: write records the depth bound to HEAD and refuses another `--head`. The queue refuses light on a standard head, a current receipt with no depth and an unbound gate scan, and accepts a legacy receipt. A plain test change carries and rebinds the scan, while a skipped test refuses the carry. The review-loop exception is refused without a fix-check, with a foreign head or without its body line, and the queue refuses a body that dropped the line. +- `test/review-depth.test.ts`, `test/review-collect.test.ts`, `test/review-plan-cli.test.ts`: the depth matrix, the fixed light set, fail-closed errors and the registry read at the base commit. + **A /ship receipt now survives a base merge that only regenerates outputs, and a rebase onto the tree it reviewed.** On gstack-evals, two full /ship reruns came from a merge of main that changed a generated file's diff (U1 paid about 28M tokens for its rerun), and one came from a rebase whose tree was byte-identical to the reviewed one (about 60M). Each re-reviewed code the receipt already covered. For crews the rule is now a single line. Merge main into a reviewed branch, and never rebase it. ### Changed diff --git a/bin/gstack-doctor-components.sh b/bin/gstack-doctor-components.sh index af0815ec9..dda793216 100644 --- a/bin/gstack-doctor-components.sh +++ b/bin/gstack-doctor-components.sh @@ -286,6 +286,8 @@ gstack_result_anchor() { QUEUE_TRAIN_FROZEN) printf '%s' 'queue-train-frozen' ;; QUEUE_RECEIPT_MISSING) printf '%s' 'queue-receipt-missing' ;; QUEUE_RECEIPT_STALE) printf '%s' 'queue-receipt-stale' ;; + QUEUE_RECEIPT_DEPTH_RAISED) printf '%s' 'queue-receipt-depth-raised' ;; + QUEUE_RECEIPT_EXCEPTION_INVALID) printf '%s' 'queue-receipt-exception-invalid' ;; QUEUE_SCREEN_STALE_REF) printf '%s' 'queue-screen-stale-ref' ;; QUEUE_SCREEN_NOT_RUN) printf '%s' 'queue-screen-not-run' ;; QUEUE_TRAIN_STACK_RED) printf '%s' 'queue-train-stack-red' ;; @@ -306,6 +308,7 @@ gstack_result_anchor() { REVIEW_SCOPE_RANGE) printf '%s' 'review-scope-range' ;; RECEIPT_CARRY_REFUSED) printf '%s' 'receipt-carry-refused' ;; PROSE_LINT_FOUND) printf '%s' 'prose-lint-found' ;; + REVIEW_LOOP_EXCEPTION_INVALID) printf '%s' 'review-loop-exception-invalid' ;; RECEIPT_CARRY_TREE_MISMATCH) printf '%s' 'receipt-carry-tree-mismatch' ;; RECEIPT_CARRY_PRODUCT_CHANGED) printf '%s' 'receipt-carry-product-changed' ;; DOCS_AUDIT_BLOCKED) printf '%s' 'docs-audit-blocked' ;; diff --git a/bin/gstack-ship-receipt b/bin/gstack-ship-receipt index e6d89eaf9..2a615333a 100755 --- a/bin/gstack-ship-receipt +++ b/bin/gstack-ship-receipt @@ -36,7 +36,7 @@ import { EXIT, exitTableHelp, type ShipReceipt } from '../lib/headless-artifacts import { resultLine, resultLineWithDocs } from '../lib/result-codes'; import { versionAtRef } from '../lib/restamp'; import { readPregateFile, verifyPregate } from '../lib/pregate/index'; -import { isUnavailable, loadPolicyFromBase, matchesAny, policyUnavailableLine, resolveBaseBranch, stampFilesInTree, validatePolicy, type ShipPolicy } from '../lib/ship-policy'; +import { isUnavailable, loadPolicyFromBase, matchesAny, policyUnavailableLine, resolveBaseBranch, runningGstackVersion, stampFilesInTree, validatePolicy, type ShipPolicy } from '../lib/ship-policy'; import { approvalForReceipt } from '../lib/approval-citations'; import { CARRY_METADATA_GLOBS, CARRY_NAMED_PATHS, CARRY_TEST_GLOBS, @@ -44,6 +44,7 @@ import { receiptIssuesFrom, receiptSummaryLine, renderCensus, renderReceiptBlock, validateReceipt, type CensusPr, } from '../lib/ship-receipt'; import { checkDocsCaps, docsCapLine, docsExceptionLine, parseReceiptDocs } from '../lib/ship-receipt-docs'; +import { deeper, depthBelow, parseReviewLoopException, receiptDepthAtHead, reviewLoopExceptionLine } from '../lib/ship-receipt-depth'; import { checkReceiptProse, proseLine } from '../lib/ship-receipt-prose'; import { resolveStateRoot } from '../lib/state-root'; import { isRegistryUnavailable, loadRegistryFromBase } from '../lib/regen'; @@ -83,6 +84,10 @@ write flags: --pr --head --base --version --gated-tree --predecessor --gate ({passes, blocking_found, deferred[, noted, fixed, scope]}: gstack-review-scope summary --out) --docs ({status: updated|current|blocked, scope[, exception: {accepted_by, date, via, reason}]}: Step 14.5's outcome; blocked without an exception is DOCS_AUDIT_BLOCKED, and with --body the body must carry the exception line verbatim) + --review-loop-exception ({accepted_by, date, via, reason, fix_check: {head, findings[, files]}}: the owner accepted a + stop after a fix-check edit; needs --review-loop with fix_checks >= 1, fix_check.head an ancestor of the head, and with --body the + "Review-loop exception: …" line verbatim; else REVIEW_LOOP_EXCEPTION_INVALID). Every receipt records gstack_version and + review_depth (gstack-review-plan --json at HEAD; --head must be HEAD, and no plan means nothing written). --approval-run (repeatable; resolves plan-sha citations) (W2: a class declared in origin/'s .gstack/approval-classes.json with hits needs a citation that resolves and binds to its effects: APPROVAL_CITATION_MISSING) --out --body . Defaults: head=HEAD, base=origin/HEAD's branch, version=the @@ -153,8 +158,14 @@ function reviewedProduct(repoRoot: string, base: string, head: string, pregateFi function cmdWrite(rest: string[], json: boolean): number { const repoRoot = git(['rev-parse', '--show-toplevel']) ?? process.cwd(); - const head = flag(rest, '--head') ?? git(['rev-parse', 'HEAD'], repoRoot); + const checkedOut = git(['rev-parse', 'HEAD'], repoRoot); + const head = flag(rest, '--head') ?? checkedOut; if (!head) { process.stderr.write('gstack-ship-receipt write: --head is required outside a git repository\n'); return EXIT.usage; } + const headSha = git(['rev-parse', '--verify', '--quiet', `${head}^{commit}`], repoRoot); + if (checkedOut && headSha !== checkedOut) { + process.stderr.write(resultLine('gstack-ship-receipt write', 'RECEIPT_INVALID', `--head ${head.slice(0, 12)} is not the checked-out HEAD ${checkedOut.slice(0, 12)}; the receipt binds the reviewed tree, so check out ${head.slice(0, 12)} and write there; nothing written`) + '\n'); + return EXIT.fail; + } const receiptFile = flag(rest, '--receipt'); const tree = receiptFile ? JSON.parse(fs.readFileSync(receiptFile, 'utf8')) : null; let sessionKind = flag(rest, '--session-kind') ?? process.env.GSTACK_SESSION_KIND; @@ -182,6 +193,19 @@ function cmdWrite(rest: string[], json: boolean): number { return EXIT.refused; } } + let loopException: ReturnType | undefined; + if (flag(rest, '--review-loop-exception') !== undefined) { + const refuse = (why: string) => { process.stderr.write(resultLineWithDocs('gstack-ship-receipt write', 'REVIEW_LOOP_EXCEPTION_INVALID', `${why}; nothing written`) + '\n'); return EXIT.fail; }; + try { loopException = parseReviewLoopException(jsonArg(flag(rest, '--review-loop-exception'))); } catch (e: any) { return refuse(`--review-loop-exception: ${e.message}`); } + if (!loopException.ok) return refuse(loopException.message); + let loop: unknown; try { loop = jsonArg(flag(rest, '--review-loop')); } catch {} + if (!parseReviewLoop(loop)?.fix_checks) return refuse('--review-loop-exception needs --review-loop with fix_checks >= 1: the stop it accepts follows a fix-check'); + const fc = loopException.exception.fix_check.head; + if (spawnSync('git', ['merge-base', '--is-ancestor', fc, head], { cwd: repoRoot }).status !== 0) return refuse(`fix_check.head ${fc.slice(0, 12)} is not ${head.slice(0, 12)} or an ancestor of it`); + const line = reviewLoopExceptionLine(loopException.exception); + if (bodyFile && !bodyText.split(/\r?\n/).some(l => l.trim() === line)) return refuse(`${bodyFile} does not carry the exception line verbatim: ${JSON.stringify(line)}`); + } + const reviewDepth = receiptDepthAtHead(repoRoot, base, path.dirname(Bun.fileURLToPath(import.meta.url)), headSha ?? head); const approval = approvalForReceipt({ repoRoot, base, head, citationText: [bodyText, ...flags(rest, '--approval')].join('\n'), runDirs: flags(rest, '--approval-run') }); if (approval.kind === 'invalid') { for (const line of approval.lines) process.stderr.write(resultLine('gstack-ship-receipt write', 'APPROVAL_CLASSES_INVALID', line) + '\n'); @@ -212,6 +236,7 @@ function cmdWrite(rest: string[], json: boolean): number { policy: flag(rest, '--policy') ?? tree?.policy, queue_mode: flag(rest, '--queue-mode'), preregistration: flag(rest, '--preregistration'), history: flag(rest, '--history'), ...pregate, issues: issuesFile ? receiptIssuesFrom(issuesFile) : undefined, approval: approval.kind === 'ok' ? approval.approval : undefined, review_loop: reviewLoop, ...(docs?.ok ? { docs: docs.docs } : {}), ...reviewedProduct(repoRoot, base, head, pregateFile), + gstack_version: runningGstackVersion(), review_depth: reviewDepth, review_loop_exception: loopException?.ok ? loopException.exception : undefined, }; } catch (e: any) { process.stderr.write(`gstack-ship-receipt write: ${e.message}\n`); return EXIT.usage; } if (input.pr !== undefined && !Number.isInteger(input.pr)) { process.stderr.write('gstack-ship-receipt write: --pr must be an integer\n'); return EXIT.usage; } @@ -568,7 +593,13 @@ function cmdCarry(rest: string[], json: boolean): number { } else if (receipt.pregate && !rest.includes('--dry-run')) { return refused('RECEIPT_CARRY_PREGATE_STALE', `the receipt records a pregate for ${receipt.head.slice(0, 12)}; run gstack-pregate at HEAD ${head.slice(0, 12)} and pass --pregate `); } - const carried = carryReceipt(pregateField ? { ...receipt, ...pregateField } : receipt, head, plan.carry, { tree: headTree, fromTree, productPatchId: headPatch }); + let depthField: Partial = {}; + if (receipt.review_depth) { + const now = receiptDepthAtHead(repoRoot, base, path.dirname(Bun.fileURLToPath(import.meta.url)), head); + if (depthBelow(receipt.review_depth.depth, now.depth)) return refused('RECEIPT_CARRY_REFUSED', `HEAD ${head.slice(0, 12)} needs ${now.depth} review depth (${now.reasons.join(', ')}), above the receipt's ${receipt.review_depth.depth}; run /ship on ${head.slice(0, 12)}`); + depthField = { review_depth: { ...now, depth: deeper(receipt.review_depth.depth, now.depth) } }; + } + const carried = carryReceipt({ ...receipt, ...pregateField, ...depthField }, head, plan.carry, { tree: headTree, fromTree, productPatchId: headPatch }); const errors = validateReceipt(carried); if (errors.length) return refused('RECEIPT_CARRY_REFUSED', `the carried receipt does not validate: ${errors[0]!.message} at ${errors[0]!.path}`); const newBody = placeReceiptFirst(body, carried); diff --git a/docs/review-policy.md b/docs/review-policy.md index c6847c73f..d9e87dd77 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -248,6 +248,13 @@ specialist stats they record). The summary line appends `fix_checks=`, `dispatches=` and `est_input_tokens=` only when they were recorded, so an old loop prints the old line. +Beside the counters, `write` records `review_depth` (the depth, runtime lines, +rules hash, base and registry commits, and the gate scan bound to the head) and +the queue refuses a receipt below the depth its head needs +([ship-policy.md#ship-receipt](ship-policy.md#ship-receipt)). A stop the owner +accepted after a fix-check edit goes in `review_loop_exception`, never in prose +under `REVIEW_LOOP:`. + The receipt's summary line prints `passes= blocking_found= deferred=`, which is what a scorecard tracks (target: a median of 1 pass per PR). A receipt carried across a base merge or a test-only fix diff --git a/docs/ship-policy.md b/docs/ship-policy.md index 8890a2f4a..f93d655f0 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -777,7 +777,8 @@ written by the pr-body section of `/ship` after the body is composed. The ```` ```gstack-ship-receipt ```` block is the handoff record: identifiers first (`pr`, `head`, `version`, `base`), then the evidence (`gate`, `ci`, -`pregate`, `issues`, `approval`, `docs`, `review_loop`, `carry`), validated +`pregate`, `issues`, `approval`, `docs`, `review_loop`, `review_depth`, +`review_loop_exception`, `carry`), validated against the `ship-receipt` schema (`gstack-artifact schema ship-receipt`) and read back by `gstack-ship-receipt read`, `result`, `carry` and the queue. @@ -812,6 +813,39 @@ unreadable base policy is `POLICY_SOURCE_UNAVAILABLE`); one (`absent/ over` for a path the tree lacks), exit 1 on any `over`, whose repair is trimming the file, never raising the cap in the same PR. +**`review_depth` and `gstack_version`.** `write` records the gstack that +wrote the receipt and the review depth at the checked-out head +([review-policy.md#review-depth](review-policy.md#review-depth)): +`{ depth, class, runtime_lines, rules, base, registry, gate_scan: { tagged, head }, reasons }`, +computed from git objects for the range against `origin/`, with the +gate scan run on that head. `--head` must be the checked-out HEAD +(`RECEIPT_INVALID` otherwise), so the record and `head_tree` describe the +tree the review read. `gstack-ship-queue ready` and `train build` recompute +the depth the head needs and print `REVIEW_DEPTH: receipt= required= +runtime_lines= class= reasons=<…> inputs=base@,registry@,gate_scan@`; +a recorded gate scan counts only when it names the bound head, otherwise the +scan is unavailable and the head needs `full`. A receipt below the required +depth, or one whose writer records `gstack_version` but no `review_depth`, is +[`QUEUE_RECEIPT_DEPTH_RAISED`](troubleshooting.md#queue-receipt-depth-raised): +the receipt is left unchanged and the fix is /ship on that head. A receipt with +neither field predates depth receipts and is accepted as legacy. A carry +recomputes the depth at the new head, rebinds the gate scan, and refuses when +the new head needs a deeper review than the receipt records (a test change that +skips a test raises the diff to gate class). + +**`review_loop_exception`.** The owner's acceptance of a stop after a +fix-check edit under frozen scope (#3559), from `write --review-loop-exception +`: `{ accepted_by, date, via, reason, fix_check: { head, findings[, files] } }`. +It needs `--review-loop` with `fix_checks` of 1 or more, a `fix_check.head` +that is the receipt head or its ancestor, and, with `--body`, the line +`Review-loop exception: accepted by via : (fix-check at , findings)` +word for word; otherwise +[`REVIEW_LOOP_EXCEPTION_INVALID`](troubleshooting.md#review-loop-exception-invalid) +and nothing is written. The queue checks both owner exceptions the same way +before READY and train build: the record must parse and the body must still +carry its line, or the receipt is +[`QUEUE_RECEIPT_EXCEPTION_INVALID`](troubleshooting.md#queue-receipt-exception-invalid). + **Prose against the receipt.** `gstack-ship-receipt read --pr --check-prose` (or `--body `) compares every counter the body quotes with the receipt's integers under `review_loop`, `gate` and `ci`: `passes=2`, `passes: 2` and diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index e180fbf97..0566be1f5 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -4434,6 +4434,36 @@ written for the pinned head is the binding one and `verify --train` accepts it. `gstack-ship-queue ready --pr --screen|--no-screen`. A lane never stamps to fix this: the stamp comes only when the slot is called. + +### `QUEUE_RECEIPT_DEPTH_RAISED` + +**Meaning.** `ready` or `train build` recomputed the review depth the head +needs from git (the range against `origin/`, the base commit's +generated-output registry, and the receipt's gate scan when it names this +head) and the receipt's `review_depth.depth` is below it, or the receipt comes +from a gstack that records `gstack_version` but carries no `review_depth`. The +`REVIEW_DEPTH:` line prints the receipt's depth, the required depth, runtime +lines, the class, the reasons and the inputs (`base@`, `registry@`, +`gate_scan@` or `missing`). A gate scan recorded for another head counts as +missing, which makes the head need `full`. The receipt is unchanged. + +**Fix.** Run /ship on the head the READY names; it reviews at the depth +`gstack-review-plan` plans for that head and writes a receipt that records it. +Never edit `review_depth` by hand ([review-policy.md](review-policy.md#review-depth)). + + +### `QUEUE_RECEIPT_EXCEPTION_INVALID` + +**Meaning.** The receipt records an owner exception (`docs.exception` or +`review_loop_exception`) that the queue cannot accept: the record does not +parse, a review-loop exception names no fix-check (`review_loop.fix_checks` +is 0 or missing), or the PR body no longer carries the exception's line word +for word (`Documentation exception: …` or `Review-loop exception: …`). + +**Fix.** Restore the line in the PR body exactly as `write` printed it, or run +/ship again and record the owner's acceptance with `--docs` or +`--review-loop-exception` ([ship-policy.md](ship-policy.md#ship-receipt)). + ### `QUEUE_SCREEN_STALE_REF` @@ -4707,6 +4737,20 @@ head, so `gstack-ship-queue ready` refuses `QUEUE_RECEIPT_STALE`. already rewritten and the receipt predates `head_tree`, run /ship on the new head; next time merge the base in instead of rebasing. + +### `REVIEW_LOOP_EXCEPTION_INVALID` + +**Meaning.** `gstack-ship-receipt write --review-loop-exception` refused the +record and wrote nothing: it is not `{ accepted_by, date, via, reason, +fix_check: { head, findings[, files] } }`, `--review-loop` is missing or counts +no fix-check, `fix_check.head` is neither the receipt head nor its ancestor, +or the `--body` file lacks the `Review-loop exception: …` line. + +**Fix.** Record the owner's exact words and the fix-check the stop followed, +pass the `gstack-review-scope summary` file as `--review-loop`, put the printed +line in the PR body, and write again +([ship-policy.md](ship-policy.md#ship-receipt)). + ### `RECEIPT_CARRY_TREE_MISMATCH` diff --git a/lib/headless-artifacts.ts b/lib/headless-artifacts.ts index 2c37e3dcc..d387f38f8 100644 --- a/lib/headless-artifacts.ts +++ b/lib/headless-artifacts.ts @@ -102,6 +102,11 @@ export interface ShipReceipt { carry?: { from: string; reason: string; product_patch_id: string; changed_paths: string[]; merge_commits: number; range_diff: string; covers: string[]; reruns: string[]; trees?: string[]; ancestry?: 'linear' | 'rewritten' }; /** The reviewed head's tree and its product patch-id against the base (gstack-ship-receipt write; carry rebinds both): what a carry across rewritten history compares. */ head_tree?: string; product_patch_id?: string; + /** The gstack that wrote the receipt, and the review depth gstack-review-plan computed at the reviewed head (lib/ship-receipt-depth.ts). */ + gstack_version?: string; + review_depth?: { depth: 'light' | 'standard' | 'full'; class: 'docs' | 'tests' | 'code' | 'gate'; runtime_lines: number; rules: string; base: string; registry: string | null; gate_scan: { tagged: number | null; head: string }; reasons: string[] }; + /** The owner's acceptance of a stop after a fix-check edit under frozen scope (#3559); the PR body repeats it word for word. */ + review_loop_exception?: { accepted_by: string; date: string; via: string; reason: string; fix_check: { head: string; findings: number; files?: string[] } }; } export const RESULT_STATUSES: readonly ResultStatus[] = ['complete', 'gate_pending', 'amend_pending', 'incomplete', 'interrupted', 'refused']; @@ -222,7 +227,9 @@ export const ARTIFACT_SCHEMAS = { approval: { type: 'object', description: 'approval classes: { source, status: checked|unavailable, classes: [{ class, scope, effects, citation: verified|unverified, approved_by, date, via, binding, evidence }], report_only }' }, review_loop: { type: 'object', required: ['passes', 'blocking_found', 'deferred'], properties: { passes: { type: 'integer', minimum: 0 }, blocking_found: { type: 'integer', minimum: 0 }, deferred: { type: 'integer', minimum: 0 }, noted: { type: 'integer', minimum: 0 }, fixed: { type: 'integer', minimum: 0 }, scope: str, fix_checks: { type: 'integer', minimum: 0 }, dispatches: { type: 'object', description: 'reviewer dispatches by role' }, est_input_tokens: { type: 'integer', minimum: 0 } } }, carry: { type: 'object', required: ['from', 'reason', 'product_patch_id'], properties: { from: str, reason: str, product_patch_id: str, changed_paths: strList, merge_commits: { type: 'integer', minimum: 0 }, range_diff: str, covers: strList, reruns: strList, trees: strList, ancestry: { enum: ['linear', 'rewritten'] } } }, - head_tree: str, product_patch_id: str, + head_tree: str, product_patch_id: str, gstack_version: str, + review_depth: { type: 'object', required: ['depth', 'class', 'runtime_lines', 'rules', 'base', 'gate_scan'], properties: { depth: { enum: ['light', 'standard', 'full'] }, class: { enum: ['docs', 'tests', 'code', 'gate'] }, runtime_lines: { type: 'integer', minimum: 0 }, rules: str, base: str, registry: nullableStr, gate_scan: { type: 'object', required: ['tagged', 'head'], properties: { tagged: { type: ['integer', 'null'], minimum: 0 }, head: str } }, reasons: strList } }, + review_loop_exception: { type: 'object', required: ['accepted_by', 'date', 'via', 'reason', 'fix_check'], properties: { accepted_by: str, date: str, via: str, reason: str, fix_check: { type: 'object', required: ['head', 'findings'], properties: { head: str, findings: { type: 'integer', minimum: 0 }, files: strList } } } }, issues: { type: 'object', properties: { fixes: { type: 'array', items: { type: 'integer' } }, refs: { type: 'array', items: { type: 'integer' } }, flakes: { type: 'array', items: { type: 'object', required: ['issue', 'run'], properties: { issue: { type: 'integer' }, run: str } } }, related: { type: 'array', items: { type: 'object', required: ['pr', 'author', 'closes'], properties: { pr: { type: 'integer' }, repo: str, author: str, closes: { type: 'array', items: { type: 'integer' } }, supersedes: { type: 'boolean' }, credit: { type: 'boolean' } } } } } }, }, }, diff --git a/lib/result-codes-release.ts b/lib/result-codes-release.ts index 27cab8d9b..18a20b106 100644 --- a/lib/result-codes-release.ts +++ b/lib/result-codes-release.ts @@ -148,6 +148,16 @@ export const RELEASE_RESULT_CODES = { summary: 'the PR body’s /ship receipt is for another head than the one being READY’d or merged (a push after /ship, or a receipt copied from another PR)', fix: 'rerun /ship on the current head so the receipt names it, then gstack-ship-queue ready again; a slot’s stamp commit keeps the receipt written for its pinned head', }, + QUEUE_RECEIPT_DEPTH_RAISED: { + anchor: 'queue-receipt-depth-raised', + summary: 'the PR body’s /ship receipt records a review depth below the one its head needs (the queue recomputed it from git), or a current receipt records no review_depth at all', + fix: 'run /ship on the head the READY names; the REVIEW_DEPTH line shows the receipt’s depth, the required depth, its reasons and inputs, and the receipt is left unchanged', + }, + QUEUE_RECEIPT_EXCEPTION_INVALID: { + anchor: 'queue-receipt-exception-invalid', + summary: 'the PR body’s /ship receipt records an owner exception (docs or review loop) that is malformed, lacks the fix-check it follows, or whose line the body no longer carries verbatim', + fix: 'restore the exception line in the PR body word for word, or run /ship again and record the owner’s acceptance with --docs or --review-loop-exception', + }, QUEUE_SCREEN_STALE_REF: { anchor: 'queue-screen-stale-ref', summary: 'GitHub’s refs/pull//merge is missing or still merges an older head than the PR’s: the PR conflicts with the base (a CHANGELOG-only conflict counts), so a screen dispatched on it would plan no cases and report success', @@ -254,6 +264,11 @@ export const RELEASE_RESULT_CODES = { summary: 'the prose /ship is about to publish (a CHANGELOG entry or a PR body) carries an AI tell a regular expression can decide', fix: 'rewrite each PROSE: line in the draft the way its hint says, then rerun gstack-prose-lint on the same file', }, + REVIEW_LOOP_EXCEPTION_INVALID: { + anchor: 'review-loop-exception-invalid', + summary: 'write --review-loop-exception got an exception that is malformed, follows no fix-check (review_loop.fix_checks is 0 or missing), names a fix-check head that is not the receipt head or its ancestor, or is missing from the PR body', + fix: 'pass { accepted_by, date, via, reason, fix_check: { head, findings } } with the owner’s words, the --review-loop summary that counts the fix-check, and the printed "Review-loop exception: …" line in the body', + }, RECEIPT_CARRY_TREE_MISMATCH: { anchor: 'receipt-carry-tree-mismatch', summary: 'the branch was rebased or force-pushed since its /ship receipt, and HEAD\'s tree is not a tree the receipt covers, so no review saw this code', diff --git a/lib/ship-queue-train.ts b/lib/ship-queue-train.ts index e44c46a5b..5485f9b20 100644 --- a/lib/ship-queue-train.ts +++ b/lib/ship-queue-train.ts @@ -22,7 +22,7 @@ import { baseOverlapLine } from './pr-wait'; import { versionAtRef, versionFiles } from './restamp'; import type { LoadedPolicy, ShipPolicy, TrainPolicy } from './ship-policy'; import { loadBasePolicy, type QueueContext } from './ship-queue'; -import { bindReceipt } from './ship-receipt'; +import { bindShippedReceipt } from './ship-receipt-depth'; import { git, sha256, withIndex, type QueueFailure } from './ship-queue-gate'; import { recordVerdict, screenVerdict, verdictLines } from './ship-queue-screen'; import { stackTestLine, stackTests } from './ship-queue-stack-tests'; @@ -351,8 +351,8 @@ function admitSlots(env: TrainEnv, pins: Array<{ pr: string; sha: string }>, dry if (!info.headRefOid.startsWith(p.sha) || fetched !== info.headRefOid) { return outRefused({ code: 'QUEUE_TRAIN_MOVED', message: `PR #${p.pr} moved: pinned ${p.sha.slice(0, 12)}, head is now ${info.headRefOid.slice(0, 12)}` }, [], `ask #${p.pr}'s lane for READY on ${info.headRefOid.slice(0, 12)}, then ${Q} train build --prs `, 'none'); } - const shipped = bindReceipt(info.body, p.pr, [info.headRefOid]); - if (!shipped.ok) return outRefused({ code: shipped.code, message: shipped.message }, [shipped.line], `#${p.pr}'s lane runs /ship, then ${Q} ready --pr ${p.pr} --screen|--no-screen; then ${Q} train build --prs `, 'none'); + const shipped = bindShippedReceipt(ctx.repoRoot, `refs/remotes/${env.remote}/${ctx.base}`, info.body, p.pr, [info.headRefOid]); + if (!shipped.ok) return outRefused({ code: shipped.code, message: shipped.message }, [shipped.line, ...(shipped.depthLine ? [shipped.depthLine] : [])], `#${p.pr}'s lane runs /ship, then ${Q} ready --pr ${p.pr} --screen|--no-screen; then ${Q} train build --prs `, 'none'); const ready = loadReady(env.repo.keys, p.pr, info.headRefOid); if (!ready) return outRefused({ code: 'QUEUE_NOT_READY', message: `no READY receipt for #${p.pr} at ${info.headRefOid.slice(0, 12)} (local state or ${'refs/notes/gstack-trains'})` }, [], `the lane runs ${Q} ready --pr ${p.pr} --screen|--no-screen`, 'none'); let screen = ready.screen; diff --git a/lib/ship-queue.ts b/lib/ship-queue.ts index 166a9a24b..e4588ea13 100644 --- a/lib/ship-queue.ts +++ b/lib/ship-queue.ts @@ -31,7 +31,8 @@ import type { PregateFile } from './headless-artifacts'; import { CHECKS, pregateMirrorPath, readPregateFile, verifyPregate, PREGATE_OUT_REL } from './pregate/index'; import { ensureTmpExcluded, restamp, versionAtRef } from './restamp'; import type { ResultCodeName } from './result-codes'; -import { bindReceipt, extractReceipt } from './ship-receipt'; +import { extractReceipt } from './ship-receipt'; +import { bindShippedReceipt } from './ship-receipt-depth'; import { isUnavailable, loadPolicyFromBase, type LoadedPolicy } from './ship-policy'; import { QUEUE_DIR_REL, applyStampPatch, commitForTree, currentAttempt, git, listAttempts, recomputeMergeTree, sha256, writeJsonAtomic, @@ -254,7 +255,7 @@ export function ready(ctx: QueueContext, o: { pr: string; pregate?: string; gate pregateLine = `PREGATE: ${read.file.verdict} tree=${read.file.tree.slice(0, 12)} head=${(read.file.head ?? '').slice(0, 12)} checks=${read.file.checks.length} (from the state root's pregate/by-tree/${treeNow.tree.slice(0, 12)}.json, recorded by another checkout)`; } else if (gap) pregateLine += `; the state root's mirror for this tree cannot stand in: ${gap}`; } - const receipt = bindReceipt(info.body ?? '', o.pr, acceptedReceiptHeads(ctx, policy, head, info.body ?? '')); + const receipt = bindShippedReceipt(ctx.repoRoot, `origin/${ctx.base}`, info.body ?? '', o.pr, acceptedReceiptHeads(ctx, policy, head, info.body ?? '')); const attempt = currentAttempt(ctx.repoRoot, o.pr); const hint = conflictHint(info.mergeable, ci, info.headRefName, info.isCrossRepository === true, head); const lines = [ @@ -268,6 +269,7 @@ export function ready(ctx: QueueContext, o: { pr: string; pregate?: string; gate pregateLine, gateLine(policy, attempt), receipt.line, + ...(receipt.depthLine ? [receipt.depthLine] : []), ]; const gate = policy.policy.full_e2e ? gateEvidence(ctx, o.pr, head, attempt, o.gateExit, o.gateLog) : null; if (gate) lines.push(gate.line); diff --git a/lib/ship-receipt-depth.ts b/lib/ship-receipt-depth.ts new file mode 100644 index 000000000..e2c22ee02 --- /dev/null +++ b/lib/ship-receipt-depth.ts @@ -0,0 +1,147 @@ +/** + * ship-receipt-depth — what the /ship receipt says about how deeply the + * review read the head, and what the queue checks against it + * (docs/review-policy.md#review-depth, docs/ship-policy.md#ship-receipt). + * + * `review_depth`: written by `gstack-ship-receipt write` from + * gstack-review-plan at the reviewed head (depth, runtime lines, the rules + * hash, the base and registry commits, the gate scan bound to that head). + * `review_loop_exception`: the owner's acceptance of a stop after a + * fix-check edit under frozen scope (#3559), shaped like the docs exception. + * The queue recomputes the depth the head needs from git objects + * (lib/review-collect.ts) and refuses a receipt below it. Kept beside + * lib/ship-receipt.ts so that module stays under the size ratchet. + */ +import { spawnSync } from 'node:child_process'; +import * as path from 'node:path'; +import type { ShipReceipt } from './headless-artifacts'; +import { collectCommit } from './review-collect'; +import { classifyPath, depthOfCommitInput, type Depth } from './review-depth'; +import { runScriptSync } from './script-invocation'; +import { bindReceipt, type ReceiptBindingCode } from './ship-receipt'; +import { docsExceptionLine, parseReceiptDocs } from './ship-receipt-docs'; + +export type ReceiptDepth = NonNullable; +export type ReviewLoopException = NonNullable; +const RANK: Record = { light: 0, standard: 1, full: 2 }; +const text = (v: unknown): v is string => typeof v === 'string' && v.trim().length > 0 && !/[\x00-\x1f\x7f]/.test(v); +const sha = (v: unknown): v is string => typeof v === 'string' && /^[0-9a-f]{40}$/.test(v); +const short = (s: string | null | undefined) => (s ? s.slice(0, 12) : 'none'); + +function git(root: string, args: string[]): { ok: boolean; out: string } { + const r = spawnSync('git', args, { cwd: root, encoding: 'utf8', timeout: 60_000 }); + return { ok: r.status === 0, out: (r.stdout ?? '').trim() }; +} + +/** + * The depth record for `head` (the checked-out commit), computed the way the + * queue recomputes it: the range from git objects (collectCommit), plus + * gstack-diff-scope and, for a tests or code diff, the gate scan of this + * checkout, which therefore names `head`. + */ +export function receiptDepthAtHead(repoRoot: string, base: string, binDir: string, head: string): ReceiptDepth { + const branch = base.replace(/^origin\//, ''); + const baseSha = git(repoRoot, ['rev-parse', '--verify', '--quiet', `origin/${branch}^{commit}`]); + const c = collectCommit(repoRoot, baseSha.ok ? baseSha.out : branch, head); + const run = (bin: string, timeout: number) => runScriptSync(path.join(binDir, bin), [branch], { cwd: repoRoot, encoding: 'utf8', timeout, maxBuffer: 16 * 1024 * 1024 }); + const scope: Record = {}; + const sr = run('gstack-diff-scope', 60_000); + for (const l of (sr.stdout ?? '').split('\n')) { const m = /^(SCOPE_[A-Z_]+)=(.*)$/.exec(l); if (m) scope[m[1]!] = m[2]!; } + if (sr.status !== 0 && !scope.SCOPE_ERROR) scope.SCOPE_ERROR = `exit ${sr.status}`; + let tagged: number | null = 0; + if (c.paths.some(p => classifyPath(p, c.root) !== 'docs')) { + const g = run('gstack-gate-diff', 120_000); + const m = /GATE_SUMMARY: .*?tagged=\{([^}]*)\}/.exec(g.stdout ?? ''); + tagged = g.status === 0 && m ? m[1]!.split(',').reduce((n, kv) => n + (Number(kv.split(':')[1]) || 0), 0) : null; + } + const d = depthOfCommitInput({ ...c, scope, gateTagged: tagged }); + return { depth: d.depth, class: d.class, runtime_lines: d.runtimeLines, rules: d.rules, base: baseSha.ok ? baseSha.out : branch, registry: c.registry.sha, gate_scan: { tagged, head }, reasons: d.reasons.slice(0, 12) }; +} + +export const depthBelow = (have: Depth, need: Depth) => RANK[have] < RANK[need]; +export const deeper = (a: Depth, b: Depth): Depth => (RANK[a] >= RANK[b] ? a : b); + +/** `write --review-loop-exception` input and the queue's check: who accepted the stop, when, where, why, and the fix-check it followed. */ +export function parseReviewLoopException(value: unknown): { ok: true; exception: ReviewLoopException } | { ok: false; message: string } { + const bad = (message: string) => ({ ok: false as const, message }); + if (!value || typeof value !== 'object' || Array.isArray(value)) return bad('review_loop_exception must be { accepted_by, date, via, reason, fix_check: { head, findings[, files] } }'); + const v = value as Record; + for (const key of Object.keys(v)) if (!['accepted_by', 'date', 'via', 'reason', 'fix_check'].includes(key)) return bad(`review_loop_exception: unknown key ${JSON.stringify(key)}`); + for (const key of ['accepted_by', 'date', 'via', 'reason']) if (!text(v[key])) return bad(`review_loop_exception.${key} must be a non-empty one-line string`); + if (!/^\d{4}-\d{2}-\d{2}$/.test(v.date as string)) return bad('review_loop_exception.date must be YYYY-MM-DD'); + const f = v.fix_check as Record | null; + if (!f || typeof f !== 'object' || Array.isArray(f)) return bad('review_loop_exception.fix_check must be { head, findings[, files] }'); + for (const key of Object.keys(f)) if (!['head', 'findings', 'files'].includes(key)) return bad(`review_loop_exception.fix_check: unknown key ${JSON.stringify(key)}`); + if (!sha(f.head)) return bad('review_loop_exception.fix_check.head must be the full commit the fix-check reviewed'); + if (typeof f.findings !== 'number' || !Number.isInteger(f.findings) || f.findings < 0) return bad('review_loop_exception.fix_check.findings must be an integer >= 0 (what the fix-check reported)'); + if (f.files !== undefined && (!Array.isArray(f.files) || !f.files.every(text))) return bad('review_loop_exception.fix_check.files must be a list of paths'); + const files = (f.files as string[] | undefined)?.slice(0, 50); + return { ok: true, exception: { accepted_by: (v.accepted_by as string).trim(), date: v.date as string, via: (v.via as string).trim(), reason: (v.reason as string).trim(), fix_check: { head: f.head, findings: f.findings, ...(files?.length ? { files } : {}) } } }; +} + +/** The line the PR body must carry, word for word, while the exception stands. */ +export function reviewLoopExceptionLine(e: ReviewLoopException): string { + return `Review-loop exception: accepted by ${e.accepted_by} ${e.date} via ${e.via}: ${e.reason} (fix-check at ${e.fix_check.head.slice(0, 12)}, ${e.fix_check.findings} finding${e.fix_check.findings === 1 ? '' : 's'})`; +} + +const carries = (body: string, line: string) => body.split(/\r?\n/).some(l => l.trim() === line); + +/** The owner exceptions a receipt records, checked the same way: valid shape, and the body carries each line verbatim. */ +export function exceptionProblems(body: string, r: ShipReceipt): string[] { + const out: string[] = []; + const raw = r as ShipReceipt & { docs?: unknown }; + if (raw.docs !== undefined) { + const d = parseReceiptDocs(raw.docs); + if (!d.ok) out.push(`docs: ${d.message}`); + else { const line = docsExceptionLine(d.docs); if (line && !carries(body, line)) out.push(`the body does not carry the documentation exception line verbatim: ${JSON.stringify(line)}`); } + } + if (r.review_loop_exception !== undefined) { + const e = parseReviewLoopException(r.review_loop_exception); + if (!e.ok) out.push(e.message); + else { + if (!r.review_loop?.fix_checks) out.push('review_loop_exception needs review_loop.fix_checks >= 1 (the stop it accepts follows a fix-check)'); + const line = reviewLoopExceptionLine(e.exception); + if (!carries(body, line)) out.push(`the body does not carry the review-loop exception line verbatim: ${JSON.stringify(line)}`); + } + } + return out; +} + +export interface DepthVerdict { ok: boolean; line: string; message?: string } + +/** + * The depth the bound head needs against `baseRef`, recomputed from git + * objects; the recorded gate scan counts only when it names the receipt's + * head, otherwise the scan is unavailable and the head needs full. A receipt + * with no review_depth and no gstack_version predates depth receipts and is + * accepted as legacy; one from a writer that records depth but lacks it is not. + */ +export function receiptDepthVerdict(root: string, baseRef: string, r: ShipReceipt, head: string): DepthVerdict { + const rd = r.review_depth; + if (!rd && !r.gstack_version) return { ok: true, line: 'REVIEW_DEPTH: legacy receipt (written before receipts recorded review_depth); accepted' }; + const baseSha = git(root, ['rev-parse', '--verify', '--quiet', `${baseRef}^{commit}`]); + const c = collectCommit(root, baseSha.ok ? baseSha.out : baseRef, head); + const scanBound = !!rd && rd.gate_scan.tagged !== null && head.startsWith(rd.gate_scan.head); + const need = depthOfCommitInput({ ...c, gateTagged: scanBound ? rd!.gate_scan.tagged : null }); + const have = rd?.depth; + const inputs = `base@${short(baseSha.ok ? baseSha.out : null)},registry@${short(c.registry.sha)},gate_scan@${scanBound ? short(rd!.gate_scan.head) : 'missing'}`; + const line = `REVIEW_DEPTH: receipt=${have ?? 'none'} required=${need.depth} runtime_lines=${need.runtimeLines} class=${need.class} reasons=${need.reasons.join(',')} inputs=${inputs}`; + if (have && !depthBelow(have, need.depth)) return { ok: true, line }; + const why = have ? `was reviewed at ${have} depth` : 'records no review_depth though its writer records one'; + const errs = need.errors.map(e => `${e.code} (${e.input})`).join('; '); + return { ok: false, line, message: `the receipt ${why}, but head ${head.slice(0, 12)} needs ${need.depth} (${need.reasons.join(', ')}${errs ? `; ${errs}` : ''}); the receipt is unchanged; run /ship on ${head.slice(0, 12)}` }; +} + +/** bindReceipt plus the owner exceptions and the review depth: what READY and train build accept. */ +export function bindShippedReceipt(root: string, baseRef: string, body: string, pr: string | undefined, accepted: readonly string[]): + | { ok: true; receipt: ShipReceipt; head: string; line: string; depthLine: string } + | { ok: false; code: ReceiptBindingCode | 'QUEUE_RECEIPT_DEPTH_RAISED' | 'QUEUE_RECEIPT_EXCEPTION_INVALID'; message: string; line: string; depthLine?: string } { + const b = bindReceipt(body, pr, accepted); + if (!b.ok) return b; + const problems = exceptionProblems(body, b.receipt); + if (problems.length) return { ok: false, code: 'QUEUE_RECEIPT_EXCEPTION_INVALID', message: `the receipt's owner exception does not stand: ${problems.join('; ')}`, line: b.line }; + const d = receiptDepthVerdict(root, baseRef, b.receipt, b.head); + if (!d.ok) return { ok: false, code: 'QUEUE_RECEIPT_DEPTH_RAISED', message: d.message!, line: b.line, depthLine: d.line }; + return { ...b, depthLine: d.line }; +} + diff --git a/lib/ship-receipt.ts b/lib/ship-receipt.ts index ba6a03f31..02f64abd8 100644 --- a/lib/ship-receipt.ts +++ b/lib/ship-receipt.ts @@ -17,7 +17,8 @@ export const RECEIPT_FENCE = 'gstack-ship-receipt'; const KEY_ORDER: Array = [ 'schema_version', 'pr', 'head', 'version', 'base', 'gated_tree', 'predecessor', 'tree', 'gate_reuse', 'gate', 'ci', 'spend_usd', 'session_kind', 'artifacts_consumed', 'run', 'thread', 'policy', 'queue_mode', 'preregistration', 'history', - 'head_tree', 'product_patch_id', 'pregate', 'requires_remote', 'issues', 'approval', 'docs', 'review_loop', 'carry', + 'head_tree', 'product_patch_id', 'gstack_version', 'review_depth', 'pregate', 'requires_remote', 'issues', 'approval', 'docs', 'review_loop', + 'review_loop_exception', 'carry', ]; /** Keys in the fixed order so identifiers lead; undefined values are dropped. */ @@ -122,6 +123,7 @@ export function receiptSummaryLine(r: ShipReceipt): string { const loop = parseReviewLoop(r.review_loop); if (loop) parts.push(`passes=${loop.passes} blocking_found=${loop.blocking_found} deferred=${loop.deferred}` + (loop.fix_checks ? ` fix_checks=${loop.fix_checks}` : '') + (loop.dispatches ? ` dispatches=${Object.values(loop.dispatches).reduce((a, c) => a + c, 0)}` : '')); + if (r.review_depth && typeof r.review_depth.depth === 'string') parts.push(`depth=${r.review_depth.depth}`); if (r.carry && typeof r.carry.from === 'string') parts.push(`carried_from=${r.carry.from.slice(0, 12)}`); return `SHIP_RECEIPT: ${oneLine(parts.join(' '))}`; } diff --git a/ship/sections/pr-body.md b/ship/sections/pr-body.md index 414abef37..a19f4aee7 100644 --- a/ship/sections/pr-body.md +++ b/ship/sections/pr-body.md @@ -227,6 +227,9 @@ rm -f .gstack/tmp/review-loop.json `--docs` reads the record Step 14.5 saved (`{status, scope[, exception]}`): the receipt carries it as `docs`, and a `blocked` status without the owner's exception, or a body missing the exception line, is `DOCS_AUDIT_BLOCKED` with nothing written. +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:`. 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). The `&&` diff --git a/ship/sections/pr-body.md.tmpl b/ship/sections/pr-body.md.tmpl index 55103556d..c9c727af0 100644 --- a/ship/sections/pr-body.md.tmpl +++ b/ship/sections/pr-body.md.tmpl @@ -210,6 +210,9 @@ rm -f .gstack/tmp/review-loop.json `--docs` reads the record Step 14.5 saved (`{status, scope[, exception]}`): the receipt carries it as `docs`, and a `blocked` status without the owner's exception, or a body missing the exception line, is `DOCS_AUDIT_BLOCKED` with nothing written. +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:`. 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). The `&&` diff --git a/test/gstack-ship-receipt.test.ts b/test/gstack-ship-receipt.test.ts index b9fa23a3b..15ac078b2 100644 --- a/test/gstack-ship-receipt.test.ts +++ b/test/gstack-ship-receipt.test.ts @@ -44,11 +44,11 @@ describe('gstack-ship-receipt write', () => { '--session-kind', 'unattended', '--artifacts-consumed', '2/3', '--run', 'run-1', '--thread', 'jam_1', '--queue-mode', 'stamp-at-merge', '--preregistration', 'none (no preregistration_shas)', '--history', 'squash-ok', '--body', body, '--out', path.join(repo.root, 'receipt.md')]); expect(ok.status).toBe(EXIT.ok); - expect(ok.err.trim()).toBe(`SHIP_RECEIPT: pr=42 head=${head.slice(0, 12)} version=1.2.4.0 base=main gated_tree=${repo.git('rev-parse', 'HEAD^{tree}').slice(0, 12)} tree=same-modulo-stamps gate-reuse=eligible (reusable: tests; rerun: -) predecessor=${'b'.repeat(12)} session_kind=unattended artifacts_consumed=2/3`); + expect(ok.err.trim()).toBe(`SHIP_RECEIPT: pr=42 head=${head.slice(0, 12)} version=1.2.4.0 base=main gated_tree=${repo.git('rev-parse', 'HEAD^{tree}').slice(0, 12)} tree=same-modulo-stamps gate-reuse=eligible (reusable: tests; rerun: -) predecessor=${'b'.repeat(12)} session_kind=unattended artifacts_consumed=2/3 depth=light`); const parsed = extractReceipt(ok.out); expect(parsed.found).toBe(true); expect(parsed.errors).toEqual([]); - expect(Object.keys(parsed.receipt!)).toEqual(['schema_version', 'pr', 'head', 'version', 'base', 'gated_tree', 'predecessor', 'tree', 'gate_reuse', 'gate', 'ci', 'spend_usd', 'session_kind', 'artifacts_consumed', 'run', 'thread', 'policy', 'queue_mode', 'preregistration', 'history', 'head_tree', 'product_patch_id']); + expect(Object.keys(parsed.receipt!)).toEqual(['schema_version', 'pr', 'head', 'version', 'base', 'gated_tree', 'predecessor', 'tree', 'gate_reuse', 'gate', 'ci', 'spend_usd', 'session_kind', 'artifacts_consumed', 'run', 'thread', 'policy', 'queue_mode', 'preregistration', 'history', 'head_tree', 'product_patch_id', 'gstack_version', 'review_depth']); expect(parsed.receipt).toMatchObject({ schema_version: 1, pr: 42, head, version: '1.2.4.0', base: 'main', tree: 'same-modulo-stamps', spend_usd: 0.42, ci: { checks: { pass: 3 } } }); expect(parsed.receipt!.head_tree).toBe(repo.git('rev-parse', 'HEAD^{tree}')); expect(parsed.receipt!.product_patch_id).toMatch(/^([0-9a-f]{40}|none)$/); diff --git a/test/ship-receipt-depth.test.ts b/test/ship-receipt-depth.test.ts new file mode 100644 index 000000000..a22ea057d --- /dev/null +++ b/test/ship-receipt-depth.test.ts @@ -0,0 +1,154 @@ +/** + * The /ship receipt's review depth and review-loop exception (GSTA-147 PR 2, + * docs/review-policy.md#review-depth, #3559): write records the depth gstack + * computes at the checked-out head, the queue recomputes the depth the head + * needs and refuses a receipt below it, a carry rebinds the gate scan or + * refuses when the head needs more, and the review-loop exception is checked + * like the documentation exception. + */ +import { afterEach, describe, expect, test } from 'bun:test'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { EXIT } from '../lib/headless-artifacts'; +import { extractReceipt, renderReceiptBlock } from '../lib/ship-receipt'; +import { bindShippedReceipt, receiptDepthVerdict, reviewLoopExceptionLine } from '../lib/ship-receipt-depth'; +import { cleanup, commit, makeFixtureRepo, runBin, type FixtureRepo } from './helpers/pregate-fixture'; + +const repos: FixtureRepo[] = []; +afterEach(() => { while (repos.length) cleanup(repos.pop()!); }); +const receiptIn = (body: string) => extractReceipt(fs.readFileSync(body, 'utf8')).receipt!; +const writeReceipt = (repo: FixtureRepo, body: string, extra: string[] = []) => + runBin(repo, 'gstack-ship-receipt', ['write', '--pr', '7', '--base', 'main', '--version', '1.0.0', '--session-kind', 'unattended', '--body', body, ...extra]); +const lines = (n: number) => Array.from({ length: n }, (_, i) => `exports.f${i} = () => ${i};`).join('\n') + '\n'; +const rewrite = (body: string, edit: (r: Record) => void) => { + const text = fs.readFileSync(body, 'utf8'); + const r = extractReceipt(text).receipt as Record; + edit(r); + fs.writeFileSync(body, text.replace(/```gstack-ship-receipt[\s\S]*?\n```\n/, renderReceiptBlock(r as any))); + return fs.readFileSync(body, 'utf8'); +}; + +describe('write records the review depth at the checked-out head', () => { + // Value: protects=the receipt names the depth and a gate scan bound to the head it is written for, and never binds a + // tree it did not review; fails_when=write records no depth, or --head accepts a commit other than HEAD; seam=none + test('a light code diff records depth, runtime lines, the base and the gate scan at HEAD; --head of another commit writes nothing', () => { + const repo = makeFixtureRepo(); repos.push(repo); + const older = commit(repo, { 'lib/util.js': lines(3) }); + const head = commit(repo, { 'lib/more.js': lines(2) }); + const body = path.join(repo.root, 'body.md'); + const ok = writeReceipt(repo, body); + expect(ok.status).toBe(EXIT.ok); + expect(ok.err).toContain('depth=light'); + const r = receiptIn(body); + expect(r.gstack_version).toMatch(/^\d+\.\d+\.\d+\.\d+$/); + const base = repo.git('rev-parse', 'origin/main'); + expect(r.review_depth).toMatchObject({ depth: 'light', class: 'code', base, registry: base, gate_scan: { tagged: 0, head } }); + expect(r.review_depth!.runtime_lines).toBeGreaterThanOrEqual(5); + expect(r.review_depth!.runtime_lines).toBeLessThanOrEqual(50); + const other = path.join(repo.root, 'other.md'); + const bad = writeReceipt(repo, other, ['--head', older]); + expect(bad.status).toBe(EXIT.fail); + expect(bad.err).toContain(`--head ${older.slice(0, 12)} is not the checked-out HEAD ${head.slice(0, 12)}`); + expect(bad.err).toContain('(RECEIPT_INVALID)'); + expect(fs.existsSync(other)).toBe(false); + }, 60_000); +}); + +describe('the queue refuses a receipt below the depth its head needs', () => { + // Value: protects=a receipt reviewed lighter than its head needs never READYs or seats, and the refusal says what to do; + // fails_when=the queue trusts the recorded depth, or prints no required depth, reasons and inputs; seam=bindShippedReceipt + test('light on a standard head, a missing depth from a current writer and an unbound gate scan refuse; a legacy receipt is accepted', () => { + const repo = makeFixtureRepo(); repos.push(repo); + const head = commit(repo, { 'lib/big.js': lines(80) }); + const body = path.join(repo.root, 'body.md'); + expect(writeReceipt(repo, body).status).toBe(EXIT.ok); + expect(receiptIn(body).review_depth).toMatchObject({ depth: 'standard', runtime_lines: 80 }); + const base = repo.git('rev-parse', 'origin/main').slice(0, 12); + const bind = (text: string) => bindShippedReceipt(repo.work, 'origin/main', text, '7', [head]); + const good = bind(fs.readFileSync(body, 'utf8')); + expect(good.ok).toBe(true); + expect(good.ok && good.depthLine).toContain('REVIEW_DEPTH: receipt=standard required=standard runtime_lines=80 class=code'); + + const low = bind(rewrite(body, r => { r.review_depth.depth = 'light'; })); + expect(low.ok).toBe(false); + if (low.ok) return; + expect(low.code).toBe('QUEUE_RECEIPT_DEPTH_RAISED'); + expect(low.depthLine).toContain('receipt=light required=standard runtime_lines=80'); + expect(low.depthLine).toContain(`inputs=base@${base},registry@${base},gate_scan@${head.slice(0, 12)}`); + expect(low.message).toContain('runtime=80>50'); + expect(low.message).toContain(`the receipt is unchanged; run /ship on ${head.slice(0, 12)}`); + + const unbound = bind(rewrite(body, r => { r.review_depth.depth = 'standard'; r.review_depth.gate_scan.head = 'a'.repeat(40); })); + expect(unbound.ok).toBe(false); + expect(!unbound.ok && unbound.depthLine).toContain('required=full'); + expect(!unbound.ok && unbound.message).toContain('gate_scan_unavailable'); + + const none = bind(rewrite(body, r => { delete r.review_depth; })); + expect(!none.ok && none.code).toBe('QUEUE_RECEIPT_DEPTH_RAISED'); + expect(!none.ok && none.message).toContain('records no review_depth though its writer records one'); + + const legacy = rewrite(body, r => { delete r.gstack_version; }); + const verdict = receiptDepthVerdict(repo.work, 'origin/main', extractReceipt(legacy).receipt!, head); + expect(verdict).toEqual({ ok: true, line: 'REVIEW_DEPTH: legacy receipt (written before receipts recorded review_depth); accepted' }); + }, 60_000); +}); + +describe('carry and the review depth', () => { + // Value: protects=a carried receipt keeps a gate scan bound to the head it now names, and a carry whose new head needs a + // deeper review (a skipped test) is refused; fails_when=carry keeps the old scan, or carries a gate-tagged test change; seam=none + test('a plain test change carries and rebinds the gate scan; a skipped test raises the depth and refuses the carry', () => { + const repo = makeFixtureRepo(); repos.push(repo); + commit(repo, { 'lib/util.js': lines(3), 'test/util.test.js': "test('adds', () => { expect(1).toBe(1); });\n" }); + const body = path.join(repo.root, 'body.md'); + expect(writeReceipt(repo, body).status).toBe(EXIT.ok); + expect(receiptIn(body).review_depth!.depth).toBe('light'); + const plain = commit(repo, { 'test/util.test.js': "test('adds', () => { expect(1).toBe(1); });\ntest('more', () => { expect(2).toBe(2); });\n" }); + const carried = runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--base', 'main']); + expect(carried.status).toBe(EXIT.ok); + expect(receiptIn(body).review_depth).toMatchObject({ depth: 'light', gate_scan: { tagged: 0, head: plain } }); + const skipped = commit(repo, { 'test/util.test.js': "test.skip('adds', () => { expect(1).toBe(1); });\ntest('more', () => { expect(2).toBe(2); });\n" }); + const refused = runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--base', 'main']); + expect(refused.status).toBe(EXIT.refused); + expect(refused.out).toContain('CARRY: refused (RECEIPT_CARRY_REFUSED)'); + expect(refused.err).toContain(`HEAD ${skipped.slice(0, 12)} needs full review depth`); + expect(refused.err).toContain("above the receipt's light"); + expect(receiptIn(body).head).toBe(plain); + }, 90_000); +}); + +describe('the review-loop exception (#3559)', () => { + // Value: protects=an accepted stop after a fix-check is a structured record the queue checks, not prose under REVIEW_LOOP; + // fails_when=write accepts it without a fix-check, with a foreign head or without the body line, or the queue ignores a + // body that dropped the line; seam=bindShippedReceipt + test('write refuses an exception with no fix-check, a foreign head or no body line, records it otherwise, and the queue checks the line', () => { + const repo = makeFixtureRepo(); repos.push(repo); + const head = commit(repo, { 'lib/util.js': lines(3) }); + const body = path.join(repo.root, 'body.md'); + const exception = { accepted_by: 'Garry', date: '2026-10-28', via: 'GSTA-147 relay', reason: 'fix-check edit is a comment; scope frozen', fix_check: { head, findings: 1 } }; + const arg = (e: unknown) => ['--review-loop-exception', JSON.stringify(e)]; + const loop = (n: number) => ['--review-loop', JSON.stringify({ passes: 2, blocking_found: 1, deferred: 0, fix_checks: n })]; + fs.writeFileSync(body, '## What changed\n'); + for (const [extra, why] of [ + [[...arg(exception)], 'needs --review-loop with fix_checks >= 1'], + [[...arg(exception), ...loop(0)], 'needs --review-loop with fix_checks >= 1'], + [[...arg({ ...exception, fix_check: { head: 'a'.repeat(40), findings: 1 } }), ...loop(1)], 'is not ' + head.slice(0, 12) + ' or an ancestor of it'], + [[...arg({ ...exception, extra: 1 }), ...loop(1)], 'unknown key "extra"'], + [[...arg(exception), ...loop(1)], 'does not carry the exception line verbatim'], + ] as Array<[string[], string]>) { + const out = writeReceipt(repo, body, extra); + expect(out.status).toBe(EXIT.fail); + expect(out.err).toContain('(REVIEW_LOOP_EXCEPTION_INVALID;'); + expect(out.err).toContain(why); + } + expect(extractReceipt(fs.readFileSync(body, 'utf8')).found).toBe(false); + const line = reviewLoopExceptionLine(exception); + expect(line).toBe(`Review-loop exception: accepted by Garry 2026-10-28 via GSTA-147 relay: fix-check edit is a comment; scope frozen (fix-check at ${head.slice(0, 12)}, 1 finding)`); + fs.writeFileSync(body, `## What changed\n\n${line}\n`); + expect(writeReceipt(repo, body, [...arg(exception), ...loop(1)]).status).toBe(EXIT.ok); + expect(receiptIn(body).review_loop_exception).toEqual(exception); + expect(bindShippedReceipt(repo.work, 'origin/main', fs.readFileSync(body, 'utf8'), '7', [head]).ok).toBe(true); + const dropped = bindShippedReceipt(repo.work, 'origin/main', fs.readFileSync(body, 'utf8').replace(line, ''), '7', [head]); + expect(!dropped.ok && dropped.code).toBe('QUEUE_RECEIPT_EXCEPTION_INVALID'); + expect(!dropped.ok && dropped.message).toContain('does not carry the review-loop exception line verbatim'); + }, 90_000); +}); diff --git a/test/size-ledger/ship/gsta147-review-depth.json b/test/size-ledger/ship/gsta147-review-depth.json index 79b65d6cc..f583f9ecc 100644 --- a/test/size-ledger/ship/gsta147-review-depth.json +++ b/test/size-ledger/ship/gsta147-review-depth.json @@ -1,4 +1,4 @@ { - "bytes": 745, - "why": "GSTA-147 review depth: Step 9 saves REVIEW_DEPTH/GATE_SCAN, passes --depth-floor, records depth fields and reads each reviewer's DIFF_CMD; the batched light specialist; Step 11's light note; the skeleton's Review policy paragraph names the depth and --depth-floor (trimmed to hold the skeleton cap)" + "bytes": 1008, + "why": "GSTA-147 review depth: Step 9 saves REVIEW_DEPTH/GATE_SCAN, passes --depth-floor, records depth fields and reads each reviewer's DIFF_CMD; the batched light specialist; Step 11's light note; the skeleton's Review policy paragraph names the depth and --depth-floor (trimmed to hold the skeleton cap); the pr-body section's --review-loop-exception instruction (#3559)" }