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 b490efc99..6a2cf2153 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,24 @@ # Changelog +## [1.91.307.0] - 2026-10-29 + +**/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 or open 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, a body that dropped its line, or a fix-check head that is not the bound head or its ancestor is `QUEUE_RECEIPT_EXCEPTION_INVALID`. The queue reads the same committed range as the writer and trusts a recorded gate scan only when it names the full bound commit. 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). It also refuses a `--head` other than the checked-out HEAD, a receipt with `gstack_version` but no `review_depth`, and an owner exception that no longer stands, since the queue would refuse each of them. Tracked edits in the checkout make the write-time gate scan unavailable, which records `full`. +- **`gstack-diff-scope --head `**: the scope signals for the committed range only, with no working-tree or untracked files; an unknown sha is `SCOPE_ERROR=no_head` and a missing merge base `SCOPE_ERROR=no_merge_base`. +- **`.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 `package.json` version line that also carries other keys is not stamp-only. +- `test/diff-scope.test.ts`: `--head` reads the committed range and ignores working-tree edits, and refuses an unknown sha. + ## [1.91.306.0] - 2026-10-29 **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. diff --git a/TODOS.md b/TODOS.md index d2be7ab44..edcf57447 100644 --- a/TODOS.md +++ b/TODOS.md @@ -2,6 +2,11 @@ ## NEXT PRIORITY +### P1: review-depth receipt follow-ups (filed 2026-10-28, from the #3567 /ship plan audit) + +- **Derive `carry.covers` from depth.** A carried receipt's `covers` is the fixed `CARRY_COVERS` list (lib/ship-receipt.ts); derive it from the recorded `review_depth` so a light review's carry does not claim the checks a full review ran (plan item 5, deferred at /ship). **Effort:** S. **Priority:** P1. +- **Bound the queue's recompute time.** `gstack-ship-queue` recomputes each receipt's needed depth at READY and at train build; add a test that a five-PR train recomputes within a fixed budget, so a large diff cannot stall the train. **Effort:** S. **Priority:** P1. + ### P3: `/codex --effort` follow-ups (filed 2026-10-11, from the #3146 autoplan review) - **Show the effort that ran.** Step 0.5 prints `CODEX_EFFORT: (source: /codex --effort)` once, but the result footers do not repeat the effort, so it scrolls away. Add an `Effort:` line to the review, challenge and consult result footers. Blocked on skeleton bytes in `codex/SKILL.md` (cap 60,200). **Effort:** S. **Priority:** P3. diff --git a/VERSION b/VERSION index 98b9266b3..019ebe885 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.91.306.0 +1.91.307.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index 74c007ef9..d30192102 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.91.306.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.91.307.0 — regenerate/re-copy after upgrading gstack Behavioral rules from gstack (https://github.com/garrytan/gstack), compressed for agent hosts without a full skill install. The full skills add workflows, diff --git a/bin/gstack-diff-scope b/bin/gstack-diff-scope index 2fa10b41d..3ec81cf09 100755 --- a/bin/gstack-diff-scope +++ b/bin/gstack-diff-scope @@ -2,6 +2,9 @@ # gstack-diff-scope — categorize what changed in the diff against a base branch # Usage: source <(gstack-diff-scope main) → sets SCOPE_FRONTEND=true SCOPE_BACKEND=false ... # Or: gstack-diff-scope main → prints SCOPE_*=... lines +# Or: gstack-diff-scope --head → the committed range ... only +# (no working tree, no untracked files): what a receipt writer and the +# merge queue compute for one commit, from any checkout (GSTA-147). # # Output contract (#2526 — all-false must be distinguishable from "we could # not look" and from "nothing matched"): @@ -31,6 +34,8 @@ _default_base() { echo "main" } BASE="${1:-$(_default_base)}" +HEAD_REF="" +if [ "${2:-}" = "--head" ]; then HEAD_REF="${3:-}"; fi FRONTEND=false BACKEND=false @@ -79,6 +84,21 @@ _collect() { [ -n "$f" ] && FILES_LIST+=("$f") done } +if [ -n "$HEAD_REF" ] || [ "${2:-}" = "--head" ]; then + if ! git rev-parse --verify -q "${HEAD_REF}^{commit}" >/dev/null 2>&1; then + _print_flags + echo "SCOPE_ERROR=no_head" + echo "# --head '${HEAD_REF}' is not a commit in this repository" + exit 2 + fi + if ! git merge-base "${BASE}" "${HEAD_REF}" >/dev/null 2>&1; then + _print_flags + echo "SCOPE_ERROR=no_merge_base" + echo "# '${BASE}' and '${HEAD_REF}' share no merge base" + exit 2 + fi + _collect < <(git diff -z "${BASE}...${HEAD_REF}" --name-only 2>/dev/null) +else # Committed diff vs base (merge-base form; two-dot fallback when no merge base). _collect < <(git diff -z "${BASE}...HEAD" --name-only 2>/dev/null || git diff -z "${BASE}" --name-only 2>/dev/null || true) # Working-tree changes (staged + unstaged). `git diff HEAD` fails on a repo @@ -87,6 +107,7 @@ _collect < <(git diff -z HEAD --name-only 2>/dev/null || true) # Untracked files: a brand-new component/migration/test is exactly what a # reviewer should see, and /ship commits it in Step 15 regardless. _collect < <(git ls-files -z --others --exclude-standard 2>/dev/null || true) +fi if [ "${#FILES_LIST[@]}" -eq 0 ]; then _print_flags 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-review-plan b/bin/gstack-review-plan index a5424c25a..06f3e3976 100755 --- a/bin/gstack-review-plan +++ b/bin/gstack-review-plan @@ -4,20 +4,25 @@ * * gstack-review-plan --base [--pass ] [--last none|full|fix-check] [--fixed-blocking] [--fixed-below] * [--blocking-open] [--touched-gate] [--critical-found] [--adversarial-ran] [--gate-tagged ] [--gated a,b] [--force a,b|all] - * [--reviewed-numbers ] [--no-tiers] [--json] + * [--reviewed-numbers ] [--depth-floor standard|full] [--no-tiers] [--json] * gstack-review-plan --fix-check-prompt --fix-list --hunks --tests [--callers ] * - * Reads the diff against the merge base with (committed, working tree and - * untracked), runs gstack-diff-scope and, unless --gate-tagged is given, - * gstack-gate-diff; prints REVIEW_CLASS:, REVIEW_PLAN:, one DISPATCH: line per - * reviewer and one SKIP: line per reviewer left out. Logic: lib/review-depth.ts. + * Reads the diff against the merge base with (committed, tracked uncommitted + * edits and untracked files, through lib/review-collect.ts collectLive), runs + * gstack-diff-scope and, unless --gate-tagged is given, gstack-gate-diff; prints + * REVIEW_CLASS:, REVIEW_DEPTH: (depth, runtime lines, the rules hash, reasons and + * result codes), GATE_SCAN:, REVIEW_PLAN:, one DEPTH_ERROR: per input that failed + * the depth closed, one DISPATCH: plus DIFF_CMD: (and DIFF_EXCLUDE: none) per + * reviewer and one SKIP: per reviewer left out. A plan that fails closed to full + * exits 0; a diff git cannot list exits 1. Logic: lib/review-depth.ts. */ -import { spawnSync } from 'node:child_process'; import * as fs from 'node:fs'; import * as path from 'node:path'; +import { spawnSync } from 'node:child_process'; import { EXIT, exitTableHelp } from '../lib/headless-artifacts'; import { runScriptSync } from '../lib/script-invocation'; -import { planReview, reviewPlanLine, scanNumbers, type History } from '../lib/review-depth'; +import { collectLive } from '../lib/review-collect'; +import { diffCommand, planReview, reviewDepthLine, reviewPlanLine, scanNumbers, type Depth, type History } from '../lib/review-depth'; import { renderFixCheckPrompt } from '../scripts/resolvers/fix-check'; const HELP = `gstack-review-plan — the review depth of the next /ship step (docs/review-policy.md#review-depth) @@ -25,19 +30,23 @@ const HELP = `gstack-review-plan — the review depth of the next /ship step (do --base the base branch (merge base with HEAD is the diff base); required --pass full review passes already run in this invocation (default 0) --last none|full|fix-check the step that just finished (default none: plan pass 1) - --fixed-blocking the finished step fixed a blocking finding + --fixed-blocking the finished step fixed a blocking finding (raises light to standard) --fixed-below the finished step fixed findings below the floor --blocking-open a blocking finding is still open in fix scope --touched-gate a fix touched a gate-class path - --critical-found a specialist reported a CRITICAL finding (Red Team on code class) + --critical-found a specialist reported a CRITICAL finding (Red Team on code class; raises light to standard) --adversarial-ran Step 11's native adversarial pass already ran (a repeat skips it) --gate-tagged gate hunks the gate scan tagged (default: run gstack-gate-diff; failure means unavailable) --gated a,b specialists the hit-rate stats tag GATE_CANDIDATE --force a,b|all specialists forced by prompt flags (--all-specialists is all) --reviewed-numbers the numeric-claim fingerprint the outside voice already reviewed + --depth-floor standard|full raise the depth to at least this (never lowers it); /ship passes its prompt flag on every run --no-tiers audits and watchers at the host's default effort too (as gstack-config review_effort_tiers off does) - --json the plan as JSON + --json the plan as JSON (depth, runtime_lines, rules, errors with input and repair, gate_scan, per-dispatch exclude) --fix-check-prompt print the fix-check reviewer's prompt from --fix-list, --hunks, --tests and --callers files + +Depth: light (runtime lines <= 50), standard (above 50), full (gate class or any DEPTH_ERROR). A full plan +from an error exits 0; exit 1 means no plan could be built (git could not list the diff). ${exitTableHelp()}`; const args = process.argv.slice(2); @@ -65,34 +74,17 @@ function main(): number { if (!['none', 'full', 'fix-check'].includes(last)) { process.stderr.write('gstack-review-plan: --last must be none, full or fix-check\n'); return EXIT.usage; } const pass = Number(flag('--pass') ?? '0'); if (!Number.isInteger(pass) || pass < 0) { process.stderr.write('gstack-review-plan: --pass needs an integer >= 0\n'); return EXIT.usage; } + const floor = flag('--depth-floor') as Depth | undefined; + if (has('--depth-floor') && floor !== 'standard' && floor !== 'full') { process.stderr.write('gstack-review-plan: --depth-floor must be standard or full (a floor only raises the depth)\n'); return EXIT.usage; } const root = git(['rev-parse', '--show-toplevel'])?.trim(); if (!root) { process.stderr.write('gstack-review-plan: run it inside a git checkout\n'); return EXIT.fail; } process.chdir(root); - const ref = git(['rev-parse', '--verify', '-q', `origin/${base}`]) !== null ? `origin/${base}` : base; - const mergeBase = git(['merge-base', ref, 'HEAD'])?.trim(); - if (!mergeBase) { process.stderr.write(`gstack-review-plan: no merge base with ${ref} (fetch the base first)\n`); return EXIT.fail; } - const nameStatusRaw = git(['diff', '--name-status', '--no-renames', mergeBase]); - const untrackedRaw = git(['ls-files', '--others', '--exclude-standard']); - const numstatRaw = git(['diff', '--numstat', mergeBase]); - if (nameStatusRaw === null || untrackedRaw === null || numstatRaw === null) { process.stderr.write('gstack-review-plan: git could not list the diff; no plan is safer than a plan for an empty diff\n'); return EXIT.fail; } - const scratch = (p: string) => p.startsWith('.gstack/tmp/'); - const nameStatus = nameStatusRaw.split('\n').filter(Boolean).map(l => l.split('\t')).filter(c => !scratch(c[1] ?? '')); - const untracked = untrackedRaw.split('\n').filter(Boolean).filter(p => !scratch(p)); - const paths = [...new Set([...nameStatus.map(c => c[1]!).filter(Boolean), ...untracked])].sort(); - const deleted = nameStatus.filter(c => c[0] === 'D').map(c => c[1]!); - const stampOnly = paths.filter(p => p.endsWith('package.json') && !untracked.includes(p)).filter(p => { - const changed = (git(['diff', '-U0', '--no-color', mergeBase, '--', p]) ?? '').split('\n').filter(l => /^[+-]/.test(l) && !/^(\+\+\+|---) /.test(l)); - return changed.length > 0 && changed.every(l => /^[+-]\s*"version"\s*:/.test(l)); - }); - const diffLines = numstatRaw.split('\n').reduce((n, l) => { const [a, d] = l.split('\t'); return n + (Number(a) || 0) + (Number(d) || 0); }, 0) - + untracked.reduce((n, p) => { try { return n + fs.readFileSync(p, 'utf8').split('\n').length; } catch { return n; } }, 0); - const added: Record = {}; - let current = ''; - for (const line of (git(['diff', '-U0', '--no-color', mergeBase]) ?? '').split('\n')) { - if (line.startsWith('+++ b/')) { current = line.slice(6); continue; } - if (line.startsWith('+') && !line.startsWith('+++') && current) (added[current] ??= []).push(line.slice(1)); + const collected = collectLive(root, base); + if (collected.errors.length) { + for (const e of collected.errors) process.stderr.write(`gstack-review-plan: git could not list the diff (${e.input}: ${e.detail}); fix: ${e.repair}; no plan is safer than a plan for an empty diff\n`); + return EXIT.fail; } - for (const p of untracked) { try { added[p] = fs.readFileSync(p, 'utf8').split('\n'); } catch { /* unreadable: no claims */ } } + const head = git(['rev-parse', 'HEAD'])?.trim() ?? 'unknown'; const binDir = path.dirname(Bun.fileURLToPath(import.meta.url)); const scopeRun = runScriptSync(path.join(binDir, 'gstack-diff-scope'), [base], { encoding: 'utf8', timeout: 60_000 }); const scope: Record = {}; @@ -109,14 +101,25 @@ function main(): number { const force = flag('--force') === 'all' ? 'all' as const : list('--force'); const tiersSetting = runScriptSync(path.join(binDir, 'gstack-config'), ['get', 'review_effort_tiers'], { encoding: 'utf8', timeout: 15_000 }); const auditEffort = has('--no-tiers') || (tiersSetting.stdout ?? '').trim() === 'off' ? 'host' : 'light'; + const { added, diffLines, paths, deleted, stampOnly, perPath, registry } = collected; const plan = planReview({ - paths, deleted, stampOnly, root, scope, gateTagged, diffLines, numbers: scanNumbers(added), force, gated: list('--gated'), + ...collected, scope, gateTagged, numbers: scanNumbers(added), force, gated: list('--gated'), floor, criticalFound: has('--critical-found'), adversarialRan: has('--adversarial-ran'), reviewedNumbers: flag('--reviewed-numbers'), history: { fullPasses: pass, last, fixedBlocking: has('--fixed-blocking'), fixedBelow: has('--fixed-below'), blockingOpen: has('--blocking-open'), touchedGate: has('--touched-gate') }, }); - if (has('--json')) { process.stdout.write(JSON.stringify({ ...plan, diff_lines: diffLines, pass, audit_effort: auditEffort, scope, gate_tagged: gateTagged, paths, deleted, stamp_only: stampOnly }) + '\n'); return EXIT.ok; } - const out = [`REVIEW_CLASS: ${plan.class} (${plan.reasons.slice(0, 3).join('; ') || 'no signals'})`, reviewPlanLine(plan, pass + (plan.next.step === 'full' ? 1 : 0)), `NEXT: ${plan.next.step} (${plan.next.reason})`]; - for (const d of plan.dispatches) out.push(`DISPATCH: ${d.role} ${d.name} effort=${d.effort}`); + const gateScan = { tagged: gateTagged, head }; + if (has('--json')) { + const { runtimeLines, stepDepth, depthReasons, ...rest } = plan; + process.stdout.write(JSON.stringify({ ...rest, step_depth: stepDepth, runtime_lines: runtimeLines, depth_reasons: depthReasons, depth_floor: floor ?? null, gate_scan: gateScan, registry: { sha: registry.sha, outputs: registry.outputs }, per_path: perPath, diff_lines: diffLines, pass, audit_effort: auditEffort, scope, gate_tagged: gateTagged, paths, deleted, stamp_only: stampOnly }) + '\n'); + return EXIT.ok; + } + const out = [`REVIEW_CLASS: ${plan.class} (${plan.reasons.slice(0, 3).join('; ') || 'no signals'})`, reviewDepthLine(plan), `GATE_SCAN: ${gateTagged === null ? 'unavailable' : `tagged=${gateTagged}`} head=${head}`, reviewPlanLine(plan, pass + (plan.next.step === 'full' ? 1 : 0)), `NEXT: ${plan.next.step} (${plan.next.reason})`]; + for (const e of plan.errors) out.push(`DEPTH_ERROR: ${e.code} input=${e.input} detail=${e.detail}; fix: ${e.repair}${e.result ? ` (${e.result})` : ''}`); + for (const d of plan.dispatches) { + const key = d.name.split(' ')[0]; + out.push(`DISPATCH: ${d.role} ${d.name} effort=${d.effort}${d.checklists ? ` checklists=${d.checklists.join(',')}` : ''}${d.reason ? ` reason=${d.reason}` : ''}`, `DIFF_CMD: ${key} ${diffCommand(d)}`); + if (!d.exclude.length) out.push(`DIFF_EXCLUDE: ${key} none`); + } for (const s of plan.skipped) out.push(`SKIP: ${s}`); out.push(`AUDIT_EFFORT: ${auditEffort} (coverage, plan-completion and docs audits, decision-trail reviewers, watchers; reviewers above stay full)`); process.stdout.write(out.join('\n') + '\n'); diff --git a/bin/gstack-ship-receipt b/bin/gstack-ship-receipt index d3e2c342b..6d50425b1 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, exceptionProblems, 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,11 @@ 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: the depth HEAD needs, recomputed from the committed range (--head must be HEAD; a dirty checkout or an + unreadable input records full). --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 +159,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 +194,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 +237,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; } @@ -573,7 +599,19 @@ 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 = {}; + const isAncestor = (commit: string) => !ancestor || spawnSync('git', ['merge-base', '--is-ancestor', commit, head], { cwd: repoRoot, timeout: 30_000 }).status === 0; + const problems = exceptionProblems(body, receipt, isAncestor); + if (problems.length) return refused('RECEIPT_CARRY_REFUSED', `the receipt's owner exception does not stand: ${problems.join('; ')}; run /ship on ${head.slice(0, 12)}`); + if (receipt.review_depth || receipt.gstack_version) { + if (!receipt.review_depth) return refused('RECEIPT_CARRY_REFUSED', `the receipt records gstack_version but no review_depth, so the queue would refuse it; run /ship on ${head.slice(0, 12)}`); + const checkedOut = git(['rev-parse', 'HEAD'], repoRoot)?.toLowerCase(); + if (checkedOut !== head) return refused('RECEIPT_CARRY_REFUSED', `--head ${head.slice(0, 12)} is not the checked-out HEAD ${(checkedOut ?? 'none').slice(0, 12)}; the gate scan reads the checkout, so check out ${head.slice(0, 12)} and carry there`); + 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 149abc89b..bafc25869 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -128,26 +128,75 @@ applies no edits is its exit too. ## Review depth by diff class -`gstack-review-plan --base ` classifies the diff and prints the reviewers -the next step dispatches (`REVIEW_CLASS:`, `REVIEW_PLAN:`, one `DISPATCH:` per -reviewer with its effort, one `SKIP:` per reviewer left out, `--json`). A mixed -diff takes its most restrictive class; a `gstack-diff-scope` error fails closed -to code, and code with the gate scan unavailable fails closed to gate. +`gstack-review-plan --base ` classifies the diff, picks a **depth** and prints +the reviewers the next step dispatches: `REVIEW_CLASS:`, `REVIEW_DEPTH: +runtime_lines= rules= reasons=<…> codes=<…>`, `GATE_SCAN: tagged=|unavailable +head=`, one `DEPTH_ERROR:` per input that failed closed, one `DISPATCH:` per reviewer +with its effort and a shell-quoted `DIFF_CMD:` (plus `DIFF_EXCLUDE: none` when it +reads everything), one `SKIP:` per reviewer left out, and `--json` with the same fields. +A plan that fails closed to full exits 0; a diff git cannot list exits 1. -| Class | What it is | Pass 1 dispatches | +**Classes** are unchanged. A mixed diff takes its most restrictive class: **docs** +(markdown no agent or gate reads), **tests** (test files and fixtures), **code** +(everything else, including every `SKILL.md` and template, markdown in a skill +directory, `*/sections/**`, `scripts/resolvers/**`, rule docs, checklists and bins) and +**gate** (code plus auth or migration scope, a hunk the gate scan tags, a deleted code +or test path, CI workflows, credential, state-root and the paths below). + +**Runtime lines** are the changed lines of code-class paths, minus outputs the base +commit's `.gstack/generated.json` registers. Docs, tests and release metadata are never +runtime; a `.tmpl` edit plus its regenerated `SKILL.md` counts the template once. + +| Depth | When | Pass 1 dispatches | |---|---|---| -| **docs** | markdown no agent or gate reads: README, CHANGELOG, TODOS, how-to and explanation docs | the parent's core checklist only | -| **tests** | test files and fixtures only | core plus the testing specialist | -| **code** | everything else, including every `SKILL.md` and template, any markdown in a skill directory, `*/sections/**`, `scripts/resolvers/**`, `docs/*-policy.md`, `docs/captain.md`, `docs/unattended.md`, `docs/pregate.md`, checklists, specialists and bins | specialists by scope (as before), the native adversarial pass, the outside voice; the Red Team past 200 lines or after a CRITICAL | -| **gate** | code plus auth or migration scope, a hunk the gate scan tags (an RH rule: a relaxed threshold, a skipped test), a deleted code file, CI workflows, credential or state-root paths | code's set, plus the Red Team always, at full effort, and no hit-rate gating | +| **light**, runtime 0 | docs and tests only, or only registered outputs | the core checklist only; the outside voice when the diff publishes numbers | +| **light**, runtime 1–50 | `SMALL_DIFF_LINES` or fewer runtime lines | core, one **batched** specialist (testing, maintainability and security checklists, the specialists scope selects and the Red Team prompt in one context), the native adversarial pass (kept: backtest inconclusive, GSTA-147 D5) and the outside voice on numbers | +| **standard** | more than 50 runtime lines | the code set with every threshold (specialists at 50, simplification at 100, Red Team at 200) read on runtime lines; specialists leave docs and registered outputs out of their diff, and every specialist but Testing leaves tests out too; adversarial and outside voice read everything | +| **full** | gate class, or any classifier error | the gate set, unchanged: every reviewer on the whole diff, the Red Team always, no hit-rate gating | -**Numbers keep the outside voice.** On a docs or tests diff the outside voice is -skipped unless the diff publishes numeric claims: `gstack-review-plan` scans the -added lines of every path for percentages, money, multipliers, durations, token -or credit counts and score, rate or speed claims (`NUMBERS: yes`), and a -claim always dispatches it. The claims' fingerprint (`fingerprint=`) is passed -back as `--reviewed-numbers`; when a later fix changes a claim, only the -outside voice reruns, on the numbers. +A fixed blocking finding, a CRITICAL or an open blocker raises a light diff to +standard for the next step (`step=standard`): its specialists read at least 50 lines +and the Red Team runs, so a raise never dispatches less than light did. A fix-check +is the same at every depth. + +**Fail closed.** Each of these makes the depth full and is listed in `errors[]` with +the input it could not read, a sanitized cause and the repair: a `gstack-diff-scope` +error, a gate scan that is unavailable, a base registry that cannot be read or is +invalid (`REGEN_REGISTRY_INVALID` or `REGEN_CONTAINMENT` in `codes=`), a numstat git +could not produce, and a binary code-class path (it is never counted as 0 lines). + +**The registry rule.** Only the base commit's registry counts (the merge base's +`origin/` commit for live /ship and the current base ref's commit for the queue: `origin/` in `ready`, `/` in `train build`), and only +entries with a `command`, which pregate's regen check proves for that tree. An entry +added on the branch never excludes anything; `command: null` entries, vendored code +and lockfiles stay runtime. To adopt it, commit an entry like this on the base branch: + +```json +{ + "$schema": "https://github.com/garrytan/gstack/docs/pregate.md#generated-v1", + "entries": [ + { "id": "skill-docs", "command": "bun run gen:skill-docs --host all", "outputs": ["*/SKILL.md", "*/sections/*.md"], "inputs": ["*/SKILL.md.tmpl", "*/sections/*.tmpl"] } + ] +} +``` + +**Gate paths for the machinery itself.** An edit to `lib/review-depth.ts`, +`lib/review-collect.ts`, `bin/gstack-review-plan`, `lib/review-scope.ts`, +`lib/ship-queue*.ts`, `lib/regen.ts`, `.gstack/generated.json`, +`ship/sections/review-army.md.tmpl`, `ship/sections/adversarial.md.tmpl` or +`scripts/resolvers/review-army.ts` is gate class: changing how much review runs gets +the most review. `rules=` (`DEPTH_RULES`) hashes the deciding constants and patterns, +not file bytes, so a comment edit keeps it. + +**`--depth-floor standard|full`** (a /ship prompt flag, passed to every plan run) +raises the depth and never lowers it. + +**Numbers keep the outside voice.** At light depth the outside voice runs only when +the diff publishes numeric claims: `gstack-review-plan` scans the added lines of +prose and data files for percentages, money, multipliers, durations, token or credit +counts and score, rate or speed claims (`NUMBERS: yes`). The claims' fingerprint +(`fingerprint=`) is passed back as `--reviewed-numbers`; when a later fix changes a +claim, only the outside voice reruns, on the numbers. It always reads the whole diff. **The adversarial and outside voices run once per invocation** (Step 11, on the final tree), as the last full-pass plan lists them; after they ran, decision 2 @@ -166,6 +215,15 @@ default. `gstack-config set review_effort_tiers off` (or `--no-tiers`) runs the audits and watchers at the host default too ([model-policy.md](model-policy.md#reasoning-effort)). +### Behavior changes + +- A tests-only diff (runtime 0) no longer dispatches the Testing specialist: the + core checklist and the gate scan review it. +- Light no longer runs a separate Red Team: the batched specialist carries its + prompt. At runtime 0 light runs no adversarial pass either. +- Standard reads thresholds on runtime lines, not the whole diff, so a test-heavy + diff with few runtime lines dispatches fewer specialists. + ## What the receipt records `gstack-review-scope summary --since ` reads this invocation's @@ -190,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 @@ -268,9 +333,10 @@ trailers, appended }`. ## Where this is enforced -- `scripts/resolvers/review-army.ts`: the floor and `NO FINDINGS` in every specialist and Red Team prompt (rendered into `review/sections/review-army.md` and `ship/sections/review-army.md`), and the lead judgment's "On record" line that turns a [ruled class](#rulings-below-the-floor) into a note; in `/ship` every specialist and Red Team prompt it renders carries its diff base as a literal sha the parent writes in (`DIFF_BASE=`: the merge base on pass 1, `RANGE_BASE` from pass 2 on), because a reviewer is its own process and never sees the parent's shell variables; the fan-out batches above. -- `ship/sections/review-army.md` Step 9: passes, `RANGE_BASE`, classify, defer, render, the cap check before any edit, the Step 11 late-defer row, scratch files under `.gstack/tmp/`, the cap decisions; `ship/sections/adversarial.md` Step 11: the outside voice reads the same range and scopes queued fixes the same way. -- `lib/review-depth.ts`, `bin/gstack-review-plan`: the diff class, the transition table, the numeric-claim scan and each role's effort tier (`test/review-depth.test.ts`, `test/review-plan-cli.test.ts`). +- `scripts/resolvers/review-army.ts`: the floor and `NO FINDINGS` in every specialist and Red Team prompt (rendered into `review/sections/review-army.md` and `ship/sections/review-army.md`), and the lead judgment's "On record" line that turns a [ruled class](#rulings-below-the-floor) into a note; in `/ship` every specialist and Red Team prompt it renders carries its diff base as a literal sha the parent writes in (`DIFF_BASE=`: the merge base on pass 1, `RANGE_BASE` from pass 2 on) beside its plan's `DIFF_CMD` excludes, because a reviewer is its own process and never sees the parent's shell variables; the fan-out batches above. +- `ship/sections/review-army.md` Step 9: the plan's `REVIEW_DEPTH`, `GATE_SCAN` and `--depth-floor` on every run, the depth fields of the review-log row, passes, `RANGE_BASE`, classify, defer, render, the cap check before any edit, the Step 11 late-defer row, scratch files under `.gstack/tmp/`, the cap decisions; `ship/sections/adversarial.md` Step 11: the outside voice reads the same range and scopes queued fixes the same way. +- `lib/review-depth.ts`, `bin/gstack-review-plan`: the diff class, the depth (`depthFor`, `DEPTH_RULES`, `depthOfCommitInput`), each dispatch's `exclude` pathspecs and `DIFF_CMD`, the transition table, the numeric-claim scan and each role's effort tier (`test/review-depth.test.ts`, `test/review-plan-cli.test.ts`). +- `lib/review-collect.ts`: `collectCommit` (git objects only, from any checkout) and `collectLive` (the working tree), per-path lines and the base commit's registry (`test/review-collect.test.ts`). - `lib/review-scope.ts`, `bin/gstack-review-scope`: `classify`, `defer`, `render`, `summary` (with `fix_checks`, `dispatches`, `est_input_tokens`); refusals are `REVIEW_SCOPE_INPUT` and `REVIEW_SCOPE_RANGE` ([troubleshooting.md](troubleshooting.md#review-scope-input)). - `lib/ship-policy.ts`: the `review_scope` key and its `ship: review_scope …` consequence line. - `review/checklist.md`: `prereg-with-results` under Gate Integrity and `test-keyboard-count-loop` under Pass 2; `bin/gstack-coauthor-trailers` for the contributor-credit trailers of a port. diff --git a/docs/ship-policy.md b/docs/ship-policy.md index 61e0059ef..bd848b5a2 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -221,8 +221,11 @@ It refuses (`QUEUE_NOT_READY`) when the working tree has tracked edits, HEAD is not the PR's pushed head, CI at that exact head is not green, or the pregate result is not a current pass, and (`QUEUE_RECEIPT_MISSING`, `QUEUE_RECEIPT_STALE`) when the PR body carries no /ship receipt for that -head: /ship runs before every READY ([details](#ship-before-ready)). Send the -block as printed. +head: /ship runs before every READY ([details](#ship-before-ready)). It also +refuses `QUEUE_RECEIPT_DEPTH_RAISED` when the receipt's review depth is below +what the head needs and `QUEUE_RECEIPT_EXCEPTION_INVALID` when an owner +exception the receipt records does not stand ([the ship receipt](#ship-receipt)). +Send the block as printed. **2. Lane behind another PR: gate ahead.** Gate the tree the queue will have once the predecessor lands, while it is still open: @@ -370,12 +373,17 @@ slot. The queue enforces this at every step from the PR body alone: - `ready` refuses with `QUEUE_RECEIPT_MISSING` when the body has no valid receipt and with `QUEUE_RECEIPT_STALE` when the receipt's `head` is not the head being READY'd (or its `pr` is another PR); the informational - `SHIP_RECEIPT:` line still prints. There is no flag around it. + `SHIP_RECEIPT:` line still prints. It refuses `QUEUE_RECEIPT_DEPTH_RAISED` + (with a `REVIEW_DEPTH:` line) when the receipt records a review depth below + what the head needs, and `QUEUE_RECEIPT_EXCEPTION_INVALID` when an owner + exception it records does not stand ([the ship receipt](#ship-receipt)). + There is no flag around it. - `train build` refuses a pin whose PR has no current receipt with the same codes, naming the PR, before anything is written, pushed or dispatched, `--dry-run` included. -- `verify --train` refuses the slot with the same codes before printing - `MERGE_CONDITION`. +- `verify --train` refuses the slot with `QUEUE_RECEIPT_MISSING` or + `QUEUE_RECEIPT_STALE` before printing `MERGE_CONDITION`; it does not + recompute the review depth. **The stamp keeps the pin's receipt.** The slot's stamp commit changes only stamp files (and, from slot 2 on, merges main into the pin), so no /ship runs @@ -478,6 +486,9 @@ its record. Refusals, all before any spend: `QUEUE_TRAIN_UNTRUSTED` (a fork head), `QUEUE_TRAIN_MOVED` (a head moved from its pin), `QUEUE_RECEIPT_MISSING` and `QUEUE_RECEIPT_STALE` (the PR body has no /ship receipt for the pin; see [/ship before READY](#ship-before-ready)), +`QUEUE_RECEIPT_DEPTH_RAISED` and `QUEUE_RECEIPT_EXCEPTION_INVALID` (the +receipt's review depth is below what the pin needs, or an owner exception it +records does not stand; see [the ship receipt](#ship-receipt)), `QUEUE_NOT_READY` (no READY receipt at that head), `QUEUE_TRAIN_CONFLICT` (the printed `QUEUE_TRAIN_CONFLICT: train stopped at # (conflicts: ) after merging #,#; nothing dispatched, no branch pushed; fix: …` line names @@ -777,8 +788,9 @@ 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`, -`head_tree`, `product_patch_id`, `pregate`, `issues`, `approval`, `docs`, -`review_loop`, `carry`), validated +`head_tree`, `product_patch_id`, `gstack_version`, `review_depth`, `pregate`, +`issues`, `approval`, `docs`, `review_loop`, `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. @@ -813,6 +825,46 @@ 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/` +(`gstack-diff-scope --head ` for the scope signals), 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; tracked edits in the checkout make the scan unavailable, +which records `full`. `gstack-ship-queue ready` and `train build` recompute +the depth the head needs from the same range and scope and print `REVIEW_DEPTH: receipt= required= +runtime_lines= class= reasons=<…> inputs=base@,registry@,gate_scan@`; +a recorded gate scan counts only when its `head` is the full bound commit, +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), when `--head` is not the +checked-out HEAD, when the receipt records `gstack_version` without +`review_depth`, or when an owner exception no longer stands, each as +[`RECEIPT_CARRY_REFUSED`](troubleshooting.md#receipt-carry-refused). + +**`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, the body must still +carry its line, and the fix-check head must be the bound head or its ancestor +(after a carry across rewritten history the old fix-check commit is off the +branch, so that check is skipped), 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 64688a386..e531ff7b8 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -2900,10 +2900,12 @@ for an older PR, write it once by hand with the same command. ### `RECEIPT_INVALID` **Meaning.** The block exists but does not validate against the `ship-receipt` -schema (`gstack-artifact schema ship-receipt`). +schema (`gstack-artifact schema ship-receipt`). `gstack-ship-receipt write` +also refuses with this code, and writes nothing, when `--head` is not the +checked-out HEAD: the receipt binds the tree the review read. **Fix.** Regenerate it with `gstack-ship-receipt write`; never hand-edit the -block. +block. For the `--head` refusal, check out that commit and write there. ### `EXECUTION_UNKNOWN` @@ -4434,6 +4436,39 @@ 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 the base ref, `origin/` in `ready` +and `/` in `train build`, 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), its `fix_check.head` is neither the bound head nor an +ancestor of it (not checked after a carry across rewritten history), 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` @@ -4699,13 +4734,34 @@ untracked changes (a carry certifies a commit, never a tree that differs from it), the receipt's head left HEAD's history and the receipt records no tree to compare (a receipt written before `head_tree`, whose head is not in this clone), or git could not compare the two product diffs or read the working -tree's status. The receipt in the body is unchanged and still names the old -head, so `gstack-ship-queue ready` refuses `QUEUE_RECEIPT_STALE`. +tree's status. It also refuses when an owner exception the receipt records +no longer stands, and, for a receipt from a gstack that records review +depth, when HEAD needs a deeper review than the receipt records, `--head` is +not the checked-out HEAD, or the receipt records `gstack_version` without +`review_depth` ([ship-policy.md](ship-policy.md#ship-receipt)). The receipt in +the body is unchanged and still names the old head, so `gstack-ship-queue +ready` refuses `QUEUE_RECEIPT_STALE`. **Fix.** Commit or stash every working-tree change, then rerun `gstack-ship-receipt carry --pr ` at the new head. When the history is already rewritten and the receipt predates `head_tree`, run /ship on the new -head; next time merge the base in instead of rebasing. +head; next time merge the base in instead of rebasing. For a depth or +exception refusal, run /ship on the new head (or check out `--head` and carry +there). + + +### `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 a42338611..c82ba790c 100644 --- a/lib/headless-artifacts.ts +++ b/lib/headless-artifacts.ts @@ -104,6 +104,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 the reviewed head needs, recomputed at write (lib/ship-receipt-depth.ts receiptDepthAtHead). */ + 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']; @@ -224,7 +229,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 51eccaa22..ee4004adb 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/review-collect.ts b/lib/review-collect.ts new file mode 100644 index 000000000..3846df0f3 --- /dev/null +++ b/lib/review-collect.ts @@ -0,0 +1,137 @@ +/** + * review-collect — the git reads behind a review depth (docs/review-policy.md#review-depth), + * kept out of lib/review-depth.ts so the policy stays pure. collectCommit reads git + * objects only (a range between two commits, from any checkout, dirty or not); + * collectLive reads the working tree as /ship reviews it (committed, tracked + * uncommitted edits and untracked files). Both return a ClassInput with per-path + * line counts, the base commit's generated-output registry and collector errors; + * the caller adds the gate-scan result. + */ +import { spawnSync } from 'node:child_process'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { GENERATED_PATH, parseRegistry } from './regen'; +import { RESULT_CODES } from './result-codes'; +import type { ClassInput, DepthError, PathLines, RegistryInfo } from './review-depth'; + +export interface Collected extends ClassInput { + perPath: PathLines[]; + registry: RegistryInfo; + errors: DepthError[]; + deleted: string[]; + stampOnly: string[]; + /** Added lines per path, for scanNumbers. */ + added: Record; + diffLines: number; + mergeBase: string | null; + untracked: string[]; +} + +function git(root: string, args: string[]): { ok: boolean; out: string; err: string } { + const r = spawnSync('git', ['-c', 'core.quotePath=false', ...args], { cwd: root, encoding: 'utf8', timeout: 60_000, maxBuffer: 256 * 1024 * 1024 }); + return { ok: r.status === 0, out: r.stdout ?? '', err: (r.stderr ?? '').trim() }; +} +const scratch = (p: string) => p.startsWith('.gstack/tmp/'); +const oneLine = (s: string) => s.replace(/[\0-\x1f]+/g, ' ').trim().slice(0, 200); +const unavailable = (input: string, detail: string): RegistryInfo => ({ sha: null, outputs: [], error: { code: 'registry_unavailable', detail: oneLine(detail), input, repair: 'fetch the base branch (git fetch origin ) so its commit and registry resolve' } }); + +/** The registry as committed at `sha`: outputs of entries with a command (a `command: null` entry never excludes); a missing file is an empty registry. */ +export function loadRegistryAt(root: string, sha: string): RegistryInfo { + const commit = git(root, ['rev-parse', '--verify', '--quiet', `${sha}^{commit}`]); + if (!commit.ok) return unavailable(`${sha}:${GENERATED_PATH}`, `${sha} is not a commit in this repository`); + const resolved = commit.out.trim(); + const input = `${resolved.slice(0, 12)}:${GENERATED_PATH}`; + if (!git(root, ['cat-file', '-e', `${resolved}:${GENERATED_PATH}`]).ok) return { sha: resolved, outputs: [] }; + const show = git(root, ['show', `${resolved}:${GENERATED_PATH}`]); + if (!show.ok) return unavailable(input, show.err || 'git show failed'); + const { registry, errors } = parseRegistry(show.out); + const e = errors[0]; + if (e) return { sha: resolved, outputs: [], error: { code: 'registry_invalid', detail: oneLine(`${e.path || GENERATED_PATH}: ${e.message}`), input, repair: RESULT_CODES[e.code].fix, result: e.code } }; + return { sha: resolved, outputs: registry.entries.filter(x => x.command !== null).flatMap(x => x.outputs) }; +} + +/** Numstat, deletions and the -U0 patch for `range` (`[mergeBase, head]` or `[mergeBase]` for the working tree). */ +function readDiff(root: string, range: string[], errors: DepthError[]): Pick { + const numstat = git(root, ['diff', '--numstat', '-z', '--no-renames', ...range]); + const status = git(root, ['diff', '--name-status', '-z', '--no-renames', '--diff-filter=D', ...range]); + const patch = git(root, ['diff', '-U0', '--no-color', '--no-ext-diff', '--no-renames', ...range]); + const empty = { perPath: [], deleted: [], stampOnly: [], added: {} }; + for (const [r, what] of [[numstat, '--numstat'], [status, '--name-status'], [patch, '-U0']] as const) { + if (!r.ok) { errors.push({ code: 'numstat_failed', detail: oneLine(r.err || 'git diff failed'), input: `git diff ${what} ${range.join(' ')}`, repair: 'check that both commits exist locally (git fetch origin ) and rerun' }); return empty; } + } + const perPath: PathLines[] = []; + for (const rec of numstat.out.split('\0').filter(Boolean)) { + const [a, d, ...rest] = rec.split('\t'); + const p = rest.join('\t'); + if (!scratch(p)) perPath.push({ path: p, lines: (Number(a) || 0) + (Number(d) || 0), binary: a === '-' && d === '-' }); + } + const statusParts = status.out.split('\0'); + const deleted = statusParts.flatMap((s, k) => (s === 'D' && statusParts[k + 1] ? [statusParts[k + 1]!] : [])).filter(p => !scratch(p)); + const added: Record = {}; + const changed: Record = {}; + let cur = ''; + let minus = ''; + let inHunk = false; + for (const line of patch.out.split('\n')) { + if (line.startsWith('diff --git ')) { inHunk = false; cur = ''; minus = ''; continue; } + if (!inHunk) { + if (line.startsWith('--- ')) minus = line === '--- /dev/null' ? '' : line.slice(6).replace(/\t$/, ''); + else if (line.startsWith('+++ ')) cur = line === '+++ /dev/null' ? minus : line.slice(6).replace(/\t$/, ''); + else if (line.startsWith('@@')) inHunk = true; + continue; + } + if (!cur || scratch(cur) || line.startsWith('@@')) continue; + if (line[0] === '+') (added[cur] ??= []).push(line.slice(1)); + if (line[0] === '+' || line[0] === '-') (changed[cur] ??= []).push(line); + } + const stampOnly = perPath.map(e => e.path).filter(p => p.endsWith('package.json') && !!changed[p]?.length && changed[p]!.every(l => /^[+-]\s*"version"\s*:\s*"[^"]*"\s*,?\s*$/.test(l))); + return { perPath, deleted, stampOnly, added }; +} + +function result(diff: ReturnType, extra: Pick): Collected { + const paths = diff.perPath.map(e => e.path).sort(); + return { ...diff, ...extra, paths, gateTagged: null, diffLines: diff.perPath.reduce((n, e) => n + e.lines, 0) }; +} + +/** The range `baseSha`...`headSha` from git objects alone; the registry is the base commit's, never the head's. */ +export function collectCommit(root: string, baseSha: string, headSha: string): Collected { + const errors: DepthError[] = []; + const registry = loadRegistryAt(root, baseSha); + const mb = git(root, ['merge-base', baseSha, headSha]); + if (!mb.ok) { + errors.push({ code: 'numstat_failed', detail: oneLine(mb.err || 'no merge base'), input: `git merge-base ${baseSha} ${headSha}`, repair: 'check that both commits exist locally (git fetch origin ) and rerun' }); + return result({ perPath: [], deleted: [], stampOnly: [], added: {} }, { root: [], registry, errors, mergeBase: null, untracked: [] }); + } + const mergeBase = mb.out.trim(); + const diff = readDiff(root, [mergeBase, headSha], errors); + const tree = git(root, ['ls-tree', '-r', '--name-only', '-z', headSha]); + const skillDirs = [...new Set(tree.out.split('\0').flatMap(p => /^([^/]+)\/SKILL\.md(\.tmpl)?$/.exec(p)?.[1] ?? []))].sort(); + return result(diff, { root: skillDirs, registry, errors, mergeBase, untracked: [] }); +} + +/** The working tree against the merge base with origin/ (or ), untracked files counted per path; the registry is the base ref's commit. */ +export function collectLive(root: string, base: string): Collected { + const errors: DepthError[] = []; + const ref = git(root, ['rev-parse', '--verify', '-q', `origin/${base}^{commit}`]).ok ? `origin/${base}` : base; + const registry = loadRegistryAt(root, ref); + const mb = git(root, ['merge-base', ref, 'HEAD']); + if (!mb.ok) { + errors.push({ code: 'numstat_failed', detail: oneLine(mb.err || `no merge base with ${ref}`), input: `git merge-base ${ref} HEAD`, repair: `fetch the base first (git fetch origin ${base})` }); + return result({ perPath: [], deleted: [], stampOnly: [], added: {} }, { root, registry, errors, mergeBase: null, untracked: [] }); + } + const mergeBase = mb.out.trim(); + const diff = readDiff(root, [mergeBase], errors); + const others = git(root, ['ls-files', '--others', '--exclude-standard', '-z']); + if (!others.ok) errors.push({ code: 'numstat_failed', detail: oneLine(others.err || 'git ls-files failed'), input: 'git ls-files --others --exclude-standard', repair: 'run the command by hand and fix what git reports' }); + const untracked = others.out.split('\0').filter(p => p && !scratch(p)).sort(); + for (const p of untracked) { + let buf: Buffer; + try { buf = fs.readFileSync(path.join(root, p)); } catch { diff.perPath.push({ path: p, lines: 0, binary: true }); continue; } + const binary = buf.subarray(0, 8000).includes(0); + const text = binary ? '' : buf.toString('utf8'); + const lines = text ? text.split('\n').length - (text.endsWith('\n') ? 1 : 0) : 0; + diff.perPath.push({ path: p, lines, binary }); + if (!binary) diff.added[p] = text.split('\n'); + } + return result(diff, { root, registry, errors, mergeBase, untracked }); +} diff --git a/lib/review-depth.ts b/lib/review-depth.ts index f7331dbba..0cae65d45 100644 --- a/lib/review-depth.ts +++ b/lib/review-depth.ts @@ -1,17 +1,22 @@ /** * review-depth — the one policy owner for how much review a /ship run dispatches - * (docs/review-policy.md#review-depth). A pure function from the changed paths, - * gstack-diff-scope flags, the diff size, the gate scan and the pass history to a - * diff class, the next step of the loop and the reviewers that step dispatches, - * each with its reasoning-effort tier. bin/gstack-review-plan prints it; the + * (docs/review-policy.md#review-depth). A pure function from the changed paths and + * their line counts, gstack-diff-scope flags, the base registry's generated outputs, + * the gate scan and the pass history to a diff class, a review depth (light, + * standard, full), the next step of the loop and the reviewers that step + * dispatches, each with its effort tier and the pathspecs it leaves out. Git reads + * live in lib/review-collect.ts; bin/gstack-review-plan prints the plan; the * review-army resolver and the skills read the same definitions. */ import { createHash } from 'node:crypto'; import * as fs from 'node:fs'; import * as path from 'node:path'; +import type { ResultCodeName } from './result-codes'; +import { matchesAny } from './ship-policy'; export type DiffClass = 'docs' | 'tests' | 'code' | 'gate'; export type PathClass = 'docs' | 'tests' | 'code'; +export type Depth = 'light' | 'standard' | 'full'; export type Effort = 'light' | 'full' | 'host'; export type Role = 'specialist' | 'red-team' | 'adversarial-native' | 'outside-voice' | 'fix-check'; export type Step = 'full' | 'fix-check' | 'done' | 'stop'; @@ -22,7 +27,12 @@ export const FULL_PASS_CAP = 2; export const SMALL_DIFF_LINES = 50; export const RED_TEAM_LINES = 200; export const LARGE_DIFF_LINES = 100; +/** The checklists light's batched specialist always reads, whatever the scope. */ +export const LIGHT_FIXED: readonly Specialist[] = ['testing', 'maintainability', 'security']; +/** Why light with runtime above 0 keeps the native adversarial pass as its own dispatch. */ +export const LIGHT_ADVERSARIAL_REASON = 'backtest inconclusive (GSTA-147 D5)'; const CLASS_RANK: Record = { docs: 0, tests: 1, code: 2, gate: 3 }; +const DEPTH_RANK: Record = { light: 0, standard: 1, full: 2 }; const AGENT_PROSE = [/(^|\/)SKILL\.md(\.tmpl)?$/, /(^|\/)sections\//, /^scripts\/resolvers\//, /^docs\/[^/]*-policy\.md$/, /^docs\/(captain|unattended|pregate)\.md$/, /(^|\/)DRIVER\.md(\.tmpl)?$/, /(^|\/)templates\//, /(^|\/)(AGENTS|CLAUDE|ETHOS)\.md$/, /(^|\/)specialists\//, /(^|\/)checklist\.md$/, /^\.gstack\//]; /** gstack-diff-scope's TEST_PATH_RE (the test-bootstrap rule), plus fixture directories. */ @@ -30,19 +40,41 @@ const TEST_PATH = [/(^|\/)(tests?|spec|__tests__)\/|(^|\/)tests?\.py$|(^|\/)test const DOC_PATH = /\.(md|mdx|markdown|rst|txt|adoc)$/i; /** Release metadata a version stamp writes: never a reason for review depth on its own. */ const RELEASE_METADATA = [/^CHANGELOG\.md$/, /^VERSION$/, /^agents-digest\//]; -const GATE_PATH = [/(^|\/)\.github\/workflows\//, /(^|\/)(secrets?|credentials?|keychain|token[s]?)([._/-]|$)/i, /state-root/, /(^|\/)(pregate|gate|ship-receipt|ship-policy)[^/]*\.(ts|sh|json)$/, /(^|\/)bin\/gstack-(pregate|gate|ship-receipt|verify-gate|ci-gate)$/]; +/** Gate paths, the review-depth machinery included: an edit to how much review runs gets the most review. */ +const GATE_PATH = [ + /(^|\/)\.github\/workflows\//, /(^|\/)(secrets?|credentials?|keychain|token[s]?)([._/-]|$)/i, /state-root/, /(^|\/)(pregate|gate|ship-receipt|ship-policy)[^/]*\.(ts|sh|json)$/, /(^|\/)bin\/gstack-(pregate|gate|ship-receipt|verify-gate|ci-gate)$/, + /^lib\/review-(depth|collect|scope)\.ts$/, /^bin\/gstack-review-plan$/, /^lib\/ship-queue[^/]*\.ts$/, /^lib\/regen\.ts$/, /^\.gstack\/generated\.json$/, /^ship\/sections\/(review-army|adversarial)\.md\.tmpl$/, /^scripts\/resolvers\/review-army\.ts$/, +]; -/** One changed path's class. Markdown an agent or a gate reads (skills, sections, resolvers, policy and rule docs, templates, any markdown in a skill directory under `root`) is code: it is gstack's product. */ -export function classifyPath(p: string, root?: string): PathClass { +/** A hash of the constants and patterns that decide a depth, not of this file's bytes: a comment edit keeps it, a threshold edit changes it. */ +export const DEPTH_RULES = createHash('sha256').update(JSON.stringify({ + SMALL_DIFF_LINES, RED_TEAM_LINES, LARGE_DIFF_LINES, FULL_PASS_CAP, LIGHT_FIXED, + patterns: Object.entries({ GATE_PATH, AGENT_PROSE, TEST_PATH, RELEASE_METADATA, DOC_PATH: [DOC_PATH] }).map(([k, v]) => [k, v.map(r => `/${r.source}/${r.flags}`)]), +})).digest('hex').slice(0, 16); + +/** + * One changed path's class. Markdown an agent or a gate reads (skills, sections, resolvers, policy and rule docs, + * templates, any markdown in a skill directory) is code: it is gstack's product. `root` is the repository root + * (skill directories probed on disk) or the top-level skill directory names read from a commit. + */ +export function classifyPath(p: string, root?: string | readonly string[]): PathClass { if (RELEASE_METADATA.some(r => r.test(p))) return 'docs'; if (AGENT_PROSE.some(r => r.test(p))) return 'code'; if (TEST_PATH.some(r => r.test(p))) return 'tests'; if (!DOC_PATH.test(p)) return 'code'; const top = p.includes('/') ? p.split('/')[0]! : ''; - if (root && top && ['SKILL.md', 'SKILL.md.tmpl'].some(f => fs.existsSync(path.join(root, top, f)))) return 'code'; - return 'docs'; + if (!top || !root) return 'docs'; + if (typeof root === 'string') return ['SKILL.md', 'SKILL.md.tmpl'].some(f => fs.existsSync(path.join(root, top, f))) ? 'code' : 'docs'; + return root.includes(top) ? 'code' : 'docs'; } +export type DepthErrorCode = 'scope_error' | 'gate_scan_unavailable' | 'registry_unavailable' | 'registry_invalid' | 'numstat_failed' | 'binary_code_path'; +/** Why a depth failed closed to full: the input it could not read, a sanitized cause and the repair; `result` is the result code behind it when one exists. */ +export interface DepthError { code: DepthErrorCode; detail: string; input: string; repair: string; result?: ResultCodeName } +export interface PathLines { path: string; lines: number; binary: boolean } +/** The base commit's `.gstack/generated.json`: outputs of entries with a command only; `error` when it could not be read or validated. */ +export interface RegistryInfo { sha: string | null; outputs: readonly string[]; error?: DepthError } + export interface ClassInput { paths: readonly string[]; /** gstack-diff-scope output (SCOPE_* keys). */ @@ -53,23 +85,38 @@ export interface ClassInput { deleted?: readonly string[]; /** Paths whose whole change is a version stamp (package.json's version key): classified as docs. */ stampOnly?: readonly string[]; - /** The repository root: markdown inside a skill directory there is code. */ - root?: string; + /** The repository root, or the skill directory names at the reviewed commit: markdown inside a skill directory is code. */ + root?: string | readonly string[]; + /** Changed lines per path; without it a code diff counts every line as runtime. */ + perPath?: readonly PathLines[]; + registry?: RegistryInfo; + /** Collector failures (a diff git could not list): each fails the depth closed. */ + errors?: readonly DepthError[]; } -export interface ClassResult { class: DiffClass; reasons: string[] } +export interface ClassResult { class: DiffClass; reasons: string[]; errors: DepthError[]; runtimeLines: number | null } -/** The diff's class: the most restrictive of its paths, raised to gate by any gate signal; errors fail closed. */ +const sanitize = (s: string) => s.replace(/[\0-\x1f]+/g, ' ').slice(0, 200); +const isOutput = (i: ClassInput, p: string) => !!i.registry?.outputs.length && matchesAny(i.registry.outputs, p); +const pathClass = (i: ClassInput, p: string): PathClass => ((i.stampOnly ?? []).includes(p) ? 'docs' : classifyPath(p, i.root)); + +/** The diff's class, raised to gate by any gate signal; `errors` lists every input that fails the depth closed; `runtimeLines` counts code-class lines outside the base registry's outputs. */ export function classifyDiff(i: ClassInput): ClassResult { const reasons: string[] = []; + const errors: DepthError[] = [...(i.errors ?? [])]; let cls: DiffClass = 'docs'; const raise = (c: DiffClass, why: string) => { if (CLASS_RANK[c] > CLASS_RANK[cls]) cls = c; reasons.push(why); }; if (!i.paths.length) raise('docs', 'no changed paths'); - for (const p of i.paths) { const c = (i.stampOnly ?? []).includes(p) ? 'docs' : classifyPath(p, i.root); if (CLASS_RANK[c] > CLASS_RANK[cls]) raise(c, `${c}: ${p}`); } + for (const p of i.paths) { const c = pathClass(i, p); if (CLASS_RANK[c] > CLASS_RANK[cls]) raise(c, `${c}: ${p}`); } const s = i.scope ?? {}; - if (s.SCOPE_ERROR) raise('code', `scope error: ${s.SCOPE_ERROR}`); + if (s.SCOPE_ERROR) { + raise('code', `scope error: ${s.SCOPE_ERROR}`); + errors.push({ code: 'scope_error', detail: sanitize(s.SCOPE_ERROR), input: 'gstack-diff-scope ', repair: 'run gstack-diff-scope by hand and fix what it reports' }); + } if (CLASS_RANK[cls] >= CLASS_RANK.tests) { - if (i.gateTagged === null) raise('gate', 'gate scan unavailable'); - else if (i.gateTagged > 0) raise('gate', `gate scan tagged ${i.gateTagged} hunk(s)`); + if (i.gateTagged === null) { + raise('gate', 'gate scan unavailable'); + errors.push({ code: 'gate_scan_unavailable', detail: 'gstack-gate-diff reported no GATE_SUMMARY', input: 'gstack-gate-diff ', repair: 'run gstack-gate-diff and read its GATE_ERROR line, or pass the recorded scan as --gate-tagged ' }); + } else if (i.gateTagged > 0) raise('gate', `gate scan tagged ${i.gateTagged} hunk(s)`); const deletedGuarded = (i.deleted ?? []).filter(p => classifyPath(p, i.root) !== 'docs'); if (deletedGuarded.length) raise('gate', `deletes ${deletedGuarded.length} code or test path(s)`); } @@ -77,8 +124,38 @@ export function classifyDiff(i: ClassInput): ClassResult { if (s.SCOPE_AUTH === 'true') raise('gate', 'auth paths'); if (s.SCOPE_MIGRATIONS === 'true') raise('gate', 'migrations'); for (const p of i.paths) if (GATE_PATH.some(r => r.test(p))) { raise('gate', `gate path: ${p}`); break; } + if (i.registry?.error) errors.push(i.registry.error); } - return { class: cls, reasons }; + if (!i.perPath) return { class: cls, reasons, errors, runtimeLines: null }; + let runtimeLines = 0; + for (const e of i.perPath) { + if (pathClass(i, e.path) !== 'code' || isOutput(i, e.path)) continue; + if (e.binary) errors.push({ code: 'binary_code_path', detail: 'git reports a binary change, so its lines cannot be counted', input: e.path, repair: "register it as an output of an entry with a command in the base branch's .gstack/generated.json, or accept the full review" }); + else runtimeLines += e.lines; + } + return { class: cls, reasons, errors, runtimeLines }; +} + +/** Any classifier error or gate class is full; runtime above SMALL_DIFF_LINES is standard; otherwise light. A floor only raises. */ +export function depthFor(o: { cls: DiffClass; runtimeLines: number; errors: readonly DepthError[]; floor?: Depth }): Depth { + const d: Depth = o.errors.length || o.cls === 'gate' ? 'full' : o.runtimeLines > SMALL_DIFF_LINES ? 'standard' : 'light'; + return o.floor && DEPTH_RANK[o.floor] > DEPTH_RANK[d] ? o.floor : d; +} + +export interface DepthResult { depth: Depth; class: DiffClass; runtimeLines: number; reasons: string[]; errors: DepthError[]; codes: ResultCodeName[]; rules: string } + +function decideDepth(c: ClassResult, runtimeLines: number, floor?: Depth): DepthResult { + const unfloored = depthFor({ cls: c.class, runtimeLines, errors: c.errors }); + const reasons = [...c.errors.map(e => `error:${e.code}`), ...(c.class === 'gate' ? ['gate-class'] : []), `runtime=${runtimeLines}${runtimeLines > SMALL_DIFF_LINES ? '>' : '<='}${SMALL_DIFF_LINES}`]; + if (floor && DEPTH_RANK[floor] > DEPTH_RANK[unfloored]) reasons.push(`floor:${floor}`); + const codes = [...new Set(c.errors.flatMap(e => (e.result ? [e.result] : [])))]; + return { depth: depthFor({ cls: c.class, runtimeLines, errors: c.errors, floor }), class: c.class, runtimeLines, reasons, errors: c.errors, codes, rules: DEPTH_RULES }; +} + +/** The depth of a collected range (lib/review-collect.ts) plus a recorded gate-scan result: what the queue recomputes at a head. */ +export function depthOfCommitInput(input: ClassInput & { floor?: Depth }): DepthResult { + const c = classifyDiff(input); + return decideDepth(c, c.runtimeLines ?? 0, input.floor); } // --------------------------------------------------------------------------- @@ -153,34 +230,48 @@ export interface PlanInput extends ClassInput { force?: readonly string[] | 'all'; /** Specialists the hit-rate stats tag GATE_CANDIDATE. */ gated?: readonly string[]; - /** A specialist produced a CRITICAL finding (Red Team activation on code class). */ + /** A specialist produced a CRITICAL finding (Red Team activation on code class; raises light to standard). */ criticalFound?: boolean; /** Step 11's native adversarial pass already ran in this invocation. */ adversarialRan?: boolean; /** The numeric-claim fingerprint the outside voice last reviewed; a change reruns it. */ reviewedNumbers?: string; + /** --depth-floor: raises the depth, never lowers it. */ + floor?: Depth; } -export interface Dispatch { role: Role; name: string; effort: Effort } -export interface ReviewPlan { class: DiffClass; reasons: string[]; next: NextStep; numbers: NumbersResult; dispatches: Dispatch[]; skipped: string[] } +/** One reviewer. `exclude` holds git pathspec argv items the reviewer's diff leaves out; `checklists` names what light's batched specialist reads. */ +export interface Dispatch { role: Role; name: string; effort: Effort; exclude: string[]; checklists?: string[]; reason?: string } +export interface ReviewPlan extends DepthResult { + /** The depth this step runs at: light raised to standard by a fixed blocker, a CRITICAL or an open blocker. */ + stepDepth: Depth; + depthReasons: string[]; + next: NextStep; numbers: NumbersResult; dispatches: Dispatch[]; skipped: string[]; +} + +const scopeWants: Partial) => boolean>> = { + performance: s => s.SCOPE_BACKEND === 'true' || s.SCOPE_FRONTEND === 'true', + 'data-migration': s => s.SCOPE_MIGRATIONS === 'true', + 'api-contract': s => s.SCOPE_API === 'true', + design: s => s.SCOPE_FRONTEND === 'true', + 'state-machines': s => s.SCOPE_STATE_MACHINE === 'true' || s.SCOPE_STATE_MACHINE === 'unknown' || s.SCOPE_STATE_MACHINE === undefined, +}; +const forcedBy = (i: PlanInput) => (n: string) => i.force === 'all' || (Array.isArray(i.force) && i.force.includes(n)); /** The specialists a full pass selects for a class: class sets the eligible set, hit-rate gating applies inside it, gate class ignores gating. */ export function specialistsFor(cls: DiffClass, i: PlanInput, skipped: string[] = []): Specialist[] { const s = i.scope ?? {}; - const forced = (n: string) => i.force === 'all' || (Array.isArray(i.force) && i.force.includes(n)); + const forced = forcedBy(i); if (cls === 'docs') { skipped.push('specialists (docs class)'); return SPECIALISTS.filter(forced); } if (cls === 'tests') { skipped.push('specialists except testing (tests class)'); return SPECIALISTS.filter(n => n === 'testing' || forced(n)); } const want = (n: Specialist): boolean => { if (forced(n)) return true; - if (n === 'state-machines') return s.SCOPE_STATE_MACHINE === 'true' || s.SCOPE_STATE_MACHINE === 'unknown' || s.SCOPE_STATE_MACHINE === undefined; + if (n === 'state-machines') return scopeWants[n]!(s); if (i.diffLines < SMALL_DIFF_LINES) return false; switch (n) { case 'testing': case 'maintainability': return true; case 'security': return s.SCOPE_AUTH === 'true' || (s.SCOPE_BACKEND === 'true' && i.diffLines > LARGE_DIFF_LINES); - case 'performance': return s.SCOPE_BACKEND === 'true' || s.SCOPE_FRONTEND === 'true'; - case 'data-migration': return s.SCOPE_MIGRATIONS === 'true'; - case 'api-contract': return s.SCOPE_API === 'true'; - case 'design': return s.SCOPE_FRONTEND === 'true'; case 'simplification': return i.diffLines > LARGE_DIFF_LINES; + default: return scopeWants[n]!(s); } }; return SPECIALISTS.filter(n => { @@ -190,34 +281,76 @@ export function specialistsFor(cls: DiffClass, i: PlanInput, skipped: string[] = }); } -/** The reviewers the next step dispatches, with each role's effort tier. */ +/** A path as one git exclude pathspec; a glob character or a leading colon makes it literal. */ +const excludeSpec = (p: string) => `:(exclude${/[*?[\]\\]/.test(p) || p.startsWith(':') ? ',literal' : ''})${p}`; +const shellQuote = (s: string) => `'${s.replace(/'/g, "'\\''")}'`; + +/** The diff command a dispatched reviewer runs; `` is the literal sha the parent writes in. */ +export function diffCommand(d: Dispatch): string { + return d.exclude.length ? `git diff -- . ${d.exclude.map(shellQuote).join(' ')}` : 'git diff '; +} + +/** The reviewers the next step dispatches at its depth, with each role's effort tier and excluded pathspecs. */ export function planReview(i: PlanInput): ReviewPlan { - const { class: cls, reasons } = classifyDiff(i); + const c = classifyDiff(i); + const depth = decideDepth(c, c.runtimeLines ?? (CLASS_RANK[c.class] >= CLASS_RANK.code ? i.diffLines : 0), i.floor); + const { class: cls, runtimeLines } = depth; const next = nextStep(i.history); + const h = i.history; + const raise = depth.depth === 'light' && (h.fixedBlocking || h.blockingOpen || i.criticalFound); + const stepDepth: Depth = raise ? 'standard' : depth.depth; + const depthReasons = [...depth.reasons, ...(raise ? [`history:${h.fixedBlocking ? 'fixed-blocking' : h.blockingOpen ? 'blocking-open' : 'critical-found'}`] : [])]; const skipped: string[] = []; const dispatches: Dispatch[] = []; + const add = (role: Role, name: string, effort: Effort, exclude: string[] = [], extra: Partial = {}) => dispatches.push({ role, name, effort, exclude, ...extra }); + const docsAndOutputs = i.paths.filter(p => pathClass(i, p) === 'docs' || isOutput(i, p)).map(excludeSpec); + const tests = i.paths.filter(p => pathClass(i, p) === 'tests' && !isOutput(i, p)).map(excludeSpec); const numbersChanged = i.numbers.publishes && i.reviewedNumbers !== undefined && i.reviewedNumbers !== i.numbers.fingerprint; - if (next.step === 'full') { - for (const n of specialistsFor(cls, i, skipped)) dispatches.push({ role: 'specialist', name: n, effort: 'full' }); - const redTeam = cls === 'gate' || (cls === 'code' && (i.diffLines > RED_TEAM_LINES || !!i.criticalFound)); - if (redTeam) dispatches.push({ role: 'red-team', name: 'red-team', effort: 'full' }); - else skipped.push(`red-team (${cls} class${cls === 'code' ? `, ${i.diffLines} lines, no critical` : ''})`); - if (i.history.fullPasses === 0 || (!i.adversarialRan && (cls === 'code' || cls === 'gate'))) { - if (cls === 'docs' || cls === 'tests') skipped.push(`adversarial-native (${cls} class)`); - else dispatches.push({ role: 'adversarial-native', name: 'adversarial', effort: 'full' }); - if (cls === 'code' || cls === 'gate' || i.numbers.publishes) dispatches.push({ role: 'outside-voice', name: 'outside-voice', effort: 'host' }); - else skipped.push(`outside-voice (${cls} class, no numeric claims)`); + const firstVoices = h.fullPasses === 0 || !i.adversarialRan; + if (next.step === 'full' && stepDepth === 'light') { + const forced = forcedBy(i); + if (runtimeLines > 0) { + const scoped = SPECIALISTS.filter(n => !LIGHT_FIXED.includes(n) && (forced(n) || (scopeWants[n]?.(i.scope ?? {}) && !(i.gated ?? []).includes(n)))); + add('specialist', 'batched', 'full', docsAndOutputs, { checklists: [...LIGHT_FIXED, ...scoped, 'red-team'] }); + skipped.push('red-team (light depth: the batched specialist carries the Red Team prompt)'); + if (firstVoices) add('adversarial-native', 'adversarial', 'full', [], { reason: LIGHT_ADVERSARIAL_REASON }); + } else { + for (const n of SPECIALISTS.filter(forced)) add('specialist', n, 'full'); + skipped.push('specialists, red-team and adversarial-native (light depth, runtime 0)'); + } + if (h.fullPasses === 0 && i.numbers.publishes) add('outside-voice', 'outside-voice', 'host'); + else if (numbersChanged) add('outside-voice', 'outside-voice (numbers changed)', 'host'); + else skipped.push(`outside-voice (${cls} class, no numeric claims)`); + } else if (next.step === 'full') { + const full = stepDepth === 'full'; + const dcls: DiffClass = full ? 'gate' : cls; + const raised = !full && runtimeLines <= SMALL_DIFF_LINES; + const lines = full ? i.diffLines : Math.max(runtimeLines, raised ? SMALL_DIFF_LINES : 0); + for (const n of specialistsFor(dcls, { ...i, diffLines: lines }, skipped)) add('specialist', n, 'full', full ? [] : n === 'testing' ? docsAndOutputs : [...docsAndOutputs, ...tests]); + const redTeam = dcls === 'gate' || (dcls === 'code' && (lines > RED_TEAM_LINES || !!i.criticalFound || raised)); + if (redTeam) add('red-team', 'red-team', 'full'); + else skipped.push(`red-team (${dcls} class${dcls === 'code' ? `, ${lines} runtime lines, no critical` : ''})`); + const codeLike = dcls === 'code' || dcls === 'gate'; + if (h.fullPasses === 0 || (!i.adversarialRan && codeLike)) { + if (codeLike) add('adversarial-native', 'adversarial', 'full'); + else skipped.push(`adversarial-native (${dcls} class)`); + if (codeLike || i.numbers.publishes) add('outside-voice', 'outside-voice', 'host'); + else skipped.push(`outside-voice (${dcls} class, no numeric claims)`); } else { skipped.push('adversarial-native and outside-voice (run once per invocation)'); - if (numbersChanged) dispatches.push({ role: 'outside-voice', name: 'outside-voice (numbers changed)', effort: 'host' }); + if (numbersChanged) add('outside-voice', 'outside-voice (numbers changed)', 'host'); } } else if (next.step === 'fix-check') { - dispatches.push({ role: 'fix-check', name: 'fix-check', effort: 'full' }); - if (numbersChanged) dispatches.push({ role: 'outside-voice', name: 'outside-voice (numbers changed)', effort: 'host' }); + add('fix-check', 'fix-check', 'full'); + if (numbersChanged) add('outside-voice', 'outside-voice (numbers changed)', 'host'); } - return { class: cls, reasons, next, numbers: i.numbers, dispatches, skipped }; + return { ...depth, stepDepth, depthReasons, reasons: c.reasons, next, numbers: i.numbers, dispatches, skipped }; } export function reviewPlanLine(p: ReviewPlan, pass: number): string { return `REVIEW_PLAN: class=${p.class} pass=${pass} next=${p.next.step} dispatches=${p.dispatches.length} numbers=${p.numbers.publishes ? `yes(${p.numbers.claims})` : 'no'} fingerprint=${p.numbers.fingerprint}`; } + +export function reviewDepthLine(p: ReviewPlan): string { + return `REVIEW_DEPTH: ${p.depth} runtime_lines=${p.runtimeLines} rules=${p.rules} reasons=${p.depthReasons.join(',')} codes=${p.codes.join(',') || 'none'}${p.stepDepth !== p.depth ? ` step=${p.stepDepth}` : ''}`; +} 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..8cff3b954 --- /dev/null +++ b/lib/ship-receipt-depth.ts @@ -0,0 +1,164 @@ +/** + * 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`: the depth the reviewed head needs, recomputed by + * `gstack-ship-receipt write` (receiptDepthAtHead: the committed range from + * git objects, gstack-diff-scope on that range and the gate scan of the clean + * checkout at that head), with the runtime lines, the rules hash, the base and + * registry commits and the gate scan bound to the 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 BIN_DIR = path.join(import.meta.dir, '..', 'bin'); +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() }; +} + +/** gstack-diff-scope on the committed range `base`...`head` (`--head`): the same signals at write, carry and the queue. */ +function scopeOfRange(root: string, binDir: string, base: string, head: string): Record { + const sr = runScriptSync(path.join(binDir, 'gstack-diff-scope'), [base, '--head', head], { cwd: root, encoding: 'utf8', timeout: 60_000, maxBuffer: 16 * 1024 * 1024 }); + const scope: Record = {}; + 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}`; + return scope; +} + +/** + * The depth record for `head`: the range from git objects (collectCommit) and + * its diff-scope, as the queue recomputes them, plus, for a tests or code diff, + * the gate scan of this checkout. The scan reads the working tree, so it counts + * only when the checkout is `head` with no tracked edits; otherwise it is + * unavailable and the depth fails closed to full. + */ +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 baseRev = baseSha.ok ? baseSha.out : branch; + const c = collectCommit(repoRoot, baseRev, head); + const scope = scopeOfRange(repoRoot, binDir, baseRev, head); + let tagged: number | null = 0; + if (c.paths.some(p => classifyPath(p, c.root) !== 'docs')) { + const atHead = git(repoRoot, ['rev-parse', 'HEAD']).out === head && git(repoRoot, ['diff', '--quiet', 'HEAD', '--']).ok; + const g = atHead ? runScriptSync(path.join(binDir, 'gstack-gate-diff'), [baseRev], { cwd: repoRoot, encoding: 'utf8', timeout: 120_000, maxBuffer: 16 * 1024 * 1024 }) : null; + 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: baseRev, 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. + * `isAncestor`, when given, also requires the review-loop exception's fix-check head to be the receipt head or its ancestor. + */ +export function exceptionProblems(body: string, r: ShipReceipt, isAncestor?: (commit: string) => boolean): 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)'); + if (isAncestor && !isAncestor(e.exception.fix_check.head)) out.push(`review_loop_exception.fix_check.head ${e.exception.fix_check.head.slice(0, 12)} is not the receipt head or an ancestor of it`); + 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 baseRev = baseSha.ok ? baseSha.out : baseRef; + const c = collectCommit(root, baseRev, head); + const scanBound = !!rd && rd.gate_scan.tagged !== null && /^[0-9a-f]{40,64}$/.test(rd.gate_scan.head) && rd.gate_scan.head === head.toLowerCase(); + const need = depthOfCommitInput({ ...c, scope: scopeOfRange(root, BIN_DIR, baseRev, head), 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 ancestor = (commit: string) => b.receipt.carry?.ancestry === 'rewritten' || git(root, ['merge-base', '--is-ancestor', commit, b.head]).ok; + const problems = exceptionProblems(body, b.receipt, ancestor); + 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 aa86abf11..25ba416e4 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. */ @@ -118,6 +119,7 @@ export function receiptSummaryLine(r: ShipReceipt): string { if (r.predecessor) parts.push(`predecessor=${r.predecessor.slice(0, 12)}`); if (r.session_kind) parts.push(`session_kind=${r.session_kind}`); if (r.artifacts_consumed) parts.push(`artifacts_consumed=${r.artifacts_consumed}`); + if (r.review_depth && typeof r.review_depth.depth === 'string') parts.push(`depth=${r.review_depth.depth}`); if (r.approval) parts.push(`approval=${approvalSummary(r.approval)}`); const loop = parseReviewLoop(r.review_loop); if (loop) parts.push(`passes=${loop.passes} blocking_found=${loop.blocking_found} deferred=${loop.deferred}` diff --git a/package.json b/package.json index 522b6b09a..8176ec25d 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.91.306", + "version": "1.91.307", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module", diff --git a/scripts/resolvers/review-army.ts b/scripts/resolvers/review-army.ts index 1c174ac94..f4142ee0e 100644 --- a/scripts/resolvers/review-army.ts +++ b/scripts/resolvers/review-army.ts @@ -139,7 +139,7 @@ Every other specialist, including Testing, reads the full diff with fixtures. 5. Instructions: "You are a specialist code reviewer. Read the checklist at {checklist path}, then run -\`${diffBase(ctx)} && git diff "$DIFF_BASE"\` to get the ${ctx.skillName === 'ship' ? 'diff (``: write the sha in before dispatch, the merge base on pass 1 and `RANGE_BASE` from pass 2 on; never a shell variable)' : 'full diff'}. Apply the checklist against the diff. +\`${diffBase(ctx)} && git diff "$DIFF_BASE"${ctx.skillName === 'ship' ? ' ' : ''}\` to get the ${ctx.skillName === 'ship' ? 'diff (``: write the sha in before dispatch, the merge base on pass 1 and `RANGE_BASE` from pass 2 on; never a shell variable; ``: everything after `` on this reviewer\'s `DIFF_CMD:` line, verbatim; empty on `DIFF_EXCLUDE: none`)' : 'full diff'}. Apply the checklist against the diff. \`NO FINDINGS\` alone is the expected, complete result for a clean diff; never invent an item. Otherwise output one JSON object per finding per line, 400 words at most: {\\"severity\\":\\"CRITICAL|INFORMATIONAL\\",\\"confidence\\":N,\\"path\\":\\"file\\",\\"line\\":N,\\"category\\":\\"category\\",\\"summary\\":\\"description\\",\\"fix\\":\\"recommended fix\\",\\"fingerprint\\":\\"path:line:category\\",\\"specialist\\":\\"name\\"} diff --git a/scripts/review-dispatch-compare.ts b/scripts/review-dispatch-compare.ts index 966e4e1e6..d38198e64 100755 --- a/scripts/review-dispatch-compare.ts +++ b/scripts/review-dispatch-compare.ts @@ -17,7 +17,7 @@ import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; import { runScriptSync } from '../lib/script-invocation'; -import { planReview, specialistsFor, type DiffClass, type Dispatch, type History, type NumbersResult } from '../lib/review-depth'; +import { planReview, specialistsFor, type DiffClass, type History, type NumbersResult, type PathLines } from '../lib/review-depth'; const ROOT = path.resolve(import.meta.dir, '..'); const BYTES_PER_TOKEN = 4; @@ -30,7 +30,8 @@ const DEFAULT_OUT: Record = { 'outside-voice': { full: 6000, light: 6000 }, 'fix-check': { full: 4000, light: 2000 }, audit: { full: 5000, light: 2500 }, 'autoplan-voice': { full: 9000, light: 4500 }, }; -export interface Fixture { name: string; commit: string; paths: string[]; deleted: string[]; stamp_only?: string[]; scope: Record; gate_tagged: number | null; diff_lines: number; diff_bytes: number; numbers: NumbersResult } +/** per_path and registry_outputs (captured since GSTA-147) feed the runtime-line count; an older fixture counts every code-class line as runtime. */ +export interface Fixture { name: string; commit: string; paths: string[]; deleted: string[]; stamp_only?: string[]; scope: Record; gate_tagged: number | null; diff_lines: number; diff_bytes: number; numbers: NumbersResult; per_path?: PathLines[]; registry_outputs?: string[] } /** clean: pass 1 finds nothing; nits: pass 1 fixes below-floor findings; blocker: pass 1 fixes a blocker; adversarial-fix: Step 11 finds a should that is fixed; fix-regression: a pass-1 fix introduces a blocker the next step finds. */ type Scenario = 'clean' | 'nits' | 'blocker' | 'adversarial-fix' | 'fix-regression'; const SCENARIOS: Scenario[] = ['clean', 'nits', 'blocker', 'adversarial-fix', 'fix-regression']; @@ -62,7 +63,7 @@ function legacyShip(f: Fixture, sc: Scenario): Call[] { } function base(f: Fixture) { - return { paths: f.paths, deleted: f.deleted, stampOnly: f.stamp_only ?? [], scope: f.scope, gateTagged: f.gate_tagged, diffLines: f.diff_lines, numbers: f.numbers }; + return { paths: f.paths, deleted: f.deleted, stampOnly: f.stamp_only ?? [], scope: f.scope, gateTagged: f.gate_tagged, diffLines: f.diff_lines, numbers: f.numbers, perPath: f.per_path, registry: f.registry_outputs ? { sha: null, outputs: f.registry_outputs } : undefined }; } /** The new policy, step by step through lib/review-depth's own transition table. */ @@ -121,10 +122,10 @@ function capture(sha: string, name: string, out: string): number { if (r.status !== 0) throw new Error(`gstack-review-plan failed: ${r.stderr}`); const plan = JSON.parse(r.stdout); const diffBytes = Buffer.byteLength(git(['diff', `${sha}^`, sha], dir).stdout ?? ''); - const fixture: Fixture = { name, commit: sha, paths: plan.paths, deleted: plan.deleted, stamp_only: plan.stamp_only, scope: plan.scope, gate_tagged: plan.gate_tagged, diff_lines: plan.diff_lines, diff_bytes: diffBytes, numbers: plan.numbers }; + const fixture: Fixture = { name, commit: sha, paths: plan.paths, deleted: plan.deleted, stamp_only: plan.stamp_only, scope: plan.scope, gate_tagged: plan.gate_tagged, diff_lines: plan.diff_lines, diff_bytes: diffBytes, numbers: plan.numbers, per_path: plan.per_path, registry_outputs: plan.registry.outputs }; fs.mkdirSync(path.dirname(out), { recursive: true }); fs.writeFileSync(out, JSON.stringify(fixture, null, 2) + '\n'); - process.stdout.write(`CAPTURED: ${name} ${sha} class=${plan.class} paths=${plan.paths.length} lines=${plan.diff_lines} -> ${out}\n`); + process.stdout.write(`CAPTURED: ${name} ${sha} class=${plan.class} depth=${plan.depth} runtime=${plan.runtime_lines} paths=${plan.paths.length} lines=${plan.diff_lines} -> ${out}\n`); return 0; } finally { git(['worktree', 'remove', '--force', dir]); @@ -135,12 +136,13 @@ function capture(sha: string, name: string, out: string): number { export function compare(fixtures: Fixture[], out: typeof DEFAULT_OUT) { const rows: Array> = []; for (const f of fixtures) { - const cls: DiffClass = planReview({ ...base(f), history: { fullPasses: 0, last: 'none' } }).class; + const first = planReview({ ...base(f), history: { fullPasses: 0, last: 'none' } }); + const cls: DiffClass = first.class; for (const sc of SCENARIOS) { const legacy = legacyShip(f, sc); const next = newShip(f, sc); const lt = tokens(legacy, out); const nt = tokens(next.calls, out); - rows.push({ fixture: f.name, class: cls, scenario: sc, legacy_dispatches: legacy.length, new_dispatches: next.calls.length, legacy_tokens: lt, new_tokens: nt, ratio: Math.round((nt / lt) * 100) / 100, ending: next.ending }); + rows.push({ fixture: f.name, class: cls, depth: first.depth, runtime_lines: first.runtimeLines, scenario: sc, legacy_dispatches: legacy.length, new_dispatches: next.calls.length, legacy_tokens: lt, new_tokens: nt, ratio: Math.round((nt / lt) * 100) / 100, ending: next.ending }); } } return rows; @@ -158,8 +160,8 @@ function main(argv: string[]): number { const rows = compare(fixtures, out); const ap = fixtures.map(f => ({ fixture: f.name, runs: autoplanRuns(planReview({ ...base(f), history: { fullPasses: 0, last: 'none' } }).class === 'docs') })); if (argv.includes('--json')) { process.stdout.write(JSON.stringify({ tokens_source: tokFile ?? 'DEFAULT', ship: rows, autoplan: ap }) + '\n'); return 0; } - const lines = [`TOKENS: ${tokFile ? `measured (${tokFile})` : 'DEFAULT estimates (pass --tokens for measured output)'}`, '', '| fixture | class | scenario | /ship dispatches legacy → new | est. tokens legacy → new | ratio | ending |', '|---|---|---|---|---|---|---|']; - for (const r of rows) lines.push(`| ${r.fixture} | ${r.class} | ${r.scenario} | ${r.legacy_dispatches} → ${r.new_dispatches} | ${r.legacy_tokens} → ${r.new_tokens} | ${r.ratio} | ${r.ending} |`); + const lines = [`TOKENS: ${tokFile ? `measured (${tokFile})` : 'DEFAULT estimates (pass --tokens for measured output)'}`, '', '| fixture | class | depth (runtime lines) | scenario | /ship dispatches legacy → new | est. tokens legacy → new | ratio | ending |', '|---|---|---|---|---|---|---|---|']; + for (const r of rows) lines.push(`| ${r.fixture} | ${r.class} | ${r.depth} (${r.runtime_lines}) | ${r.scenario} | ${r.legacy_dispatches} → ${r.new_dispatches} | ${r.legacy_tokens} → ${r.new_tokens} | ${r.ratio} | ${r.ending} |`); lines.push('', '| fixture | /autoplan scenario | voice dispatches legacy → new |', '|---|---|---|'); for (const a of ap) for (const r of a.runs) lines.push(`| ${a.fixture} | ${r.scenario} | ${r.legacy} → ${r.next} |`); const median = (v: number[]) => { const m = Math.floor(v.length / 2); return v.length % 2 ? v[m]! : Math.round(((v[m - 1]! + v[m]!) / 2) * 100) / 100; }; @@ -168,6 +170,9 @@ function main(argv: string[]): number { const byClass = new Map(); for (const r of rows) byClass.set(String(r.class), [...(byClass.get(String(r.class)) ?? []), Number(r.ratio)].sort((x, y) => x - y)); lines.push(`SHIP_TOKEN_RATIO_BY_CLASS: ${[...byClass].map(([c, v]) => `${c} median=${median(v)} (n=${v.length})`).join('; ')}`); + const byDepth = new Map(); + for (const r of rows) byDepth.set(String(r.depth), [...(byDepth.get(String(r.depth)) ?? []), Number(r.ratio)].sort((x, y) => x - y)); + lines.push(`SHIP_TOKEN_RATIO_BY_DEPTH: ${[...byDepth].map(([d, v]) => `${d} median=${median(v)} (n=${v.length})`).join('; ')}`); process.stdout.write(lines.join('\n') + '\n'); return 0; } diff --git a/ship/SKILL.md b/ship/SKILL.md index 881cd3934..5846b96d3 100644 --- a/ship/SKILL.md +++ b/ship/SKILL.md @@ -202,11 +202,11 @@ token; `--finish` stamps the binding fields automatically. Never borrow or repla `gstack-wtree` prints a Git tree hash covering tracked and non-ignored untracked files, not a commit ID. Use `git diff ` to compare these snapshots. -**Review policy** (docs/review-policy.md): a finding blocks another review pass -only when it would fail a test, break production, or lose data or secrets; -everything below that floor ships as a named note in the PR body. Pass 1 reviews -the diff; a later pass reads only the range; two full passes plus one fix-check -is the cap. `NO FINDINGS` is a complete reviewer result. `ship: review_scope` (Step 3) +**Review policy** (docs/review-policy.md): only a finding that would fail a test, +break production, or lose data or secrets blocks a pass; the rest is a PR-body +note. Step 9's plan sets both reviews' depth (`--depth-floor standard|full` only +raises it). Pass 1 reads the diff, later passes the range; the cap is two full +passes and a fix-check. `NO FINDINGS` is complete. `ship: review_scope` (Step 3) is the knob; the receipt's `review_loop` records the outcome. ### Ship control flow diff --git a/ship/SKILL.md.tmpl b/ship/SKILL.md.tmpl index 16f65656e..810bbd990 100644 --- a/ship/SKILL.md.tmpl +++ b/ship/SKILL.md.tmpl @@ -68,11 +68,11 @@ token; `--finish` stamps the binding fields automatically. Never borrow or repla `gstack-wtree` prints a Git tree hash covering tracked and non-ignored untracked files, not a commit ID. Use `git diff ` to compare these snapshots. -**Review policy** (docs/review-policy.md): a finding blocks another review pass -only when it would fail a test, break production, or lose data or secrets; -everything below that floor ships as a named note in the PR body. Pass 1 reviews -the diff; a later pass reads only the range; two full passes plus one fix-check -is the cap. `NO FINDINGS` is a complete reviewer result. `ship: review_scope` (Step 3) +**Review policy** (docs/review-policy.md): only a finding that would fail a test, +break production, or lose data or secrets blocks a pass; the rest is a PR-body +note. Step 9's plan sets both reviews' depth (`--depth-floor standard|full` only +raises it). Pass 1 reads the diff, later passes the range; the cap is two full +passes and a fix-check. `NO FINDINGS` is complete. `ship: review_scope` (Step 3) is the knob; the receipt's `review_loop` records the outcome. ### Ship control flow diff --git a/ship/sections/adversarial.md b/ship/sections/adversarial.md index 3436b5c24..620f17248 100644 --- a/ship/sections/adversarial.md +++ b/ship/sections/adversarial.md @@ -2,7 +2,8 @@ **Step 9's last full-pass plan governs this step:** run the native pass only on `DISPATCH: adversarial-native` and the outside voice only on -`DISPATCH: outside-voice`. Without the native line, print `ADVERSARIAL: skipped (review plan)`; +`DISPATCH: outside-voice`. Without the native line (light depth at runtime 0; +above 0 the plan lists it beside the batched specialist), print `ADVERSARIAL: skipped (review plan)`; with neither line, take decision 3 of Finish the adversarial phase. ## Step 11: Adversarial review diff --git a/ship/sections/adversarial.md.tmpl b/ship/sections/adversarial.md.tmpl index abb9615ae..df982e205 100644 --- a/ship/sections/adversarial.md.tmpl +++ b/ship/sections/adversarial.md.tmpl @@ -1,6 +1,7 @@ **Step 9's last full-pass plan governs this step:** run the native pass only on `DISPATCH: adversarial-native` and the outside voice only on -`DISPATCH: outside-voice`. Without the native line, print `ADVERSARIAL: skipped (review plan)`; +`DISPATCH: outside-voice`. Without the native line (light depth at runtime 0; +above 0 the plan lists it beside the batched specialist), print `ADVERSARIAL: skipped (review plan)`; with neither line, take decision 3 of Finish the adversarial phase. {{ADVERSARIAL_STEP}} 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/ship/sections/review-army.md b/ship/sections/review-army.md index 32f18e440..06419d047 100644 --- a/ship/sections/review-army.md +++ b/ship/sections/review-army.md @@ -14,13 +14,19 @@ Every repeat starts before the checklist read and captures a fresh REVIEW_START. Finish the complete review and QA before applying any fix in Step 9.4. **Review plan.** Pass 1 runs `~/.claude/skills/gstack/bin/gstack-review-plan ---base `; a repeat uses the plan run that picked it. Save its `REVIEW_PLAN:` line and -fingerprint. Dispatch exactly its `DISPATCH:` lines (specialists, Red Team, Step -11's voices), each at its `effort=` on the same model family, in place of 9.1's and -9.2's own selection rules; a `SKIP:` reviewer -logs `{"dispatched":false,"reason":"scope"}`. Audits run at `AUDIT_EFFORT:`. The -core checklist, gate scan and QA always run. Item 6's row adds -`"dispatches":[{"role":R,"prompt_bytes":N,"diff_bytes":N}]`. +--base `, plus `--depth-floor ` on every plan run when the /ship prompt +gave `--depth-floor ` (it only raises); a repeat uses the plan run that picked it. +Save its `REVIEW_PLAN:` line and fingerprint, `REVIEW_DEPTH:` (depth, +`runtime_lines`, `rules`) and `GATE_SCAN:`. Dispatch exactly its `DISPATCH:` lines +(specialists, Red Team, Step 11's voices), each at its `effort=` on the same model +family and reading its `DIFF_CMD:`, in place of 9.1's and 9.2's own selection rules; +`specialist batched` (light depth) is one subagent reading every checklist its +`checklists=` names, Red Team included, tagging each finding with its checklist. +A `SKIP:` reviewer logs `{"dispatched":false,"reason":"scope"}`. Audits run at +`AUDIT_EFFORT:`. The core checklist, gate scan and QA always run. Item 6's row adds +`"dispatches":[{"role":R,"prompt_bytes":N,"diff_bytes":N}]`, `"depth"`, +`"runtime_lines"`, `"depth_rules"` (the `rules` hash) and +`"gate_scan":{"tagged":N|null,"head":""}` from those lines. **Pass 2+ reads the range, not the tree** (`review_scope frozen`): the core read in item 2 becomes `git diff $RANGE_BASE`, and every dispatched prompt (specialists, @@ -391,7 +397,7 @@ Every other specialist, including Testing, reads the full diff with fixtures. 5. Instructions: "You are a specialist code reviewer. Read the checklist at {checklist path}, then run -`DIFF_BASE= && git diff "$DIFF_BASE"` to get the diff (``: write the sha in before dispatch, the merge base on pass 1 and `RANGE_BASE` from pass 2 on; never a shell variable). Apply the checklist against the diff. +`DIFF_BASE= && git diff "$DIFF_BASE" ` to get the diff (``: write the sha in before dispatch, the merge base on pass 1 and `RANGE_BASE` from pass 2 on; never a shell variable; ``: everything after `` on this reviewer's `DIFF_CMD:` line, verbatim; empty on `DIFF_EXCLUDE: none`). Apply the checklist against the diff. `NO FINDINGS` alone is the expected, complete result for a clean diff; never invent an item. Otherwise output one JSON object per finding per line, 400 words at most: {\"severity\":\"CRITICAL|INFORMATIONAL\",\"confidence\":N,\"path\":\"file\",\"line\":N,\"category\":\"category\",\"summary\":\"description\",\"fix\":\"recommended fix\",\"fingerprint\":\"path:line:category\",\"specialist\":\"name\"} @@ -802,7 +808,8 @@ After persistence, record missing output, CYCLES and fixes in the invocation rec Decision 1 always applies first. Then, under `frozen`, run `gstack-review-plan --base --pass $PASS --last full` with each flag that holds (`--fixed-blocking`, `--fixed-below`, `--blocking-open`, `--touched-gate` for a fix to an item 2.5 path, `--critical-found` when this pass -logged a CRITICAL, `--adversarial-ran` once Step 11's native pass ran, `--reviewed-numbers `); +logged a CRITICAL, `--adversarial-ran` once Step 11's native pass ran, `--reviewed-numbers `, +`--depth-floor ` when given); its `NEXT:` alone picks the next decision: `stop` 2, `full` 3, `fix-check` 3.5, `done` 4. Other scopes apply decisions 2–4 in order. diff --git a/ship/sections/review-army.md.tmpl b/ship/sections/review-army.md.tmpl index 6317b79c6..704bfb6b1 100644 --- a/ship/sections/review-army.md.tmpl +++ b/ship/sections/review-army.md.tmpl @@ -12,13 +12,19 @@ Every repeat starts before the checklist read and captures a fresh REVIEW_START. Finish the complete review and QA before applying any fix in Step 9.4. **Review plan.** Pass 1 runs `~/.claude/skills/gstack/bin/gstack-review-plan ---base `; a repeat uses the plan run that picked it. Save its `REVIEW_PLAN:` line and -fingerprint. Dispatch exactly its `DISPATCH:` lines (specialists, Red Team, Step -11's voices), each at its `effort=` on the same model family, in place of 9.1's and -9.2's own selection rules; a `SKIP:` reviewer -logs `{"dispatched":false,"reason":"scope"}`. Audits run at `AUDIT_EFFORT:`. The -core checklist, gate scan and QA always run. Item 6's row adds -`"dispatches":[{"role":R,"prompt_bytes":N,"diff_bytes":N}]`. +--base `, plus `--depth-floor ` on every plan run when the /ship prompt +gave `--depth-floor ` (it only raises); a repeat uses the plan run that picked it. +Save its `REVIEW_PLAN:` line and fingerprint, `REVIEW_DEPTH:` (depth, +`runtime_lines`, `rules`) and `GATE_SCAN:`. Dispatch exactly its `DISPATCH:` lines +(specialists, Red Team, Step 11's voices), each at its `effort=` on the same model +family and reading its `DIFF_CMD:`, in place of 9.1's and 9.2's own selection rules; +`specialist batched` (light depth) is one subagent reading every checklist its +`checklists=` names, Red Team included, tagging each finding with its checklist. +A `SKIP:` reviewer logs `{"dispatched":false,"reason":"scope"}`. Audits run at +`AUDIT_EFFORT:`. The core checklist, gate scan and QA always run. Item 6's row adds +`"dispatches":[{"role":R,"prompt_bytes":N,"diff_bytes":N}]`, `"depth"`, +`"runtime_lines"`, `"depth_rules"` (the `rules` hash) and +`"gate_scan":{"tagged":N|null,"head":""}` from those lines. **Pass 2+ reads the range, not the tree** (`review_scope frozen`): the core read in item 2 becomes `git diff $RANGE_BASE`, and every dispatched prompt (specialists, @@ -216,7 +222,8 @@ After persistence, record missing output, CYCLES and fixes in the invocation rec Decision 1 always applies first. Then, under `frozen`, run `gstack-review-plan --base --pass $PASS --last full` with each flag that holds (`--fixed-blocking`, `--fixed-below`, `--blocking-open`, `--touched-gate` for a fix to an item 2.5 path, `--critical-found` when this pass -logged a CRITICAL, `--adversarial-ran` once Step 11's native pass ran, `--reviewed-numbers `); +logged a CRITICAL, `--adversarial-ran` once Step 11's native pass ran, `--reviewed-numbers `, +`--depth-floor ` when given); its `NEXT:` alone picks the next decision: `stop` 2, `full` 3, `fix-check` 3.5, `done` 4. Other scopes apply decisions 2–4 in order. diff --git a/test/diff-scope.test.ts b/test/diff-scope.test.ts index 9597362a9..e33469c1a 100644 --- a/test/diff-scope.test.ts +++ b/test/diff-scope.test.ts @@ -367,6 +367,27 @@ describe('uncommitted work is visible (#2299)', () => { expect(vars.SCOPE_BACKEND).toBe('true'); }); + // Value: protects=--head scopes one committed range from any checkout, the input the receipt writer and the merge queue + // share; fails_when=--head still reads the working tree or untracked files, or an unknown head reads as all-false; seam=none + test('--head scopes only the committed range: working-tree and untracked files are ignored, an unknown head is an error', () => { + const dir = createRepo(['lib/auth.ts']); + const head = spawnSync('git', ['rev-parse', 'HEAD'], { cwd: dir, stdio: 'pipe' }).stdout.toString().trim(); + mkdirSync(join(dir, 'db', 'migrate'), { recursive: true }); + writeFileSync(join(dir, 'db', 'migrate', '001.rb'), '# untracked\n'); + writeFileSync(join(dir, 'styles.css'), 'a{}\n'); + const scope = (args: string[]) => spawnSync('bash', [SCRIPT, 'main', ...args], { cwd: dir, stdio: 'pipe', timeout: 5000 }); + const ranged = scope(['--head', head]); + expect(ranged.status).toBe(0); + const out = ranged.stdout.toString(); + expect(out).toContain('SCOPE_AUTH=true'); + expect(out).toContain('SCOPE_MIGRATIONS=false'); + expect(out).toContain('SCOPE_FRONTEND=false'); + expect(runScopeFull(dir).vars.SCOPE_MIGRATIONS).toBe('true'); + const unknown = scope(['--head', 'f'.repeat(40)]); + expect(unknown.status).toBe(2); + expect(unknown.stdout.toString()).toContain('SCOPE_ERROR=no_head'); + }); + test('non-ASCII path still matches extension globs (NUL-safe file listing, #2526)', () => { const dir = createRepo(['docs/M2 — Notes.md']); const { vars, status } = runScopeFull(dir); diff --git a/test/fixtures/golden/agy-ship-SKILL.md b/test/fixtures/golden/agy-ship-SKILL.md index 7494fe0c4..59c5baac2 100644 --- a/test/fixtures/golden/agy-ship-SKILL.md +++ b/test/fixtures/golden/agy-ship-SKILL.md @@ -202,11 +202,11 @@ token; `--finish` stamps the binding fields automatically. Never borrow or repla `gstack-wtree` prints a Git tree hash covering tracked and non-ignored untracked files, not a commit ID. Use `git diff ` to compare these snapshots. -**Review policy** (docs/review-policy.md): a finding blocks another review pass -only when it would fail a test, break production, or lose data or secrets; -everything below that floor ships as a named note in the PR body. Pass 1 reviews -the diff; a later pass reads only the range; two full passes plus one fix-check -is the cap. `NO FINDINGS` is a complete reviewer result. `ship: review_scope` (Step 3) +**Review policy** (docs/review-policy.md): only a finding that would fail a test, +break production, or lose data or secrets blocks a pass; the rest is a PR-body +note. Step 9's plan sets both reviews' depth (`--depth-floor standard|full` only +raises it). Pass 1 reads the diff, later passes the range; the cap is two full +passes and a fix-check. `NO FINDINGS` is complete. `ship: review_scope` (Step 3) is the knob; the receipt's `review_loop` records the outcome. ### Ship control flow diff --git a/test/fixtures/golden/claude-ship-SKILL.md b/test/fixtures/golden/claude-ship-SKILL.md index 881cd3934..5846b96d3 100644 --- a/test/fixtures/golden/claude-ship-SKILL.md +++ b/test/fixtures/golden/claude-ship-SKILL.md @@ -202,11 +202,11 @@ token; `--finish` stamps the binding fields automatically. Never borrow or repla `gstack-wtree` prints a Git tree hash covering tracked and non-ignored untracked files, not a commit ID. Use `git diff ` to compare these snapshots. -**Review policy** (docs/review-policy.md): a finding blocks another review pass -only when it would fail a test, break production, or lose data or secrets; -everything below that floor ships as a named note in the PR body. Pass 1 reviews -the diff; a later pass reads only the range; two full passes plus one fix-check -is the cap. `NO FINDINGS` is a complete reviewer result. `ship: review_scope` (Step 3) +**Review policy** (docs/review-policy.md): only a finding that would fail a test, +break production, or lose data or secrets blocks a pass; the rest is a PR-body +note. Step 9's plan sets both reviews' depth (`--depth-floor standard|full` only +raises it). Pass 1 reads the diff, later passes the range; the cap is two full +passes and a fix-check. `NO FINDINGS` is complete. `ship: review_scope` (Step 3) is the knob; the receipt's `review_loop` records the outcome. ### Ship control flow diff --git a/test/fixtures/golden/codex-ship-SKILL.md b/test/fixtures/golden/codex-ship-SKILL.md index c4d937f91..afd639848 100644 --- a/test/fixtures/golden/codex-ship-SKILL.md +++ b/test/fixtures/golden/codex-ship-SKILL.md @@ -200,11 +200,11 @@ token; `--finish` stamps the binding fields automatically. Never borrow or repla `gstack-wtree` prints a Git tree hash covering tracked and non-ignored untracked files, not a commit ID. Use `git diff ` to compare these snapshots. -**Review policy** (docs/review-policy.md): a finding blocks another review pass -only when it would fail a test, break production, or lose data or secrets; -everything below that floor ships as a named note in the PR body. Pass 1 reviews -the diff; a later pass reads only the range; two full passes plus one fix-check -is the cap. `NO FINDINGS` is a complete reviewer result. `ship: review_scope` (Step 3) +**Review policy** (docs/review-policy.md): only a finding that would fail a test, +break production, or lose data or secrets blocks a pass; the rest is a PR-body +note. Step 9's plan sets both reviews' depth (`--depth-floor standard|full` only +raises it). Pass 1 reads the diff, later passes the range; the cap is two full +passes and a fix-check. `NO FINDINGS` is complete. `ship: review_scope` (Step 3) is the knob; the receipt's `review_loop` records the outcome. ### Ship control flow diff --git a/test/fixtures/golden/factory-ship-SKILL.md b/test/fixtures/golden/factory-ship-SKILL.md index 374d879a4..e2c3e30bc 100644 --- a/test/fixtures/golden/factory-ship-SKILL.md +++ b/test/fixtures/golden/factory-ship-SKILL.md @@ -202,11 +202,11 @@ token; `--finish` stamps the binding fields automatically. Never borrow or repla `gstack-wtree` prints a Git tree hash covering tracked and non-ignored untracked files, not a commit ID. Use `git diff ` to compare these snapshots. -**Review policy** (docs/review-policy.md): a finding blocks another review pass -only when it would fail a test, break production, or lose data or secrets; -everything below that floor ships as a named note in the PR body. Pass 1 reviews -the diff; a later pass reads only the range; two full passes plus one fix-check -is the cap. `NO FINDINGS` is a complete reviewer result. `ship: review_scope` (Step 3) +**Review policy** (docs/review-policy.md): only a finding that would fail a test, +break production, or lose data or secrets blocks a pass; the rest is a PR-body +note. Step 9's plan sets both reviews' depth (`--depth-floor standard|full` only +raises it). Pass 1 reads the diff, later passes the range; the cap is two full +passes and a fix-check. `NO FINDINGS` is complete. `ship: review_scope` (Step 3) is the knob; the receipt's `review_loop` records the outcome. ### Ship control flow 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/review-collect.test.ts b/test/review-collect.test.ts new file mode 100644 index 000000000..0c86ac6ef --- /dev/null +++ b/test/review-collect.test.ts @@ -0,0 +1,119 @@ +import { afterEach, describe, expect, test } from 'bun:test'; +import { spawnSync } from 'node:child_process'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { collectCommit, collectLive, loadRegistryAt } from '../lib/review-collect'; +import { depthOfCommitInput } from '../lib/review-depth'; + +const dirs: string[] = []; +afterEach(() => { while (dirs.length) fs.rmSync(dirs.pop()!, { recursive: true, force: true }); }); +const lines = (n: number, tag = 'v') => Array.from({ length: n }, (_, i) => `export const ${tag}${i} = ${i};`).join('\n') + '\n'; +const REGISTRY = JSON.stringify({ entries: [{ id: 'docs', command: 'bun run gen', outputs: ['*/SKILL.md'] }, { id: 'manual', command: null, outputs: ['src/hand.ts'] }] }); + +function repo(baseFiles: Record) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'review-collect-')); + dirs.push(dir); + const git = (...a: string[]) => { const r = spawnSync('git', a, { cwd: dir, encoding: 'utf8', timeout: 10_000 }); if (r.status !== 0) throw new Error(`git ${a.join(' ')}: ${r.stderr}`); return r.stdout.trim(); }; + const write = (files: Record) => { for (const [p, text] of Object.entries(files)) { const f = path.join(dir, p); if (text === null) { fs.rmSync(f); continue; } fs.mkdirSync(path.dirname(f), { recursive: true }); fs.writeFileSync(f, text); } }; + git('init', '-q', '-b', 'main'); git('config', 'user.email', 't@t'); git('config', 'user.name', 't'); git('config', 'commit.gpgsign', 'false'); + write({ 'README.md': '# x\n', ...baseFiles }); git('add', '-A'); git('commit', '-qm', 'base'); + const baseSha = git('rev-parse', 'HEAD'); + git('checkout', '-qb', 'feat'); + const commit = (files: Record) => { write(files); git('add', '-A'); git('commit', '-qm', 'feat'); return git('rev-parse', 'HEAD'); }; + return { dir, git, write, commit, baseSha }; +} + +describe('collectCommit: git objects only', () => { + // Value: protects=only a bare version bump in package.json is release metadata; fails_when=a version line that also + // carries scripts counts as stamp-only (docs class, light); seam=none + test('a package.json version line that also carries scripts is not stamp-only; a bare version bump is', () => { + const r = repo({ 'package.json': '{\n "name": "x",\n "version": "1.0.0"\n}\n' }); + const bare = r.commit({ 'package.json': '{\n "name": "x",\n "version": "1.0.1"\n}\n' }); + expect(collectCommit(r.dir, r.baseSha, bare).stampOnly).toEqual(['package.json']); + const riding = r.commit({ 'package.json': '{\n "name": "x",\n "version": "1.0.2", "scripts": { "postinstall": "node x.js" }\n}\n' }); + expect(collectCommit(r.dir, r.baseSha, riding).stampOnly).toEqual([]); + }); + test('a registered glob output is excluded, a command:null entry is not, and a .tmpl edit counts once', () => { + const r = repo({ '.gstack/generated.json': REGISTRY, 'ship/SKILL.md.tmpl': 'a\n', 'ship/SKILL.md': 'a\n' }); + const head = r.commit({ 'ship/SKILL.md.tmpl': 'a\nb\nc\n', 'ship/SKILL.md': lines(120), 'src/hand.ts': lines(5) }); + const c = collectCommit(r.dir, r.baseSha, head); + expect(c.errors).toEqual([]); + expect(c.registry).toEqual({ sha: r.baseSha, outputs: ['*/SKILL.md'] }); + const d = depthOfCommitInput({ ...c, gateTagged: 0 }); + expect([d.depth, d.runtimeLines]).toEqual(['light', 2 + 5]); + }); + test('a head-only registry entry never excludes: the registry is the base commit\'s', () => { + const r = repo({ 'src/keep.ts': lines(1) }); + const head = r.commit({ '.gstack/generated.json': JSON.stringify({ entries: [{ id: 'gen', command: 'gen', outputs: ['src/gen.ts'] }] }), 'src/gen.ts': lines(60) }); + const c = collectCommit(r.dir, r.baseSha, head); + expect(c.registry).toEqual({ sha: r.baseSha, outputs: [] }); + const d = depthOfCommitInput({ ...c, gateTagged: 0 }); + expect(d.runtimeLines).toBeGreaterThanOrEqual(60); + expect(d.depth).toBe('full'); + }); + test('a rename into src/ counts the new path as runtime', () => { + const r = repo({ 'notes/a.txt': lines(30) }); + fs.mkdirSync(path.join(r.dir, 'src')); + r.git('mv', 'notes/a.txt', 'src/a.ts'); + r.git('commit', '-qm', 'mv'); + const c = collectCommit(r.dir, r.baseSha, r.git('rev-parse', 'HEAD')); + expect(c.deleted).toEqual(['notes/a.txt']); + expect(c.perPath.find(e => e.path === 'src/a.ts')).toEqual({ path: 'src/a.ts', lines: 30, binary: false }); + expect(depthOfCommitInput({ ...c, gateTagged: 0 })).toMatchObject({ depth: 'light', runtimeLines: 30 }); + }); + test('a binary code-class path is an error (full), never 0 lines; a deleted test is full', () => { + const r = repo({ 'test/old.test.ts': 'x\n' }); + const head = r.commit({ 'lib/x.wasm': Buffer.from([0, 97, 115, 109, 0, 1, 2]) }); + const d = depthOfCommitInput({ ...collectCommit(r.dir, r.baseSha, head), gateTagged: 0 }); + expect(d.depth).toBe('full'); + expect(d.errors).toEqual([expect.objectContaining({ code: 'binary_code_path', input: 'lib/x.wasm' })]); + const t = repo({ 'test/old.test.ts': 'x\n', 'test/keep.test.ts': 'x\n' }); + const del = t.commit({ 'test/old.test.ts': null }); + const deleted = depthOfCommitInput({ ...collectCommit(t.dir, t.baseSha, del), gateTagged: 0 }); + expect([deleted.depth, deleted.runtimeLines, deleted.errors]).toEqual(['full', 0, []]); + }); + test('an invalid base registry is an error with its result code; an unresolvable sha is unavailable', () => { + const r = repo({ '.gstack/generated.json': '{"entries": [{"id": "x"}]}' }); + const head = r.commit({ 'src/a.ts': lines(3) }); + const d = depthOfCommitInput({ ...collectCommit(r.dir, r.baseSha, head), gateTagged: 0 }); + expect(d.depth).toBe('full'); + expect(d.codes).toEqual(['REGEN_REGISTRY_INVALID']); + expect(d.errors[0]).toMatchObject({ code: 'registry_invalid', result: 'REGEN_REGISTRY_INVALID' }); + expect(d.errors[0]!.repair).toContain('gstack-regen validate'); + expect(loadRegistryAt(r.dir, 'f'.repeat(40)).error?.code).toBe('registry_unavailable'); + const missing = collectCommit(r.dir, 'f'.repeat(40), head); + expect(missing.errors.map(e => e.code)).toContain('numstat_failed'); + expect(depthOfCommitInput({ ...missing, gateTagged: 0 }).depth).toBe('full'); + }); + test('from a checkout at another HEAD with a dirty tree it equals the clean result at head', () => { + const r = repo({ '.gstack/generated.json': REGISTRY, 'src/a.ts': lines(2), 'ship/SKILL.md': 'a\n', 'autoplan/SKILL.md.tmpl': 'x\n' }); + const head = r.commit({ 'src/a.ts': lines(40), 'ship/SKILL.md': lines(9), 'docs/b.md': '# b\n12% faster\n', 'autoplan/notes.md': 'n\n', 'package.json': '{"version":"1.0.1"}\n' }); + const clean = collectCommit(r.dir, r.baseSha, head); + r.git('checkout', '-q', r.baseSha); + r.write({ 'src/a.ts': lines(500, 'dirty'), 'src/untracked.ts': lines(80), 'autoplan/SKILL.md.tmpl': null }); + const dirty = collectCommit(r.dir, r.baseSha, head); + expect(dirty).toEqual(clean); + expect(depthOfCommitInput({ ...dirty, gateTagged: 0 })).toEqual(depthOfCommitInput({ ...clean, gateTagged: 0 })); + expect(clean.root).toEqual(['autoplan', 'ship']); + expect(clean.added['docs/b.md']).toEqual(['# b', '12% faster']); + }); +}); + +describe('collectLive: the working tree', () => { + test('tracked uncommitted edits and untracked files count per path; scratch under .gstack/tmp/ does not', () => { + const r = repo({ 'src/a.ts': lines(1) }); + r.commit({ 'docs/a.md': '# a\n' }); + r.write({ 'src/a.ts': lines(11), 'src/new.ts': lines(20), '.gstack/tmp/x.ts': lines(500) }); + const c = collectLive(r.dir, 'main'); + expect(c.errors).toEqual([]); + expect(c.untracked).toEqual(['src/new.ts']); + expect(c.perPath.find(e => e.path === 'src/new.ts')).toEqual({ path: 'src/new.ts', lines: 20, binary: false }); + expect(c.paths).toEqual(['docs/a.md', 'src/a.ts', 'src/new.ts']); + expect(depthOfCommitInput({ ...c, gateTagged: 0 })).toMatchObject({ depth: 'light', runtimeLines: 20 + 10 }); + }); + test('no merge base is a collector error the CLI refuses on', () => { + const r = repo({}); + expect(collectLive(r.dir, 'no-such-branch').errors.map(e => e.code)).toEqual(['numstat_failed']); + }); +}); diff --git a/test/review-depth.test.ts b/test/review-depth.test.ts index d8c0ab4fa..db2ea9b05 100644 --- a/test/review-depth.test.ts +++ b/test/review-depth.test.ts @@ -1,6 +1,9 @@ import { describe, test, expect } from 'bun:test'; +import { spawnSync } from 'node:child_process'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; import * as path from 'node:path'; -import { classifyDiff, classifyPath, nextStep, planReview, scanNumbers, FULL_PASS_CAP, type PlanInput } from '../lib/review-depth'; +import { classifyDiff, classifyPath, depthFor, depthOfCommitInput, diffCommand, nextStep, planReview, scanNumbers, DEPTH_RULES, FULL_PASS_CAP, type DepthError, type PlanInput } from '../lib/review-depth'; const ROOT = path.join(import.meta.dir, '..'); const none = scanNumbers({}); @@ -73,9 +76,10 @@ describe('planReview', () => { const p = planReview(base({ paths: ['docs/bench.md'], scope: {}, numbers: scanNumbers({ 'docs/bench.md': ['3x faster'] }) })); expect(roles(p)).toEqual(['outside-voice:outside-voice:host']); }); - test('tests class: testing specialist only', () => { + test('tests class: runtime 0 is light, core only (Behavior changes: no Testing specialist)', () => { const p = planReview(base({ paths: ['test/a.test.ts'], scope: { SCOPE_TESTS: 'true' } })); - expect(roles(p)).toEqual(['specialist:testing:full']); + expect(p.depth).toBe('light'); + expect(roles(p)).toEqual([]); }); test('code class pass 1: every reviewer at full effort, the outside voice at its own', () => { const p = planReview(base({})); @@ -88,7 +92,7 @@ describe('planReview', () => { }); test('code class honours hit-rate gating and small-diff skip', () => { expect(roles(planReview(base({ gated: ['simplification'] })))).not.toContain('specialist:simplification:full'); - expect(planReview(base({ diffLines: 20 })).dispatches.filter(d => d.role === 'specialist')).toEqual([]); + expect(planReview(base({ diffLines: 20 })).dispatches.filter(d => d.role === 'specialist').map(d => d.name)).toEqual(['batched']); }); test('pass 2 reruns the army on the range but not the adversarial or outside voice once Step 11 ran', () => { const p = planReview(base({ adversarialRan: true, history: { fullPasses: 1, last: 'full', fixedBlocking: true } })); @@ -167,3 +171,121 @@ describe('GSTA-131 coverage: Red Team and specialist arms', () => { expect(roles(planReview(base({ paths: ['docs/a.md'], force: ['security'] })))).toEqual(['specialist:security']); }); }); + +const per = (o: Record) => Object.entries(o).map(([p, lines]) => ({ path: p, lines, binary: false })); +const mixed = (o: Partial = {}) => base({ paths: Object.keys(o.perPath ? Object.fromEntries(o.perPath.map(e => [e.path, 1])) : {}), ...o }); +const err = (code: DepthError['code'], result?: DepthError['result']): DepthError => ({ code, detail: 'd', input: 'i', repair: 'r', ...(result ? { result } : {}) }); + +describe('review depth: the matrix', () => { + test('runtime 0 (tests and docs only) is light: core only, nothing dispatched', () => { + const p = planReview(mixed({ perPath: per({ 'test/a.test.ts': 300, 'docs/a.md': 40 }), diffLines: 340 })); + expect([p.depth, p.runtimeLines, p.dispatches]).toEqual(['light', 0, []]); + }); + test('runtime 1-50 is light plus one batched specialist: the fixed set, scope picks and the Red Team in one context', () => { + const p = planReview(mixed({ perPath: per({ 'src/app.ts': 48, 'test/a.test.ts': 190, 'docs/a.md': 9 }), diffLines: 247 })); + expect([p.depth, p.runtimeLines]).toEqual(['light', 48]); + expect(p.dispatches).toEqual([ + { role: 'specialist', name: 'batched', effort: 'full', exclude: [':(exclude)docs/a.md'], checklists: ['testing', 'maintainability', 'security', 'performance', 'red-team'] }, + { role: 'adversarial-native', name: 'adversarial', effort: 'full', exclude: [], reason: 'backtest inconclusive (GSTA-147 D5)' }, + ]); + expect(p.skipped.join('; ')).toContain('red-team (light depth'); + expect(planReview(mixed({ perPath: per({ 'src/app.ts': 48 }), adversarialRan: true, history: { fullPasses: 1, last: 'full', touchedGate: true, fixedBelow: true } })).dispatches.some(d => d.role === 'adversarial-native')).toBe(false); + }); + test('runtime 51 is standard: thresholds read runtime lines, reviewers read their own hunks', () => { + const p = planReview(mixed({ perPath: per({ 'src/app.ts': 51, 'test/a.test.ts': 400, 'README.md': 3 }), diffLines: 454 })); + expect([p.depth, p.runtimeLines]).toEqual(['standard', 51]); + expect(roles(p)).toEqual(['specialist:testing:full', 'specialist:maintainability:full', 'specialist:performance:full', 'adversarial-native:adversarial:full', 'outside-voice:outside-voice:host']); + const ex = Object.fromEntries(p.dispatches.map(d => [d.name, d.exclude])); + expect(ex.testing).toEqual([':(exclude)README.md']); + expect(ex.maintainability).toEqual([':(exclude)README.md', ':(exclude)test/a.test.ts']); + expect([ex.adversarial, ex['outside-voice']]).toEqual([[], []]); + }); + test.each([ + ['scope error', { scope: { SCOPE_ERROR: 'boom' } }, 'scope_error', []], + ['gate scan unavailable', { gateTagged: null }, 'gate_scan_unavailable', []], + ['base registry invalid', { registry: { sha: 'abc', outputs: [], error: err('registry_invalid', 'REGEN_REGISTRY_INVALID') } }, 'registry_invalid', ['REGEN_REGISTRY_INVALID']], + ['base registry unavailable', { registry: { sha: null, outputs: [], error: err('registry_unavailable') } }, 'registry_unavailable', []], + ['numstat failure', { errors: [err('numstat_failed')] }, 'numstat_failed', []], + ['binary code path', { perPath: [...per({ 'src/app.ts': 5 }), { path: 'lib/x.wasm', lines: 0, binary: true }], paths: ['src/app.ts', 'lib/x.wasm'] }, 'binary_code_path', []], + ] as const)('%s fails closed to full with its code', (_n, o, code, codes) => { + const p = planReview(mixed({ perPath: per({ 'src/app.ts': 5 }), ...(o as Partial) })); + expect(p.depth).toBe('full'); + expect(p.errors.map(e => e.code)).toContain(code); + expect(p.codes).toEqual([...codes]); + expect(roles(p)).toContain('red-team:red-team:full'); + }); + test.each([ + ['a gate path', { paths: ['src/app.ts', '.github/workflows/ci.yml'] }], + ['a gate tag', { gateTagged: 1 }], + ['a deleted test', { deleted: ['test/old.test.ts'] }], + ['an edit to the registry', { paths: ['.gstack/generated.json'] }], + ['an edit to SMALL_DIFF_LINES', { paths: ['lib/review-depth.ts'] }], + ['the collector', { paths: ['lib/review-collect.ts'] }], + ['the plan CLI', { paths: ['bin/gstack-review-plan'] }], + ['the queue', { paths: ['lib/ship-queue-train.ts'] }], + ['the Step 9 template', { paths: ['ship/sections/review-army.md.tmpl'] }], + ] as const)('%s is full', (_n, o) => { + const paths = (o as any).paths ?? ['src/app.ts']; + expect(planReview(mixed({ perPath: per(Object.fromEntries(paths.map((p: string) => [p, 2]))), ...(o as Partial) })).depth).toBe('full'); + }); + test('a registered glob output is excluded from runtime; a .tmpl edit plus its generated SKILL.md counts the tmpl only', () => { + const p = planReview(mixed({ perPath: per({ 'ship/SKILL.md.tmpl': 30, 'ship/SKILL.md': 200 }), registry: { sha: 'abc', outputs: ['*/SKILL.md'] } })); + expect([p.depth, p.runtimeLines]).toEqual(['light', 30]); + expect(p.dispatches[0]!.exclude).toEqual([':(exclude)ship/SKILL.md']); + }); + test('a path no base-registry entry names stays runtime', () => { + expect(planReview(mixed({ perPath: per({ 'src/gen.ts': 60 }), registry: { sha: 'abc', outputs: ['lib/dom.js'] } })).runtimeLines).toBe(60); + }); + test('numbers in light dispatch the outside voice with an empty exclude', () => { + const p = planReview(mixed({ perPath: per({ 'docs/bench.md': 4 }), numbers: scanNumbers({ 'docs/bench.md': ['3x faster'] }) })); + expect(p.depth).toBe('light'); + expect(p.dispatches).toEqual([{ role: 'outside-voice', name: 'outside-voice', effort: 'host', exclude: [] }]); + }); + test('a fixed blocker in light raises the next full step to the standard set, Red Team included', () => { + const p = planReview(mixed({ perPath: per({ 'src/app.ts': 30 }), history: { fullPasses: 1, last: 'full', fixedBlocking: true } })); + expect([p.depth, p.stepDepth]).toEqual(['light', 'standard']); + expect(roles(p)).toEqual(['specialist:testing:full', 'specialist:maintainability:full', 'specialist:performance:full', 'red-team:red-team:full', 'adversarial-native:adversarial:full', 'outside-voice:outside-voice:host']); + }); + test('--depth-floor raises and never lowers', () => { + expect(depthFor({ cls: 'code', runtimeLines: 10, errors: [], floor: 'full' })).toBe('full'); + expect(depthFor({ cls: 'code', runtimeLines: 10, errors: [], floor: 'standard' })).toBe('standard'); + expect(depthFor({ cls: 'gate', runtimeLines: 10, errors: [], floor: 'standard' })).toBe('full'); + expect(depthFor({ cls: 'code', runtimeLines: 80, errors: [] })).toBe('standard'); + const p = planReview(mixed({ perPath: per({ 'src/app.ts': 10 }), floor: 'full' })); + expect(p.depth).toBe('full'); + expect(p.depthReasons).toContain('floor:full'); + }); + test('depthOfCommitInput gives the same depth as the plan for the same input', () => { + const i = { paths: ['src/app.ts', 'test/a.test.ts'], perPath: per({ 'src/app.ts': 51, 'test/a.test.ts': 9 }), gateTagged: 0 }; + expect(depthOfCommitInput(i)).toMatchObject({ depth: 'standard', runtimeLines: 51, rules: DEPTH_RULES, errors: [] }); + expect(depthOfCommitInput({ ...i, gateTagged: null }).depth).toBe('full'); + }); +}); + +describe('review depth: diff commands and the rules hash', () => { + test('DIFF_CMD shell-quotes each pathspec; a glob character makes the pathspec literal', () => { + const p = planReview(mixed({ perPath: per({ 'src/app.ts': 10, "docs/my notes*.md": 3, "docs/it's.md": 1 }) })); + const ex = p.dispatches[0]!.exclude; + expect(ex).toEqual([':(exclude,literal)docs/my notes*.md', ":(exclude)docs/it's.md"]); + expect(diffCommand(p.dispatches[0]!)).toBe("git diff -- . ':(exclude,literal)docs/my notes*.md' ':(exclude)docs/it'\\''s.md'"); + expect(diffCommand({ role: 'red-team', name: 'red-team', effort: 'full', exclude: [] })).toBe('git diff '); + }); + test('DEPTH_RULES hashes the deciding constants: a comment-only edit keeps it, a threshold edit changes it', () => { + expect(DEPTH_RULES).toMatch(/^[0-9a-f]{16}$/); + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'depth-rules-')); + try { + const src = fs.readFileSync(path.join(ROOT, 'lib/review-depth.ts'), 'utf8'); + fs.symlinkSync(path.join(ROOT, 'lib/ship-policy.ts'), path.join(dir, 'ship-policy.ts')); + const hashOf = (text: string) => { + fs.writeFileSync(path.join(dir, 'review-depth.ts'), text); + fs.writeFileSync(path.join(dir, 'print.ts'), "import { DEPTH_RULES } from './review-depth';\nconsole.log(DEPTH_RULES);\n"); + const r = spawnSync(process.execPath, ['--no-env-file', '--config=/dev/null', path.join(dir, 'print.ts')], { encoding: 'utf8', timeout: 30_000 }); + expect(r.status, r.stderr).toBe(0); + return r.stdout.trim(); + }; + expect(hashOf('// a comment-only edit\n' + src)).toBe(DEPTH_RULES); + expect(hashOf(src.replace('SMALL_DIFF_LINES = 50', 'SMALL_DIFF_LINES = 60'))).not.toBe(DEPTH_RULES); + } finally { fs.rmSync(dir, { recursive: true, force: true }); } + }); +}); + diff --git a/test/review-dispatch-compare.test.ts b/test/review-dispatch-compare.test.ts index 804d9e0cf..2771d0a95 100644 --- a/test/review-dispatch-compare.test.ts +++ b/test/review-dispatch-compare.test.ts @@ -34,7 +34,7 @@ describe('scripts/review-dispatch-compare.ts', () => { const table = run(['--tokens', tok]); expect(table).toContain(`TOKENS: measured (${tok})`); expect(table).toMatch(/^SHIP_TOKEN_RATIO: median=0\.\d+ min=0\.\d+ max=\d\.\d+ \(n=10\)$/m); - expect(table).toMatch(/^SHIP_TOKEN_RATIO_BY_CLASS: docs median=0\.\d+ \(n=5\); code median=0\.\d+ \(n=5\)$/m); + expect(table).toMatch(/^SHIP_TOKEN_RATIO_BY_CLASS: docs median=0\.\d+ \(n=5\); gate median=0\.\d+ \(n=5\)$/m); } finally { fs.rmSync(dir, { recursive: true, force: true }); } diff --git a/test/review-plan-cli.test.ts b/test/review-plan-cli.test.ts index c8aedb90e..ba8d77d9b 100644 --- a/test/review-plan-cli.test.ts +++ b/test/review-plan-cli.test.ts @@ -109,3 +109,51 @@ describe('gstack-review-plan', () => { expect(ran).toContain('SKIP: adversarial-native and outside-voice (run once per invocation)'); }); }); + +describe('gstack-review-plan: review depth', () => { + const code = (n: number) => Array.from({ length: n }, (_, i) => `export const v${i} = ${i};`).join('\n') + '\n'; + test('prints REVIEW_DEPTH with runtime lines and the rules hash, and GATE_SCAN bound to the head', () => { + const r = repo({ 'src/a.ts': code(10), 'test/a.test.ts': code(200) }); + const out = run(r, []).stdout; + const head = spawnSync('git', ['rev-parse', 'HEAD'], { cwd: r.dir, encoding: 'utf8', timeout: 10_000 }).stdout.trim(); + expect(out).toMatch(/^REVIEW_DEPTH: light runtime_lines=10 rules=[0-9a-f]{16} reasons=runtime=10<=50 codes=none$/m); + expect(out).toContain(`GATE_SCAN: tagged=0 head=${head}`); + expect(out).toContain('DISPATCH: specialist batched effort=full checklists=testing,maintainability,security,performance,state-machines,red-team'); + expect(out).toContain('DISPATCH: adversarial-native adversarial effort=full reason=backtest inconclusive (GSTA-147 D5)'); + }); + test('DIFF_CMD is shell-quoted per dispatch and excludes what the reviewer does not read; DIFF_EXCLUDE: none when empty', () => { + const r = repo({ 'src/a.ts': code(80), 'docs/my notes*.md': '# n\n', 'test/a.test.ts': code(3) }); + const out = run(r, []).stdout; + expect(out).toContain("DIFF_CMD: testing git diff -- . ':(exclude,literal)docs/my notes*.md'"); + expect(out).toContain("DIFF_CMD: maintainability git diff -- . ':(exclude,literal)docs/my notes*.md' ':(exclude)test/a.test.ts'"); + expect(out).toContain('DIFF_CMD: adversarial git diff \nDIFF_EXCLUDE: adversarial none'); + const cmd = /^DIFF_CMD: maintainability (.*)$/m.exec(out)![1]!.replace('', 'main'); + const diff = spawnSync('bash', ['-c', cmd.replace('git diff main', 'git diff --name-only main')], { cwd: r.dir, encoding: 'utf8' }); + expect(diff.stdout.trim().split('\n')).toEqual(['src/a.ts']); + }); + test('--json carries depth, runtime_lines, rules, errors, gate_scan and every dispatch\'s exclude array', () => { + const r = repo({ 'src/a.ts': code(80), 'README.md': '# y\n' }); + const plan = JSON.parse(run(r, ['--json']).stdout); + expect(plan).toMatchObject({ depth: 'standard', step_depth: 'standard', runtime_lines: 80, errors: [], codes: [], gate_scan: { tagged: 0 } }); + expect(plan.rules).toMatch(/^[0-9a-f]{16}$/); + expect(plan.dispatches.find((d: any) => d.name === 'maintainability').exclude).toEqual([':(exclude)README.md']); + expect(plan.dispatches.find((d: any) => d.role === 'outside-voice').exclude).toEqual([]); + }); + test('--depth-floor full raises a light diff, never lowers a full one, and an invalid floor is a usage error', () => { + const r = repo({ 'src/a.ts': code(5) }); + expect(run(r, ['--depth-floor', 'full']).stdout).toMatch(/^REVIEW_DEPTH: full runtime_lines=5 .*reasons=runtime=5<=50,floor:full /m); + const gate = repo({ 'src/a.ts': code(5), '.github/workflows/ci.yml': 'on: push\n' }); + expect(run(gate, ['--depth-floor', 'standard']).stdout).toMatch(/^REVIEW_DEPTH: full /m); + expect(run(r, ['--depth-floor', 'light']).status).toBe(2); + }); + test('a conservative full plan exits 0 with its errors; a plan that cannot be built exits 1', () => { + const r = repo({ 'lib/x.wasm': '\u0000asm\u0000' }); + const res = run(r, ['--json']); + expect(res.status).toBe(0); + expect(JSON.parse(res.stdout)).toMatchObject({ depth: 'full', errors: expect.arrayContaining([expect.objectContaining({ code: 'binary_code_path', input: 'lib/x.wasm' })]) }); + expect(run(r, []).stdout).toContain('DEPTH_ERROR: binary_code_path input=lib/x.wasm'); + const none = spawnSync(BIN, ['--base', 'no-such-branch', '--gate-tagged', '0'], { cwd: r.dir, encoding: 'utf8', timeout: 60_000, env: { ...process.env, GSTACK_STATE_ROOT: r.state } }); + expect(none.status).toBe(1); + }); +}); + diff --git a/test/ship-receipt-depth.test.ts b/test/ship-receipt-depth.test.ts new file mode 100644 index 000000000..1a7e2e8fd --- /dev/null +++ b/test/ship-receipt-depth.test.ts @@ -0,0 +1,207 @@ +/** + * 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); + + // Value: protects=the queue's required depth keeps the scope signals the writer used, and a gate scan binds only the full + // bound commit; fails_when=the queue recomputes without diff-scope (auth reads light) or binds an empty or short scan head + // by prefix; seam=bindShippedReceipt + test('an auth path needs full at the queue though the receipt says light; an empty or short gate scan head binds nothing', () => { + const repo = makeFixtureRepo(); repos.push(repo); + const head = commit(repo, { 'lib/auth.js': lines(3) }); + const body = path.join(repo.root, 'body.md'); + expect(writeReceipt(repo, body).status).toBe(EXIT.ok); + expect(receiptIn(body).review_depth).toMatchObject({ depth: 'full', class: 'gate' }); + const bind = (text: string) => bindShippedReceipt(repo.work, 'origin/main', text, '7', [head]); + const light = bind(rewrite(body, r => { r.review_depth.depth = 'light'; })); + expect(!light.ok && light.code).toBe('QUEUE_RECEIPT_DEPTH_RAISED'); + expect(!light.ok && light.depthLine).toContain('receipt=light required=full'); + expect(!light.ok && light.depthLine).toContain('class=gate'); + + const plain = makeFixtureRepo(); repos.push(plain); + const plainHead = commit(plain, { 'lib/util.js': lines(3) }); + const plainBody = path.join(plain.root, 'body.md'); + expect(writeReceipt(plain, plainBody).status).toBe(EXIT.ok); + for (const scanHead of ['', plainHead.slice(0, 12)]) { + const r = bindShippedReceipt(plain.work, 'origin/main', rewrite(plainBody, x => { x.review_depth.gate_scan.head = scanHead; }), '7', [plainHead]); + expect(r.ok).toBe(false); + expect(!r.ok && r.depthLine).toContain('required=full'); + expect(!r.ok && r.depthLine).toContain('gate_scan@missing'); + } + }, 90_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); + + // Value: protects=a carry never binds a scan of one checkout to another commit, and never rebinds a receipt the queue would + // refuse; fails_when=carry --head from another checkout carries a skipped test at light, or a current receipt without + // review_depth carries; seam=none + test('carry --head from another checkout refuses; a receipt with gstack_version but no review_depth refuses', () => { + const repo = makeFixtureRepo(); repos.push(repo); + const reviewed = commit(repo, { 'lib/util.js': lines(3), 'test/util.test.js': "test('a', () => {});\n" }); + const body = path.join(repo.root, 'body.md'); + expect(writeReceipt(repo, body).status).toBe(EXIT.ok); + const skipped = commit(repo, { 'test/util.test.js': "test.skip('a', () => {});\n" }); + repo.git('checkout', '-q', reviewed); + const r = runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--base', 'main', '--head', skipped]); + expect(r.status).toBe(EXIT.refused); + expect(r.err).toContain(`--head ${skipped.slice(0, 12)} is not the checked-out HEAD ${reviewed.slice(0, 12)}`); + expect(receiptIn(body).head).toBe(reviewed); + repo.git('checkout', '-q', skipped); + rewrite(body, x => { delete x.review_depth; }); + const noDepth = runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--base', 'main']); + expect(noDepth.status).toBe(EXIT.refused); + expect(noDepth.err).toContain('records gstack_version but no review_depth'); + }, 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'); + const foreign = { ...exception, fix_check: { head: 'f'.repeat(40), findings: 1 } }; + const forged = rewrite(body, x => { x.review_loop_exception = foreign; }).replace(line, reviewLoopExceptionLine(foreign)); + const offBranch = bindShippedReceipt(repo.work, 'origin/main', forged, '7', [head]); + expect(!offBranch.ok && offBranch.code).toBe('QUEUE_RECEIPT_EXCEPTION_INVALID'); + expect(!offBranch.ok && offBranch.message).toContain('is not the receipt head or an ancestor of it'); + }, 90_000); +}); diff --git a/test/size-ledger/ship/gsta147-review-depth.json b/test/size-ledger/ship/gsta147-review-depth.json new file mode 100644 index 000000000..f583f9ecc --- /dev/null +++ b/test/size-ledger/ship/gsta147-review-depth.json @@ -0,0 +1,4 @@ +{ + "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)" +}