diff --git a/bin/gstack-doctor-components.sh b/bin/gstack-doctor-components.sh index 960129724..6dba8bf82 100644 --- a/bin/gstack-doctor-components.sh +++ b/bin/gstack-doctor-components.sh @@ -326,6 +326,7 @@ gstack_result_anchor() { REGEN_DEPS_MISSING) printf '%s' 'regen-deps-missing' ;; REVIEW_SCOPE_INPUT) printf '%s' 'review-scope-input' ;; REVIEW_SCOPE_RANGE) printf '%s' 'review-scope-range' ;; + REVIEW_SCOPE_LEDGER) printf '%s' 'review-scope-ledger' ;; 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' ;; diff --git a/bin/gstack-review-scope b/bin/gstack-review-scope index fb2f2388e..8f071c17c 100755 --- a/bin/gstack-review-scope +++ b/bin/gstack-review-scope @@ -5,7 +5,7 @@ * gstack-review-scope classify --base --pass [--range-base ] [--policy frozen|diff-plus-blocking|all] [--findings |-] [--json] * gstack-review-scope defer --findings [--pr ] [--repo ] [--branch ] [--out ] [--dry-run] [--json] * gstack-review-scope render --deferred [--findings ] - * gstack-review-scope summary --since [--out ] [--json] + * gstack-review-scope summary --since (--run | --ledger ... | --no-ledger) [--out ] [--json] * * `classify` reads a JSON array of findings (the Step 9.3 result) and stamps * each with `locus` (diff | fix-range | outside), `kind` (defect | test-gap | @@ -30,6 +30,7 @@ import { parseHunks, renderDeferredIssue, renderDeferredSection, reviewLoopLine, scopeFindings, scopeLine, summarizeReviewLoop, ALREADY_FILED_REASON, DEFERRED_MARKER, findingIdentity, findingWhere, REVIEW_SCOPE_POLICIES, type DeferredRecord, type ReviewScopePolicy, type ScopeFinding, type ScopedFinding, } from '../lib/review-scope'; +import { ledgerBlockers } from '../lib/review-ledger'; const HELP = `gstack-review-scope — finding scope for /ship's review loop (docs/ship-policy.md#review-scope) @@ -42,8 +43,12 @@ const HELP = `gstack-review-scope — finding scope for /ship's review loop (doc render --deferred [--findings ] the PR-body sections: ## Deferred findings (references, never closing keywords) and ## Review notes (scope=note findings) - summary --since [--out ] the invocation's passes from this branch's review log: REVIEW_LOOP: passes=… - blocking_found=… deferred=… noted=… (the ship receipt's --review-loop input) + summary --since (--run | --ledger ... | --no-ledger) [--out ] + the invocation's passes from this branch's review log: REVIEW_LOOP: passes=… + blocking_found=… deferred=… noted=… (the ship receipt's --review-loop input); + blocking_found also counts every CRITICAL the dispatch ledgers collected or judged + (--run finds each review-ledger.jsonl under the run directory); unrecorded=N names + blockers no row gives a disposition. An unreadable ledger: REVIEW_SCOPE_LEDGER Defaults: --policy from the base branch's .gstack/ship-policy.json review_scope (frozen); an unresolvable base refuses (POLICY_SOURCE_UNAVAILABLE) unless --policy is passed; --findings - reads stdin; --branch the current branch. @@ -117,7 +122,7 @@ function readFindings(spec: string | undefined, what: string): ScopeFinding[] { if (!list || !list.every((f: unknown) => f && typeof f === 'object' && !Array.isArray(f))) throw new Error(`${what}: expected a JSON array of finding objects`); return list as ScopeFinding[]; } -function refuse(cmd: string, code: 'REVIEW_SCOPE_INPUT' | 'REVIEW_SCOPE_RANGE', detail: string): number { +function refuse(cmd: string, code: 'REVIEW_SCOPE_INPUT' | 'REVIEW_SCOPE_RANGE' | 'REVIEW_SCOPE_LEDGER', detail: string): number { process.stderr.write(resultLineWithDocs(`gstack-review-scope ${cmd}`, code, detail) + '\n'); return EXIT.fail; } @@ -245,7 +250,26 @@ function cmdSummary(rest: string[], json: boolean): number { if (!line.trim() || line === 'NO_REVIEWS') continue; try { rows.push(JSON.parse(line)); } catch { /* malformed row: gstack-review-read already warned */ } } - const summary = summarizeReviewLoop(rows, since); + const files: string[] = []; + for (let i = 0; i < rest.length; i++) { + if (rest[i] !== '--ledger') continue; + if (!rest[i + 1] || rest[i + 1]!.startsWith('--')) return refuse('summary', 'REVIEW_SCOPE_INPUT', '--ledger needs a file'); + files.push(rest[++i]!); + } + const run = flag(rest, '--run'); + if (run) { + let found: string[]; + try { found = (fs.readdirSync(run, { recursive: true }) as string[]).filter(p => path.basename(p) === 'review-ledger.jsonl').sort().map(p => path.join(run, p)); } catch (e: any) { return refuse('summary', 'REVIEW_SCOPE_LEDGER', `--run ${run}: ${e.code ?? e.message}`); } + if (!found.length) return refuse('summary', 'REVIEW_SCOPE_LEDGER', `--run ${run} holds no review-ledger.jsonl: name the run directory the passes wrote, or each --ledger`); + files.push(...found); + } + const noLedger = rest.includes('--no-ledger'); + if (noLedger && files.length) return refuse('summary', 'REVIEW_SCOPE_INPUT', '--no-ledger contradicts --run and --ledger'); + // The ledgers hold every reader's findings, the voices' too, before any row classifies them; a summary without them can undercount. + if (!noLedger && !files.length) return refuse('summary', 'REVIEW_SCOPE_LEDGER', 'name the review\'s dispatch ledgers (--run or --ledger ), or --no-ledger when no reader ran through gstack-review-dispatch'); + let ledgers: string[][]; + try { ledgers = files.map(ledgerBlockers); } catch (e: any) { return refuse('summary', 'REVIEW_SCOPE_LEDGER', e.message); } + const summary = summarizeReviewLoop(rows, since, ledgers); const line = reviewLoopLine(summary); // A /ship invocation always runs Step 9 once; zero matching rows means the wrong --since or a review log written elsewhere, never a loop to record. if (summary.passes === 0) return refuse('summary', 'REVIEW_SCOPE_INPUT', `no review-log row of this branch is a via:ship review at or after ${since}; check REVIEW_SINCE (${line})`); diff --git a/changelog.d/gst-951-receipt-counts-blockers.md b/changelog.d/gst-951-receipt-counts-blockers.md new file mode 100644 index 000000000..4a1fbc0c8 --- /dev/null +++ b/changelog.d/gst-951-receipt-counts-blockers.md @@ -0,0 +1,2 @@ +- **The /ship receipt counts every blocker its readers found** (GST-951). `gstack-review-scope summary` read only the review-log rows, but /ship's readers report through the dispatch ledgers (`gstack-review-dispatch collect` and `judge`). So a blocker that only the native adversarial pass raised, a CRITICAL logged fixed without classify's blocking stamp, and findings logged before they were classified all left `blocking_found` at 0. That happened on #3841, #3838 and #3837. The summary now also reads each ledger's collected and judged CRITICAL findings, takes the larger count per reviewer so a blocker held in both places counts once, and counts an unclassified CRITICAL row as blocking. `unrecorded=N` names blockers no row gives a disposition, and `ledgers=N` says how many ledgers it read. +- **The summary refuses rather than undercount.** It now needs `--run ` (it finds every `review-ledger.jsonl` under it), `--ledger `, or `--no-ledger` when no reader ran through `gstack-review-dispatch`. A missing choice, a run directory with no ledger, or an unreadable ledger, merged file or judged set, or a judged set changed since `judge`, is `REVIEW_SCOPE_LEDGER` with nothing written. /ship on Capy (`ship/DRIVER.md`) passes `--run`; the Claude Code flow passes `--no-ledger`. diff --git a/docs/review-policy.md b/docs/review-policy.md index 51182ed28..cde94b9a8 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -387,14 +387,17 @@ narrowed to the gate scan's tagged hunks. ## What the receipt records -`gstack-review-scope summary --since ` reads this invocation's -review-log rows and writes the counters `gstack-ship-receipt write --review-loop` +`gstack-review-scope summary --since --run ` reads this invocation's +review-log rows and the dispatch ledgers under the run directory (`--ledger ` names one; +`--no-ledger` when no reader ran through `gstack-review-dispatch`) and writes the counters `gstack-ship-receipt write --review-loop` puts in the receipt: | Field | Meaning | |---|---| | `review_loop.passes` | full review passes this ship ran (1 or 2; up to 3 under `review_scope: all`) | -| `review_loop.blocking_found` | distinct blocking findings across the passes | +| `review_loop.blocking_found` | every blocker found (GST-951): per reviewer, the larger of the rows' blocking findings (classified, or an unclassified CRITICAL) and the ledgers' CRITICAL findings as collected or judged, so an adversarial, outside-voice or not-yet-classified blocker counts. An unreadable ledger, merged file or judged set, or a judged set changed since `judge`, is `REVIEW_SCOPE_LEDGER`, never a count | +| `review_loop.unrecorded` | blockers found that no row gives a disposition (fixed, deferred, noted, skipped, a gate answer); absent when 0 | +| `review_loop.ledgers` | dispatch ledgers the count read; absent under `--no-ledger` | | `review_loop.deferred` | blocking findings in unchanged code filed as issues (a Step 11 late defer, logged as a `review-late-defer` row so it never displaces the newest `review` pass, counts here and is not a pass) | | `review_loop.noted` | below-floor findings listed in the PR body | | `review_loop.fixed` | findings fixed in this PR | diff --git a/docs/ship-policy.md b/docs/ship-policy.md index 26f3ef71e..016af873d 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -1268,11 +1268,11 @@ construction: the run and names the finding (path, severity, problem) for the owner instead of a third round; the owner may grant one scoped fix-check. -`gstack-review-scope summary --since ` reads the branch's review-log -rows for this invocation and writes the counters the ship receipt carries +`gstack-review-scope summary --since --run ` reads the branch's review-log +rows for this invocation and its dispatch ledgers, and writes the counters the ship receipt carries (`gstack-ship-receipt write … --review-loop @`): `review_loop: { passes, -blocking_found, deferred, noted, fixed, scope }`, plus `fix_checks`, `dispatches` -and `est_input_tokens` when the run recorded them, printed on the receipt summary +blocking_found, deferred, noted, fixed, scope }`, plus `fix_checks`, `dispatches`, +`est_input_tokens`, `unrecorded` and `ledgers` when the run recorded them, printed on the receipt summary line as `passes= blocking_found= deferred=`, so a scorecard can track passes per PR. diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index d73967bb1..ecb64d3f1 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -5646,6 +5646,24 @@ pass). the invocation record, and start a full pass (`--pass 1`) when the branch was rebased; /ship's own rule is to merge the base in, never rebase a reviewed head. + +### `REVIEW_SCOPE_LEDGER` + +**Meaning.** `gstack-review-scope summary` could not count the review's +dispatch ledgers, so it wrote no `REVIEW_LOOP:` line rather than a count that +might be low (GST-951). Either no ledger was named (`--run`, `--ledger` or +`--no-ledger`), `--run` holds no `review-ledger.jsonl`, or a ledger, the +`merged.jsonl` it names or its judged set is unreadable, or the judged set +changed after `judge` checked it. The ledgers hold every reader's findings +(specialists, Red Team, both adversarial voices, targeted, fix-check) before any +review-log row classifies them, which is why the count reads them. + +**Fix.** Pass `--run `, the directory the review passes wrote (on +Capy, `ship/DRIVER.md`'s run directory), or each `--ledger `. Pass +`--no-ledger` only when no reader ran through `gstack-review-dispatch`. When a +file moved, restore it; when a judged set changed, rerun `gstack-review-dispatch +judge` on it. + ### `RECEIPT_CARRY_REFUSED` diff --git a/docs/unattended.md b/docs/unattended.md index 822f38e73..5946ce12f 100644 --- a/docs/unattended.md +++ b/docs/unattended.md @@ -172,7 +172,8 @@ unpushed head is `QUEUE_RECEIPT_STALE` to the queue. The receipt's `review_loop` object, written from `gstack-review-scope summary --since `, is the loop's machine-readable outcome: `passes` (Step 9 full passes this ship ran, 1 or 2; up to 3 under `review_scope: all`), -`blocking_found` (distinct blocking findings across them), `deferred` +`blocking_found` (every blocker the readers found, rows and dispatch ledgers +both, [review-policy.md](review-policy.md#what-the-receipt-records)), `deferred` (blocking findings in unchanged code filed as issues, Step 11's late defers included), `noted`, `fixed` and `scope`. The summary line a parent reads prints it as `passes= blocking_found= diff --git a/lib/headless-artifacts.ts b/lib/headless-artifacts.ts index 433bec17b..355a51cf5 100644 --- a/lib/headless-artifacts.ts +++ b/lib/headless-artifacts.ts @@ -106,7 +106,7 @@ export interface ShipReceipt { // Related open PRs by other authors (Lane D): `supersedes` / `credit` only on the owner's confirmation. issues?: { fixes: number[]; refs: number[]; flakes: Array<{ issue: number; run: string }>; related?: Array<{ pr: number; repo?: string; author: string; closes: number[]; supersedes?: boolean; credit?: boolean }> }; /** Harness lane E: the review loop's counters (gstack-review-scope summary) and the carry record when the receipt followed a head whose product diff was unchanged (gstack-ship-receipt carry). */ - review_loop?: { passes: number; blocking_found: number; deferred: number; noted?: number; fixed?: number; scope?: string; fix_checks?: number; rounds?: number; dispatches?: Record; est_input_tokens?: number }; + review_loop?: { passes: number; blocking_found: number; deferred: number; noted?: number; fixed?: number; scope?: string; fix_checks?: number; rounds?: number; dispatches?: Record; est_input_tokens?: number; unrecorded?: number; ledgers?: number }; 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; diff --git a/lib/result-codes-release.ts b/lib/result-codes-release.ts index 3797aeae2..f85c80bc0 100644 --- a/lib/result-codes-release.ts +++ b/lib/result-codes-release.ts @@ -360,6 +360,11 @@ export const RELEASE_RESULT_CODES = { summary: 'gstack-review-scope could not read the diff it scopes findings against: the base or the range base does not resolve, or the range base is not an ancestor of HEAD', fix: 'fetch origin/, check --range-base against the saved pass heads, and start a full pass (--pass 1) when history was rewritten', }, + REVIEW_SCOPE_LEDGER: { + anchor: 'review-scope-ledger', + summary: 'gstack-review-scope summary could not count the dispatch ledgers: none was named (no --run, --ledger or --no-ledger), --run holds no review-ledger.jsonl, or a ledger, its merged file or its judged set is unreadable or changed since judge', + fix: 'pass --run (the directory the review passes wrote) or each --ledger ; --no-ledger only when no reader ran through gstack-review-dispatch; restore or re-collect a ledger file that moved', + }, RECEIPT_CARRY_REFUSED: { anchor: 'receipt-carry-refused', summary: 'gstack-ship-receipt carry could not carry the /ship receipt to HEAD: the PR body has no receipt, the working tree has uncommitted changes, the receipt head is not an ancestor of HEAD and the receipt records no tree to compare, or the product diff could not be compared', diff --git a/lib/review-ledger.ts b/lib/review-ledger.ts index cfbc91656..7e8aa54b1 100644 --- a/lib/review-ledger.ts +++ b/lib/review-ledger.ts @@ -236,6 +236,40 @@ export function recordJudgment(ledger: string, merged: MergedLine[], judged: Jud return res; } +/** + * The blocking findings a run's ledger holds (GST-951): every collected line whose label ranks CRITICAL, raised to its judged + * severity when the newest judge accepted a judged set that is unchanged since; lead judgment's additions count and a folded + * duplicate does not. Every reader counts the same way, whatever it is and whether or not Step 9.4 classified its findings. + * A ledger, merged file or judged set it cannot read, or a judged set changed since judge, throws: the count is never a guess. + */ +export function ledgerBlockers(ledger: string): string[] { + const fail = (why: string): never => { throw new Error(`${ledger}: ${why}`); }; + const read = (file: unknown, what: string): string => { + try { return fs.readFileSync(String(file), 'utf8'); } catch { return fail(`${what} ${String(file)} is unreadable`); } + }; + const jsonLines = (text: string, what: string): any[] => + text.split('\n').filter(l => l.trim()).map(l => { try { return JSON.parse(l); } catch { return fail(`${what} has a line that is not JSON`); } }); + const events = jsonLines(read(ledger, 'ledger'), 'the ledger') as LedgerEvent[]; + const ranks = new Map(); + for (const o of events.filter(e => e.event === 'output' && Array.isArray(e.ids) && e.ids.length)) { + const lines = jsonLines(read(o.merged, `merged file of ${String(o.reader)}`), String(o.merged)) as MergedLine[]; + for (const id of o.ids as string[]) { + const line = lines.find(l => l.id === id) ?? fail(`collected id ${id} is missing from ${String(o.merged)}`); + ranks.set(id, rank(line.label) ?? 0); + } + } + const verdict = [...events].reverse().find(e => e.event === 'judge'); + if (verdict?.ok === true) { + const text = read(verdict.judged, 'judged set'); + if (promptHash(text) !== verdict.judged_sha256) fail(`judged set ${String(verdict.judged)} changed since judge`); + for (const j of jsonLines(text, String(verdict.judged)) as JudgedLine[]) { + if (j.duplicate_of) ranks.delete(j.id); + else ranks.set(j.id, Math.max(ranks.get(j.id) ?? 0, rank(j.severity) ?? 0)); + } + } + return [...ranks].filter(([, r]) => r >= 4).map(([id]) => id); +} + /** * Whether a recorded output was a review: it has findings, or its file (unchanged since collect) carries the reader's clean marker, so * a ledger written before collect recorded the field is judged by the same rule; with no readable file, collect's own review field. diff --git a/lib/review-scope.ts b/lib/review-scope.ts index 833562591..43ead5e09 100644 --- a/lib/review-scope.ts +++ b/lib/review-scope.ts @@ -14,6 +14,7 @@ * Pure functions here; bin/gstack-review-scope runs git and gh. */ import { SPECIALISTS } from './review-depth'; +import { rank } from './review-ledger'; import { parseSpecialistOutput } from './state-matrix'; export type Locus = 'diff' | 'fix-range' | 'outside'; @@ -306,6 +307,10 @@ export interface ReviewLoopSummary { dispatches?: Record; /** Prompt plus diff bytes the dispatches read, / 4: a relative estimate, never a bill. */ est_input_tokens?: number; + /** Blocking findings no row gives a disposition (fixed, deferred, noted, skipped or a gate answer): found, outcome unrecorded. */ + unrecorded?: number; + /** Dispatch ledgers whose collected and judged findings were counted (gstack-review-dispatch). */ + ledgers?: number; } /** Review-log rows of one /ship invocation (skill review, via ship, at or after `since`) → the loop summary. */ @@ -348,12 +353,29 @@ function rowDispatches(row: Record): Array<{ role: string; bytes: n .map(([name]) => ({ role: name === 'red-team' ? 'red-team' : 'specialist', bytes: null })); } -export function summarizeReviewLoop(rows: readonly Record[], since: string): ReviewLoopSummary { +/** Row actions that record what happened to a finding. */ +const DISPOSITIONS = ['fixed', 'auto-fixed', 'deferred', 'noted', 'skipped', 'kept', 'restored', 'open']; +/** A row finding that blocks: classified blocking, or an unclassified CRITICAL defect (logged before Step 9.4's classify). */ +const rowBlocks = (f: Record) => f.blocking === true || (f.blocking === undefined && f.advisory !== true && rank(f.severity) === 4); +/** The reviewer a finding counts under: a row's raised_by or a ledger id's reader; every specialist is one group, as in the ledger. */ +function raiserGroup(raiser: unknown): string { + const role = typeof raiser === 'string' ? raiser.split(/[/#]/)[0]!.trim() : ''; + if (!role || role === 'unknown') return 'unattributed'; + return ['core', 'qa', 'red-team', 'adversarial-native', 'outside-voice', 'fix-check', 'targeted'].includes(role) ? role : 'specialist'; +} + +/** + * The loop summary. blocking_found counts every blocker found (GST-951): per reviewer group, the larger of the rows' + * blockers and the dispatch ledgers' CRITICAL findings (`ledgerBlockers`, one id list per ledger), so a blocker that only + * a ledger holds (an adversarial or outside-voice finding, a finding logged before classification) still counts. Two + * sources can each hold one the other lacks only when the rows name no reviewer; then the count may run high, never low. + */ +export function summarizeReviewLoop(rows: readonly Record[], since: string, ledgers: readonly (readonly string[])[] = []): ReviewLoopSummary { const sinceMs = Date.parse(since); const inRun = (r: Record) => r && r.via === 'ship' && Number.isFinite(rowTime(r)) && (!Number.isFinite(sinceMs) || rowTime(r) >= sinceMs); const loopRows = rows.filter(r => inRun(r) && (r.skill === 'review' || r.skill === LATE_DEFER_SKILL || r.skill === FIX_CHECK_SKILL)); const passes = loopRows.filter(r => r.skill === 'review'); - const blocking = new Set(); + const blocking = new Map(); const deferred = new Set(); const noted = new Set(); const fixed = new Set(); @@ -363,13 +385,25 @@ export function summarizeReviewLoop(rows: readonly Record[], since: for (const f of Array.isArray(row.findings) ? row.findings : []) { if (!f || typeof f !== 'object') continue; const id = findingIdentity(f); - if (f.blocking === true) blocking.add(id); + if (rowBlocks(f)) { + const b = blocking.get(id) ?? { group: raiserGroup(f.raised_by), disposed: false }; + if (DISPOSITIONS.includes(f.action)) b.disposed = true; + blocking.set(id, b); + } if (f.action === 'deferred') deferred.add(id); if (f.action === 'noted') noted.add(id); if (f.action === 'fixed' || f.action === 'auto-fixed') fixed.add(id); } } - const out: ReviewLoopSummary = { passes: passes.length, blocking_found: blocking.size, deferred: deferred.size, noted: noted.size, fixed: fixed.size, scope }; + const groups = new Map(); + const tally = (g: string) => groups.get(g) ?? groups.set(g, { rows: 0, disposed: 0, ledger: 0 }).get(g)!; + for (const b of blocking.values()) { const t = tally(b.group); t.rows++; if (b.disposed) t.disposed++; } + for (const id of ledgers.flat()) tally(raiserGroup(id)).ledger++; + let found = 0; let unrecorded = 0; + for (const t of groups.values()) { const n = Math.max(t.rows, t.ledger); found += n; unrecorded += n - t.disposed; } + const out: ReviewLoopSummary = { passes: passes.length, blocking_found: found, deferred: deferred.size, noted: noted.size, fixed: fixed.size, scope }; + if (unrecorded) out.unrecorded = unrecorded; + if (ledgers.length) out.ledgers = ledgers.length; const fixChecks = loopRows.filter(r => r.skill === FIX_CHECK_SKILL).length; if (fixChecks) out.fix_checks = fixChecks; out.rounds = passes.length + fixChecks; @@ -390,5 +424,6 @@ export function dispatchTotal(d: Record | undefined): number | u export function reviewLoopLine(s: ReviewLoopSummary): string { const total = dispatchTotal(s.dispatches); return `REVIEW_LOOP: passes=${s.passes} blocking_found=${s.blocking_found} deferred=${s.deferred} noted=${s.noted} fixed=${s.fixed} scope=${s.scope}` + + (s.unrecorded ? ` unrecorded=${s.unrecorded}` : '') + (s.ledgers ? ` ledgers=${s.ledgers}` : '') + (s.fix_checks ? ` fix_checks=${s.fix_checks}` : '') + (total !== undefined ? ` dispatches=${total}` : '') + (s.est_input_tokens !== undefined ? ` est_input_tokens=${s.est_input_tokens}` : ''); } diff --git a/lib/ship-receipt.ts b/lib/ship-receipt.ts index aba864ed5..4c58e0404 100644 --- a/lib/ship-receipt.ts +++ b/lib/ship-receipt.ts @@ -328,6 +328,8 @@ export function parseReviewLoop(value: unknown): ReviewLoopReceipt | undefined { if (n('fix_checks') !== undefined) out.fix_checks = n('fix_checks'); if (n('rounds') !== undefined) out.rounds = n('rounds'); if (n('est_input_tokens') !== undefined) out.est_input_tokens = n('est_input_tokens'); + if (n('unrecorded')) out.unrecorded = n('unrecorded'); + if (n('ledgers')) out.ledgers = n('ledgers'); if (v.dispatches && typeof v.dispatches === 'object' && !Array.isArray(v.dispatches)) { const d = Object.fromEntries(Object.entries(v.dispatches as Record) .filter(([k, c]) => /^[a-z][a-z0-9-]{0,39}$/.test(k) && typeof c === 'number' && Number.isInteger(c) && c >= 0).slice(0, 12)) as Record; diff --git a/ship/DRIVER.md b/ship/DRIVER.md index ac19dd0bd..549a005fd 100644 --- a/ship/DRIVER.md +++ b/ship/DRIVER.md @@ -14,7 +14,7 @@ gates and the report. `STOP:` (exit 3) means nothing after it ran; `NEXT:` names the next act. Read a section only when a phase or the list at the end sends you, never up front. A STOP or decision this file does not cover: read `ship/SKILL.md` at that step, finish it there, come back. Unattended, take a question's recommended option (log it); gate -dispositions and approvals stay pending and end the run. Keep a private invocation record (`--record`): write its six headings at Phase 1, then fill each as its phase ends: **Checks** +dispositions and approvals stay pending and end the run. Keep a private invocation record (`--record`): write its six headings at Phase 1 and fill each as its phase ends: **Checks** (Phases 2-4: lanes, pregate, evals, `Coverage:`, the plan checks, `Exploratory QA:`, `Scope Check:`), **Decisions** (any phase: each answered or auto-taken question, verbatim), **Reviews** (Phases 3-5: the plan line, start tokens, `Pre-Landing Review:`, `Gate edits:`, the reviewed tree), **Release** (Phases 5-7: @@ -28,7 +28,7 @@ file, unchanged; its findings reach `merged.jsonl` only via `collect`; `judge` r small model at low effort; their findings go to the PR body. **Calls.** One pass with no fixes is 9 calls: Phases 1-8 and the report; at most 12 with a STOP's -section read or a fix-check, plus a keep-alive per 200 s waited. A fix repeats Phases 3-4. +section read or a fix-check, plus a keep-alive per 200 s waited. Fixes redo 3-4. **Budget: observe.** `gstack-ship-budget start --mode observe` records, never admits: each program ends with `gstack-ship-budget note --step --reading R --credits C --record `, R and C read in that program @@ -119,7 +119,7 @@ STALE or MISSING reruns once), `PREGATE:`; print them, the last `Gate edits:` an ## 8. Publish: Steps 18, 19, 19-C, 20, 21, 22 - Step 18: the open-PR lookup; a title file starting `v `. `UNATTENDED_SHIP:` with no open PR at HEAD takes Step 18-H. -- Step 19 body: the receipt block, then the sections `sections/pr-body.md` lists (Pre-Landing Review: `REVIEW_LOOP:`, `Gate edits:`, deferred, notes), led under `CAPY_PR: on` (Step 19-C) by What changed, Why, Blast radius and Approvals (`gstack-approvals render`). `gstack-prose-lint --sections 'Why,Summary,Blast radius' --gate`; `gstack-issue-links`; `gstack-review-scope summary --since "$REVIEW_SINCE" && gstack-ship-receipt write --pr --base --gate @ --pregate --issues @ --review-loop @ --docs @ --body `; `gstack-post pr-body` and `pr-title` (a new PR via the host's PR tool; a draft while a gate finding is `open`). `SHIP_RECEIPT:` opens your report. +- Step 19 body: the receipt block, then the sections `sections/pr-body.md` lists (Pre-Landing Review: `REVIEW_LOOP:`, `Gate edits:`, deferred, notes), led under `CAPY_PR: on` (Step 19-C) by What changed, Why, Blast radius and Approvals (`gstack-approvals render`). `gstack-prose-lint --sections 'Why,Summary,Blast radius' --gate`; `gstack-issue-links`; `gstack-review-scope summary --since "$REVIEW_SINCE" --run && gstack-ship-receipt write --pr --base --gate @ --pregate --issues @ --review-loop @ --docs @ --body `; `gstack-post pr-body` and `pr-title` (a new PR via the host's PR tool; a draft while a gate finding is `open`). `SHIP_RECEIPT:` opens your report. - Step 20: the metrics row via `gstack-review-log`. - Step 22: `gstack-ship-budget close --reading R --credits C` (report `BUDGET_FINAL:`, `BUDGET_TIMELINE:` (tokens, credits), any `METER:` line), then `gstack-ship-receipt result --pr `: `GSTACK_RESULT: skill=ship status=complete run=`. Under a merge queue, `gstack-ship-queue ready --pr [--screen|--no-screen]` at the pushed head; paste its output. diff --git a/ship/DRIVER.md.tmpl b/ship/DRIVER.md.tmpl index 0298fb93e..7d4abf27d 100644 --- a/ship/DRIVER.md.tmpl +++ b/ship/DRIVER.md.tmpl @@ -12,7 +12,7 @@ gates and the report. `STOP:` (exit 3) means nothing after it ran; `NEXT:` names the next act. Read a section only when a phase or the list at the end sends you, never up front. A STOP or decision this file does not cover: read `ship/SKILL.md` at that step, finish it there, come back. Unattended, take a question's recommended option (log it); gate -dispositions and approvals stay pending and end the run. Keep a private invocation record (`--record`): write its six headings at Phase 1, then fill each as its phase ends: **Checks** +dispositions and approvals stay pending and end the run. Keep a private invocation record (`--record`): write its six headings at Phase 1 and fill each as its phase ends: **Checks** (Phases 2-4: lanes, pregate, evals, `Coverage:`, the plan checks, `Exploratory QA:`, `Scope Check:`), **Decisions** (any phase: each answered or auto-taken question, verbatim), **Reviews** (Phases 3-5: the plan line, start tokens, `Pre-Landing Review:`, `Gate edits:`, the reviewed tree), **Release** (Phases 5-7: @@ -26,7 +26,7 @@ file, unchanged; its findings reach `merged.jsonl` only via `collect`; `judge` r small model at low effort; their findings go to the PR body. **Calls.** One pass with no fixes is 9 calls: Phases 1-8 and the report; at most 12 with a STOP's -section read or a fix-check, plus a keep-alive per 200 s waited. A fix repeats Phases 3-4. +section read or a fix-check, plus a keep-alive per 200 s waited. Fixes redo 3-4. **Budget: observe.** `gstack-ship-budget start --mode observe` records, never admits: each program ends with `gstack-ship-budget note --step --reading R --credits C --record `, R and C read in that program @@ -117,7 +117,7 @@ STALE or MISSING reruns once), `PREGATE:`; print them, the last `Gate edits:` an ## 8. Publish: Steps 18, 19, 19-C, 20, 21, 22 - Step 18: the open-PR lookup; a title file starting `v `. `UNATTENDED_SHIP:` with no open PR at HEAD takes Step 18-H. -- Step 19 body: the receipt block, then the sections `sections/pr-body.md` lists (Pre-Landing Review: `REVIEW_LOOP:`, `Gate edits:`, deferred, notes), led under `CAPY_PR: on` (Step 19-C) by What changed, Why, Blast radius and Approvals (`gstack-approvals render`). `gstack-prose-lint --sections 'Why,Summary,Blast radius' --gate`; `gstack-issue-links`; `gstack-review-scope summary --since "$REVIEW_SINCE" && gstack-ship-receipt write --pr --base --gate @ --pregate --issues @ --review-loop @ --docs @ --body `; `gstack-post pr-body` and `pr-title` (a new PR via the host's PR tool; a draft while a gate finding is `open`). `SHIP_RECEIPT:` opens your report. +- Step 19 body: the receipt block, then the sections `sections/pr-body.md` lists (Pre-Landing Review: `REVIEW_LOOP:`, `Gate edits:`, deferred, notes), led under `CAPY_PR: on` (Step 19-C) by What changed, Why, Blast radius and Approvals (`gstack-approvals render`). `gstack-prose-lint --sections 'Why,Summary,Blast radius' --gate`; `gstack-issue-links`; `gstack-review-scope summary --since "$REVIEW_SINCE" --run && gstack-ship-receipt write --pr --base --gate @ --pregate --issues @ --review-loop @ --docs @ --body `; `gstack-post pr-body` and `pr-title` (a new PR via the host's PR tool; a draft while a gate finding is `open`). `SHIP_RECEIPT:` opens your report. - Step 20: the metrics row via `gstack-review-log`. - Step 22: `gstack-ship-budget close --reading R --credits C` (report `BUDGET_FINAL:`, `BUDGET_TIMELINE:` (tokens, credits), any `METER:` line), then `gstack-ship-receipt result --pr `: `GSTACK_RESULT: skill=ship status=complete run=`. Under a merge queue, `gstack-ship-queue ready --pr [--screen|--no-screen]` at the pushed head; paste its output. diff --git a/ship/sections/pr-body.md b/ship/sections/pr-body.md index 64e2f50cc..85178819c 100644 --- a/ship/sections/pr-body.md +++ b/ship/sections/pr-body.md @@ -220,7 +220,7 @@ per-lane counts (`{"":{"pass":N,"fail":N,"skip":N}}`) to ```bash rm -f .gstack/tmp/review-loop.json -~/.claude/skills/gstack/bin/gstack-review-scope summary --since "$REVIEW_SINCE" --out .gstack/tmp/review-loop.json && +~/.claude/skills/gstack/bin/gstack-review-scope summary --since "$REVIEW_SINCE" --no-ledger --out .gstack/tmp/review-loop.json && ~/.claude/skills/gstack/bin/gstack-ship-receipt write --pr --base --gate @.gstack/tmp/ship-gate.json --pregate .gstack/tmp/pregate.json --issues @.gstack/tmp/issues.json --review-loop @.gstack/tmp/review-loop.json --docs @.gstack/tmp/docs-audit.json --body "$PR_BODY_FILE" ``` @@ -233,7 +233,9 @@ When the owner accepted a stop after a fix-check edit, add 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 `&&` +`REVIEW_LOOP:` line is the one the Pre-Landing Review section carries). A review whose +readers ran through `gstack-review-dispatch` passes `--run ` instead of +`--no-ledger`, so their CRITICAL findings count. The `&&` matters: a summary that refuses (no pass matched `REVIEW_SINCE`) must not let `write` record a stale or empty loop file. diff --git a/ship/sections/pr-body.md.tmpl b/ship/sections/pr-body.md.tmpl index 8786e9daa..ca81552d4 100644 --- a/ship/sections/pr-body.md.tmpl +++ b/ship/sections/pr-body.md.tmpl @@ -203,7 +203,7 @@ per-lane counts (`{"":{"pass":N,"fail":N,"skip":N}}`) to ```bash rm -f .gstack/tmp/review-loop.json -~/.claude/skills/gstack/bin/gstack-review-scope summary --since "$REVIEW_SINCE" --out .gstack/tmp/review-loop.json && +~/.claude/skills/gstack/bin/gstack-review-scope summary --since "$REVIEW_SINCE" --no-ledger --out .gstack/tmp/review-loop.json && ~/.claude/skills/gstack/bin/gstack-ship-receipt write --pr --base --gate @.gstack/tmp/ship-gate.json --pregate .gstack/tmp/pregate.json --issues @.gstack/tmp/issues.json --review-loop @.gstack/tmp/review-loop.json --docs @.gstack/tmp/docs-audit.json --body "$PR_BODY_FILE" ``` @@ -216,7 +216,9 @@ When the owner accepted a stop after a fix-check edit, add 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 `&&` +`REVIEW_LOOP:` line is the one the Pre-Landing Review section carries). A review whose +readers ran through `gstack-review-dispatch` passes `--run ` instead of +`--no-ledger`, so their CRITICAL findings count. The `&&` matters: a summary that refuses (no pass matched `REVIEW_SINCE`) must not let `write` record a stale or empty loop file. diff --git a/test/helpers/ship-frozen-scope-actor.ts b/test/helpers/ship-frozen-scope-actor.ts index 85c6dca67..19a2a8d28 100644 --- a/test/helpers/ship-frozen-scope-actor.ts +++ b/test/helpers/ship-frozen-scope-actor.ts @@ -190,7 +190,7 @@ function reviewRows(repo: string, env: NodeJS.ProcessEnv) { } function loopSummary(repo: string, env: NodeJS.ProcessEnv, since: string) { - return JSON.parse(command(repo, env, path.join(ROOT, 'bin/gstack-review-scope'), ['summary', '--since', since, '--json']).stdout.trim()); + return JSON.parse(command(repo, env, path.join(ROOT, 'bin/gstack-review-scope'), ['summary', '--no-ledger', '--since', since, '--json']).stdout.trim()); } export async function shipFrozenScopeFailures(fixture: ReturnType, result: Pick) { diff --git a/test/review-scope-ledger-count.test.ts b/test/review-scope-ledger-count.test.ts new file mode 100644 index 000000000..38802cf8b --- /dev/null +++ b/test/review-scope-ledger-count.test.ts @@ -0,0 +1,153 @@ +/** + * GST-951: the /ship receipt's review_loop counters (gstack-review-scope summary) missed real blockers three times on + * 2026-11-02, because summary read only the review-log rows while /ship's readers report through the dispatch ledgers + * (gstack-review-dispatch collect and judge). Each case below is a minimal reconstruction of that PR's rows and ledgers, + * built from its public PR body and receipt (the rows and ledgers themselves stayed on the lanes' machines): the rows + * reproduce the receipt's published counters on the old summary, and the ledgers hold what the PR body says the readers found. + * Ledgers are written with the real collect and judge, so the parser and judge rules apply as in a run. + */ +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 { collect, readJsonl, recordJudgment, type JudgedLine, type MergedLine } from '../lib/review-ledger'; +import { parseReviewLoop } from '../lib/ship-receipt'; +import { cleanup, makeFixtureRepo, runBin, type FixtureRepo } from './helpers/restamp-fixture'; + +const repos: FixtureRepo[] = []; +afterEach(() => { while (repos.length) cleanup(repos.pop()!); }); + +const SINCE = '2026-11-02T01:00:00Z'; +const at = (min: number) => new Date(Date.parse(SINCE) + min * 60_000).toISOString(); +const review = (min: number, pass: number, findings: object[]) => ({ skill: 'review', via: 'ship', timestamp: at(min), status: findings.length ? 'issues_found' : 'clean', review_scope: 'frozen', pass, findings }); +const fixCheck = (min: number, findings: object[]) => ({ skill: 'review-fix-check', via: 'ship', timestamp: at(min), review_scope: 'frozen', findings }); +const note = (fp: string, raised_by: string) => ({ fingerprint: fp, severity: 'INFORMATIONAL', action: 'noted', blocking: false, raised_by }); + +/** One pass directory: each reader's output collected into its ledger, then lead judgment keeping every label. */ +function pass(dir: string, readers: Array<[string, string]>): string { + fs.mkdirSync(dir, { recursive: true }); + const ledger = path.join(dir, 'review-ledger.jsonl'); + const mergedFile = path.join(dir, 'merged.jsonl'); + fs.writeFileSync(mergedFile, ''); + for (const [reader, output] of readers) expect(collect(ledger, reader, output, mergedFile).review).toBe(true); + const merged = readJsonl(mergedFile); + const judged: JudgedLine[] = merged.map(m => ({ id: m.id, severity: m.label })); + const judgedFile = path.join(dir, 'judged.jsonl'); + fs.writeFileSync(judgedFile, judged.map(j => JSON.stringify(j) + '\n').join('')); + expect(recordJudgment(ledger, merged, judged, judgedFile, mergedFile).ok).toBe(true); + return ledger; +} + +function branchWith(rows: object[]): FixtureRepo { + const repo = makeFixtureRepo({ prefix: 'gst951-' }); + repos.push(repo); + repo.git('checkout', '-qb', 'feat'); + const slug = runBin(repo, 'gstack-slug', ['--get', 'SLUG']).out.trim(); + const logDir = path.join(repo.home, 'projects', slug); + fs.mkdirSync(logDir, { recursive: true }); + fs.writeFileSync(path.join(logDir, 'feat-reviews.jsonl'), rows.map(r => JSON.stringify(r) + '\n').join('')); + return repo; +} +const summary = (repo: FixtureRepo, ...args: string[]) => runBin(repo, 'gstack-review-scope', ['summary', '--since', SINCE, '--json', ...args]); +const loop = (r: { status: number | null; out: string; err: string }) => { expect(r.err).not.toContain('(REVIEW_SCOPE_'); expect(r.status).toBe(EXIT.ok); return JSON.parse(r.out); }; + +const CLEAN = 'NO FINDINGS'; +const OUTSIDE_CLEAN = 'No issues found in the diff.\nRecommendation: ship because no finding reaches the floor'; + +describe('GST-951: the receipt counts every blocker the readers found', () => { + // #3841 (reconstruction): the native adversarial pass's FIXABLE (CRITICAL) finding was fixed, but no row recorded it; the + // receipt's blocking_found=1 fixed=1 was entered by hand. Its two other findings were noted in pass 2. + test('#3841: an adversarial-only blocker counts', () => { + const repo = branchWith([ + review(0, 1, []), + review(40, 2, [note('lib/pregate.ts:88:docs', 'adversarial-native'), note('docs/pregate.md:12:docs', 'adversarial-native')]), + ]); + const run = path.join(repo.root, 'run'); + pass(path.join(run, 'p1'), [['specialist/testing', CLEAN], ['red-team', CLEAN]]); + pass(path.join(run, 'p1-voices'), [['outside-voice', OUTSIDE_CLEAN], ['adversarial-native', [ + '1. lib/pregate-clear.ts:140 The receipt stores the raw run URL, not the attempt it judged. FIXABLE', + '2. lib/pregate.ts:88 The refusal text does not name the attempt. INVESTIGATE', + '3. docs/pregate.md:12 The docs do not say a rerun replaces the attempt. INVESTIGATE', + 'Recommendation: fix 1 because a later attempt would be judged by the first', + ].join('\n')]]); + expect(loop(summary(repo, '--no-ledger'))).toMatchObject({ passes: 2, blocking_found: 0, noted: 2, fixed: 0 }); + const s = loop(summary(repo, '--run', run)); + expect(s).toMatchObject({ passes: 2, blocking_found: 1, noted: 2, fixed: 0, unrecorded: 1, ledgers: 2, rounds: 2 }); + }); + + // #3838 (reconstruction): the rows reproduce its receipt (passes=2 blocking_found=0 noted=8 fixed=4 fix_checks=3). Pass 2's + // red-team CRITICAL was logged fixed without classify's blocking stamp; Step 11's native adversarial CRITICAL (CSS + // image-set loading an outside image) was fixed in 0f61362c1 and no row holds it. + test('#3838: CRITICALs fixed without a classified row count', () => { + const repo = branchWith([ + review(0, 1, ['a', 'b', 'c'].map(x => ({ fingerprint: `lib/review-depth.ts:1${x}:style`, severity: 'INFORMATIONAL', action: 'fixed', blocking: false, raised_by: 'security' }))), + review(50, 2, [{ fingerprint: 'lib/review-depth.ts:212:security', severity: 'CRITICAL', action: 'fixed', raised_by: 'red-team' }, + note('lib/review-depth.ts:220:style', 'red-team'), note('lib/review-depth.ts:230:style', 'red-team')]), + fixCheck(60, [note('lib/review-depth.ts:240:style', 'fix-check'), note('lib/review-depth.ts:241:style', 'fix-check')]), + fixCheck(90, [note('lib/review-depth.ts:250:style', 'fix-check')]), + fixCheck(120, [note('lib/review-depth.ts:260:perf', 'fix-check'), note('lib/review-depth.ts:261:perf', 'fix-check'), note('lib/review-depth.ts:262:perf', 'fix-check')]), + ]); + const run = path.join(repo.root, 'run'); + pass(path.join(run, 'p1'), [['specialist/security', '- [INFORMATIONAL] lib/review-depth.ts:1a a CSS escape\n- [INFORMATIONAL] lib/review-depth.ts:1b a zero-padded reference\n- [INFORMATIONAL] lib/review-depth.ts:1c a split handler line'], ['red-team', CLEAN]]); + pass(path.join(run, 'p2'), [['red-team', '[CRITICAL] lib/review-depth.ts:212 a stray quote hides an external href from the drawing check']]); + pass(path.join(run, 'p2-voices'), [['outside-voice', OUTSIDE_CLEAN], ['adversarial-native', 'lib/review-depth.ts:300 CSS image-set() loads an outside image. FIXABLE\nRecommendation: fix because the drawing loads a remote URL']]); + expect(loop(summary(repo, '--no-ledger'))).toMatchObject({ passes: 2, blocking_found: 1, noted: 8, fixed: 4, fix_checks: 3, rounds: 5 }); + const s = loop(summary(repo, '--run', run)); + expect(s).toMatchObject({ passes: 2, blocking_found: 2, deferred: 0, noted: 8, fixed: 4, fix_checks: 3, rounds: 5, unrecorded: 1, ledgers: 3 }); + }); + + // #3837 (reconstruction): pass 1's row was logged before the outside voice's seven findings were classified, so they sit in the + // row with no severity, blocking stamp or action; the fix batch under the Captain's exception fixed them, and the receipt said + // blocking_found=0 fixed=0 noted=11 fix_checks=3. + test('#3837: blockers logged before classification count', () => { + const seven = Array.from({ length: 7 }, (_, i) => ({ fingerprint: `lib/inventory.ts:${10 + i}:logic`, raised_by: 'outside-voice' })); + const repo = branchWith([ + review(0, 1, [...seven, ...Array.from({ length: 9 }, (_, i) => note(`docs/inventory.md:${i + 1}:docs`, 'specialist'))]), + review(60, 2, [note('lib/inventory.ts:90:docs', 'red-team'), note('lib/inventory.ts:91:docs', 'red-team')]), + fixCheck(80, []), fixCheck(100, []), fixCheck(120, []), + ]); + const run = path.join(repo.root, 'run'); + pass(path.join(run, 'p1'), [['specialist-batched/1', CLEAN], ['red-team', CLEAN]]); + pass(path.join(run, 'p1-voices'), [['adversarial-native', CLEAN], ['outside-voice', [ + ...seven.map((f, i) => `${i + 1}. ${f.fingerprint.split(':').slice(0, 2).join(':')} finding ${i + 1} Severity: High Blocks: yes`), + 'Recommendation: fix 1-6 because a self-grandfathered row skips every plan rule', + ].join('\n')]]); + expect(loop(summary(repo, '--no-ledger'))).toMatchObject({ passes: 2, blocking_found: 0, noted: 11, fixed: 0, fix_checks: 3, rounds: 5 }); + const s = loop(summary(repo, '--run', run)); + expect(s).toMatchObject({ passes: 2, blocking_found: 7, noted: 11, fixed: 0, unrecorded: 7, ledgers: 2, rounds: 5 }); + const line = runBin(repo, 'gstack-review-scope', ['summary', '--since', SINCE, '--run', run]); + expect(line.out.trim()).toBe('REVIEW_LOOP: passes=2 blocking_found=7 deferred=0 noted=11 fixed=0 scope=frozen unrecorded=7 ledgers=2 fix_checks=3'); + expect(parseReviewLoop(s)).toMatchObject({ blocking_found: 7, unrecorded: 7, ledgers: 2, rounds: 5 }); + }); + + test('a blocker both sources hold counts once, and a disposed one is not unrecorded', () => { + const repo = branchWith([review(0, 1, [{ fingerprint: 'src/a.ts:3:logic', severity: 'CRITICAL', blocking: true, action: 'fixed', raised_by: 'red-team' }])]); + const run = path.join(repo.root, 'run'); + pass(path.join(run, 'p1'), [['specialist/testing', CLEAN], ['red-team', '[CRITICAL] src/a.ts:3 the guard is inverted']]); + const s = loop(summary(repo, '--run', run)); + expect(s).toMatchObject({ blocking_found: 1, fixed: 1, ledgers: 1 }); + expect(s.unrecorded).toBeUndefined(); + }); + + test('summary refuses REVIEW_SCOPE_LEDGER instead of counting without a ledger it needs, and writes nothing', () => { + const repo = branchWith([review(0, 1, [])]); + const run = path.join(repo.root, 'run'); + const ledger = pass(path.join(run, 'p1'), [['red-team', '[CRITICAL] src/a.ts:3 the guard is inverted']]); + const out = path.join(repo.root, 'loop.json'); + const refused = (r: { status: number | null; err: string }, why: string) => { expect(r.status).toBe(EXIT.fail); expect(r.err).toContain('(REVIEW_SCOPE_LEDGER;'); expect(r.err).toContain(why); expect(fs.existsSync(out)).toBe(false); }; + refused(summary(repo, '--out', out), '--no-ledger when no reader ran through gstack-review-dispatch'); + fs.mkdirSync(path.join(repo.root, 'empty')); + refused(summary(repo, '--run', path.join(repo.root, 'empty'), '--out', out), 'holds no review-ledger.jsonl'); + refused(summary(repo, '--ledger', path.join(run, 'nope.jsonl'), '--out', out), 'is unreadable'); + const judged = path.join(run, 'p1', 'judged.jsonl'); + const kept = fs.readFileSync(judged, 'utf8'); + fs.writeFileSync(judged, kept.replace('CRITICAL', 'INFORMATIONAL')); + refused(summary(repo, '--ledger', ledger, '--out', out), 'changed since judge'); + fs.writeFileSync(judged, kept); + fs.rmSync(path.join(run, 'p1', 'merged.jsonl')); + refused(summary(repo, '--run', run, '--out', out), 'is unreadable'); + const both = summary(repo, '--run', run, '--no-ledger'); + expect(both.status).toBe(EXIT.fail); + expect(both.err).toContain('(REVIEW_SCOPE_INPUT;'); + }); +}); diff --git a/test/review-scope-windows-launch.test.ts b/test/review-scope-windows-launch.test.ts index 68c077bd5..5a7405455 100644 --- a/test/review-scope-windows-launch.test.ts +++ b/test/review-scope-windows-launch.test.ts @@ -50,7 +50,7 @@ test('summary reads the review log through gstack-review-read (#3541)', () => { ]; fs.writeFileSync(path.join(state, 'projects', SLUG, 'feat-reviews.jsonl'), rows.map(r => JSON.stringify(r)).join('\n') + '\n'); const out = path.join(tmp, 'review-loop.json'); - const r = runScriptSync(path.join(ROOT, 'bin', 'gstack-review-scope'), ['summary', '--since', '2026-10-26T09:00:00Z', '--out', out, '--json'], { + const r = runScriptSync(path.join(ROOT, 'bin', 'gstack-review-scope'), ['summary', '--no-ledger', '--since', '2026-10-26T09:00:00Z', '--out', out, '--json'], { cwd: repo, env, encoding: 'utf8', timeout: 120_000, }); expect(r.error).toBeUndefined(); diff --git a/test/review-scope.test.ts b/test/review-scope.test.ts index e6bdc2ace..8b40ad3af 100644 --- a/test/review-scope.test.ts +++ b/test/review-scope.test.ts @@ -423,30 +423,30 @@ describe('gstack-review-scope (bin)', () => { fs.writeFileSync(path.join(logDir, `${branch}-reviews.jsonl`), rows.map(r => JSON.stringify(r)).join('\n') + '\n{not json\n'); const expected = { passes: 2, blocking_found: 2, deferred: 1, noted: 1, fixed: 1, scope: 'frozen', rounds: 2 }; const out = path.join(repo.root, 'review-loop.json'); - const r = runBin(repo, 'gstack-review-scope', ['summary', '--since', '2026-10-26T09:30:00Z', '--out', out, '--json']); + const r = runBin(repo, 'gstack-review-scope', ['summary', '--no-ledger', '--since', '2026-10-26T09:30:00Z', '--out', out, '--json']); expect(r.status).toBe(EXIT.ok); expect(r.err).toContain('REVIEW_LOOP: passes=2 blocking_found=2 deferred=1 noted=1 fixed=1 scope=frozen'); expect(JSON.parse(r.out)).toEqual(expected); expect(JSON.parse(fs.readFileSync(out, 'utf8'))).toEqual(expected); - const plain = runBin(repo, 'gstack-review-scope', ['summary', '--since', '2026-10-26T08:00:00Z']); + const plain = runBin(repo, 'gstack-review-scope', ['summary', '--no-ledger', '--since', '2026-10-26T08:00:00Z']); expect(plain.status).toBe(EXIT.ok); expect(plain.out.trim()).toBe('REVIEW_LOOP: passes=3 blocking_found=3 deferred=1 noted=1 fixed=2 scope=frozen'); const noSince = runBin(repo, 'gstack-review-scope', ['summary', '--json']); expect(noSince.status).toBe(EXIT.fail); expect(noSince.err).toContain('(REVIEW_SCOPE_INPUT;'); expect(noSince.out).toBe(''); - const badSince = runBin(repo, 'gstack-review-scope', ['summary', '--since', 'yesterday']); + const badSince = runBin(repo, 'gstack-review-scope', ['summary', '--no-ledger', '--since', 'yesterday']); expect(badSince.status).toBe(EXIT.fail); expect(badSince.err).toContain('(REVIEW_SCOPE_INPUT;'); // zero matching passes is a wrong --since or a log written elsewhere, never a loop to record: refused, nothing written const bare = fixture(); const noneOut = path.join(bare.root, 'review-loop.json'); - const none = runBin(bare, 'gstack-review-scope', ['summary', '--since', '2026-10-26T09:30:00Z', '--out', noneOut, '--json']); + const none = runBin(bare, 'gstack-review-scope', ['summary', '--no-ledger', '--since', '2026-10-26T09:30:00Z', '--out', noneOut, '--json']); expect(none.status).toBe(EXIT.fail); expect(none.err).toContain('(REVIEW_SCOPE_INPUT;'); expect(none.err).toContain('REVIEW_LOOP: passes=0'); expect(fs.existsSync(noneOut)).toBe(false); - const late = runBin(repo, 'gstack-review-scope', ['summary', '--since', '2026-10-27T00:00:00Z']); + const late = runBin(repo, 'gstack-review-scope', ['summary', '--no-ledger', '--since', '2026-10-27T00:00:00Z']); expect(late.status).toBe(EXIT.fail); }); }); diff --git a/test/review-targeted.test.ts b/test/review-targeted.test.ts index 375cf1d01..9ef2217d3 100644 --- a/test/review-targeted.test.ts +++ b/test/review-targeted.test.ts @@ -324,7 +324,7 @@ describe('the targeted role end to end on a fixture repository', () => { fs.mkdirSync(logDir, { recursive: true }); const row = { skill: 'review', via: 'ship', timestamp: '2026-10-30T10:00:00Z', status: 'clean', review_scope: 'frozen', findings: [], dispatches: [{ role: 'specialist' }, { role: 'adversarial-native' }, { role: 'targeted' }] }; fs.writeFileSync(path.join(logDir, 'feat-reviews.jsonl'), JSON.stringify(row) + '\n'); - const summary = runBin(repo, 'gstack-review-scope', ['summary', '--since', '2026-10-30T09:00:00Z', '--json']); + const summary = runBin(repo, 'gstack-review-scope', ['summary', '--no-ledger', '--since', '2026-10-30T09:00:00Z', '--json']); expect(summary.status, summary.err).toBe(0); expect(JSON.parse(summary.out).dispatches).toEqual({ specialist: 1, 'adversarial-native': 1, targeted: 1 }); diff --git a/test/size-ledger/ship/gst-951-receipt-counts-blockers.json b/test/size-ledger/ship/gst-951-receipt-counts-blockers.json new file mode 100644 index 000000000..be4f7a928 --- /dev/null +++ b/test/size-ledger/ship/gst-951-receipt-counts-blockers.json @@ -0,0 +1,4 @@ +{ + "bytes": 157, + "why": "GST-951: summary names its ledgers (--run in DRIVER, --no-ledger in pr-body)" +}