diff --git a/CHANGELOG.md b/CHANGELOG.md index d9bcd7f28..7618fa598 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,14 @@ # Changelog +## [1.93.22.0] - 2026-11-02 + +### Fixed + +- **The targeted reader's record is honest from verify to the receipt.** `gstack-review-dispatch verify` no longer marks a completed targeted reader `status=failed findings=0` because a Step 11 fix moved HEAD after it read (gstack-evals#140, C5-36 and C6-36). The record now says `done` with its real finding count and names the commit it read. `gstack-ship-receipt write --review-targeted` binds it only while that commit's product patch is the receipt's and nothing since it carries a money, gate, auth or secret change (a test edit that skips a case counts as gate), so a record can no longer vouch for code it never read. +- **`write` and `carry` refuse a receipt the queue refuses (#3854, #3859).** When the head touches money, gate, auth or secret code and no done targeted pass covers it, `write` refuses with `RECEIPT_INVALID` and `carry` with `RECEIPT_CARRY_REFUSED`, instead of publishing a receipt `ready` turns away. Legacy receipts still carry. +- **/ship passes the targeted record to the receipt (#3853).** Step 11's verify writes `.gstack/tmp/targeted.json`, and the Step 19, handoff and driver receipt writes pass it with `--review-targeted`. +- **Fix-check rounds count without hand edits (#3862).** `gstack-review-scope summary --run` counts fix-checks from the dispatch ledgers when no `review-fix-check` row was logged, and counts the blockers those fix-checks followed as fixed. + ## [1.93.21.0] - 2026-11-02 **/ship's finding collector lets the native reviewer's own Severity field decide outside gate and risk reviews.** diff --git a/VERSION b/VERSION index 10f963b21..3e56411d5 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.93.21.0 +1.93.22.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index ae13f8251..dbef6f88c 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.93.21.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.93.22.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-review-dispatch b/bin/gstack-review-dispatch index 3bdc0d919..6ca52ac1a 100755 --- a/bin/gstack-review-dispatch +++ b/bin/gstack-review-dispatch @@ -24,7 +24,7 @@ import { cleanMarkers, collect, fixableRaises, mayBeUnavailable, parseFindings, import { buildManifest, chunkCount, chunkOf, manifestPath, markReader, readManifest, renderStage, writeManifest, STAGES, type Extra, type Manifest, type PassReader, type ReaderState, type Stage } from '../lib/review-pass'; import type { Dispatch } from '../lib/review-depth'; import { runScriptSync } from '../lib/script-invocation'; -import { TARGETED_RULES, parseClassList, targetedBriefSha, targetedPatchId } from '../lib/review-targeted'; +import { TARGETED_RULES, parseClassList, targetedBriefSha, targetedRead } from '../lib/review-targeted'; import { resolveBaseBranch } from '../lib/ship-policy'; const HELP = `gstack-review-dispatch — verbatim reviewer briefs and loss-free collection for /ship @@ -130,7 +130,7 @@ function render(): number { fs.writeFileSync(file, text, { mode: 0o600 }); const ledger = flag('--ledger') ?? path.join(out, 'review-ledger.jsonl'); const sha = promptHash(text); - const targeted = brief === undefined ? {} : { classes: parseClassList(flag('--classes')!), rules: TARGETED_RULES, brief_sha: targetedBriefSha(checklist), diff_base: diffBase ?? null, patch_id: diffBase ? targetedPatchId(process.cwd(), diffBase) : 'none' }; + const targeted = brief === undefined ? {} : { classes: parseClassList(flag('--classes')!), rules: TARGETED_RULES, brief_sha: targetedBriefSha(checklist), diff_base: diffBase ?? null, ...(diffBase ? targetedRead(process.cwd(), diffBase) : { patch_id: 'none', head: null }) }; appendEvent(ledger, { event: 'rendered', reader: readerKey(role, name), role, name: name ?? null, file, sha256: sha, bytes: Buffer.byteLength(text), workdir, tree, ...targeted }); process.stdout.write(`BRIEF: reader=${readerKey(role, name)} sha256=${sha} bytes=${Buffer.byteLength(text)} file=${file}\n`); return EXIT.ok; @@ -279,19 +279,22 @@ function program(): number { /** * The receipt's review_targeted record from the newest targeted dispatch: done only when its brief was sent verbatim, its output - * collected, and the committed diff it was rendered on is still the one at verify (targetedPatchId; uncommitted never proves it). + * collected, and it was rendered on a committed diff (uncommitted never proves what was read). `head` names the commit it read: + * a later commit does not undo the read, and gstack-ship-receipt write binds the record only while that commit's product patch is + * the receipt's (GST-986: Step 11 fixes after the read printed status=failed findings=0 for a completed reader). */ -function targetedRecord(ledger: string, rows: VerifyRow[]): { classes: string[]; rules: string; brief_sha: string; status: 'done' | 'failed'; findings: number; raised_by: string } | null { +function targetedRecord(ledger: string, rows: VerifyRow[]): { classes: string[]; rules: string; brief_sha: string; status: 'done' | 'failed'; findings: number; raised_by: string; head: string | null } | null { const events = readLedger(ledger); const render = events.filter(e => e.event === 'rendered' && e.reader === 'targeted').at(-1); if (!render) return null; const label = rows.filter(r => r.kind === 'BRIEF' && r.reader.split('@')[0] === 'targeted').at(-1)?.reader; const brief = rows.filter(r => r.kind === 'BRIEF' && r.reader === label).at(-1); const output = rows.filter(r => r.kind === 'OUTPUT' && r.reader === label).at(-1); - const read = typeof render.patch_id === 'string' && !['uncommitted', 'none'].includes(render.patch_id) && typeof render.diff_base === 'string' && targetedPatchId(process.cwd(), render.diff_base) === render.patch_id; - const done = brief?.status === 'MATCH' && output?.status === 'MATCH' && read; + const head = typeof render.head === 'string' && /^[0-9a-f]{40,64}$/.test(render.head) ? render.head : null; + const committed = typeof render.patch_id === 'string' && !['uncommitted', 'none'].includes(render.patch_id) && head !== null; + const done = brief?.status === 'MATCH' && output?.status === 'MATCH' && committed; const ids = events.filter(e => e.event === 'output' && e.reader === 'targeted').at(-1)?.ids; - return { classes: (render.classes as string[]) ?? [], rules: String(render.rules), brief_sha: String(render.brief_sha), status: done ? 'done' : 'failed', findings: done && Array.isArray(ids) ? ids.length : 0, raised_by: 'targeted' }; + return { classes: (render.classes as string[]) ?? [], rules: String(render.rules), brief_sha: String(render.brief_sha), status: done ? 'done' : 'failed', findings: Array.isArray(ids) ? ids.length : 0, raised_by: 'targeted', head }; } function main(): number { @@ -356,12 +359,13 @@ function main(): number { return r.ok ? EXIT.ok : EXIT.fail; } if (cmd === 'verify') { + if (flag('--targeted-out')) fs.rmSync(flag('--targeted-out')!, { force: true }); const v = verify(ledger); for (const r of v.rows) process.stdout.write(`${r.kind} ${r.reader} ${r.status} ${r.detail}\n`); if (!v.briefs) { process.stdout.write('VERIFY: no brief records (this run\'s ledger kept no prompts; dispatched-vs-rendered cannot be checked)\n'); return EXIT.refused; } const record = targetedRecord(ledger, v.rows); if (record) { - process.stdout.write(`TARGETED_RECORD: classes=${record.classes.join(',')} status=${record.status} findings=${record.findings} brief=${record.brief_sha.slice(0, 12)} rules=${record.rules}\n`); + process.stdout.write(`TARGETED_RECORD: classes=${record.classes.join(',')} status=${record.status} findings=${record.findings} brief=${record.brief_sha.slice(0, 12)} rules=${record.rules} read=${record.head?.slice(0, 12) ?? 'none'}\n`); if (flag('--targeted-out')) fs.writeFileSync(flag('--targeted-out')!, JSON.stringify(record) + '\n'); } process.stdout.write(`VERIFY: ${v.clean ? 'clean' : 'dirty'} briefs=${v.briefs} outputs=${v.rows.filter(r => r.kind === 'OUTPUT').length} voided=${v.voided}${v.unavailable ? ` unavailable=${v.unavailable}` : ''}\n`); diff --git a/bin/gstack-review-scope b/bin/gstack-review-scope index 402090911..bd8280e29 100755 --- a/bin/gstack-review-scope +++ b/bin/gstack-review-scope @@ -27,7 +27,7 @@ import { runScriptSync } from '../lib/script-invocation'; import { isUnavailable, loadPolicyFromBase, policyUnavailableLine, resolveBaseBranch } from '../lib/ship-policy'; import { EXIT as POST_EXIT, runPost } from '../lib/gstack-post'; import { - parseHunks, renderDeferredIssue, renderDeferredSection, reviewLoopLine, scopeFindings, scopeLine, summarizeReviewLoop, + ledgerPass, 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, type LedgerBlocker } from '../lib/review-ledger'; @@ -269,7 +269,7 @@ function cmdSummary(rest: string[], json: boolean): number { 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: LedgerBlocker[][]; try { ledgers = files.map(ledgerBlockers); } catch (e: any) { return refuse('summary', 'REVIEW_SCOPE_LEDGER', e.message); } - const summary = summarizeReviewLoop(rows, since, ledgers); + const summary = summarizeReviewLoop(rows, since, ledgers, files.map(ledgerPass)); 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/bin/gstack-ship-receipt b/bin/gstack-ship-receipt index 773fb19c1..79c64da9e 100755 --- a/bin/gstack-ship-receipt +++ b/bin/gstack-ship-receipt @@ -49,7 +49,7 @@ import { } from '../lib/ship-receipt'; import { checkDocsCaps, docsCapLine, docsExceptionLine, parseReceiptDocs } from '../lib/ship-receipt-docs'; import { comparePath, contextPacket, PACKET_SECTION_CAP, readScopedReview, touchedPaths } from '../lib/ship-patch'; -import { deeper, depthBelow, exceptionProblems, parseReviewLoopException, parseReviewTargeted, receiptDepthAtHead, reviewLoopExceptionLine } from '../lib/ship-receipt-depth'; +import { deeper, depthBelow, exceptionProblems, parseReviewLoopException, parseReviewTargeted, receiptDepthAtHead, receiptTargetedAtHead, reviewLoopExceptionLine, targetedClassesSince } from '../lib/ship-receipt-depth'; import { checkReceiptProse, proseLine } from '../lib/ship-receipt-prose'; import { resolveStateRoot } from '../lib/state-root'; import { readPause, receiptBaseRed } from '../lib/base-red'; @@ -96,9 +96,11 @@ write flags: --pr --head --base --version --gated-tree --predecessor --gate ({classes, rules, brief_sha, status, findings, raised_by}: gstack-review-dispatch verify - --targeted-out); recorded as review_targeted bound to the receipt's product_patch_id. Without it the receipt has no - review_targeted, which the queue reports as legacy. + --review-targeted ({classes, rules, brief_sha, status, findings, raised_by, head}: gstack-review-dispatch verify + --targeted-out); recorded as review_targeted bound to the receipt's product_patch_id, and refused (RECEIPT_INVALID) when the + commit the reader read (head) is not an ancestor or a change since it carries a targeted class (gate scan included). write and carry + refuse a receipt whose head needs a targeted class (money, gate, auth, secret) that no done pass covers, as the queue's + targeted verdict does (#3859) (RECEIPT_INVALID, RECEIPT_CARRY_REFUSED). --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 @@ -222,7 +224,7 @@ function cmdWrite(rest: string[], json: boolean): number { const product = reviewedProduct(repoRoot, base, head, pregateFile); let targeted: ReturnType | undefined; if (flag(rest, '--review-targeted') !== undefined) { - try { targeted = parseReviewTargeted(jsonArg(flag(rest, '--review-targeted')), product.product_patch_id ?? 'none'); } catch (e: any) { targeted = { ok: false, message: `--review-targeted: ${e.message}` }; } + try { targeted = parseReviewTargeted(jsonArg(flag(rest, '--review-targeted')), product.product_patch_id ?? 'none', read => reviewedProduct(repoRoot, base, read, pregateFile).product_patch_id, read => targetedClassesSince(repoRoot, read, headSha ?? head)); } catch (e: any) { targeted = { ok: false, message: `--review-targeted: ${e.message}` }; } } if (targeted && !targeted.ok) { process.stderr.write(resultLine('gstack-ship-receipt write', 'RECEIPT_INVALID', `${targeted.message}; nothing written`) + '\n'); return EXIT.fail; } const approval = approvalForReceipt({ repoRoot, base, head, citationText: [bodyText, ...flags(rest, '--approval')].join('\n'), runDirs: flags(rest, '--approval-run') }); @@ -267,6 +269,12 @@ function cmdWrite(rest: string[], json: boolean): number { process.stderr.write('gstack-ship-receipt write: nothing written\n'); return EXIT.fail; } + const targetedVerdict = receiptTargetedAtHead(repoRoot, `origin/${base.replace(/^origin\//, '')}`, receipt, receipt.head); + if (targetedVerdict.line.includes('(unchecked: ')) process.stderr.write(targetedVerdict.line + '\n'); + if (!targetedVerdict.ok) { + process.stderr.write(`${targetedVerdict.line}\n${resultLine('gstack-ship-receipt write', 'RECEIPT_INVALID', `${targetedVerdict.message}; run the targeted reader on HEAD and pass its record (--review-targeted @); nothing written`)}\n`); + return EXIT.fail; + } const body = bodyFile; const missing = body && receipt.issues ? missingIssueLines(fs.existsSync(body) ? fs.readFileSync(body, 'utf8') : '', receipt.issues) : []; if (missing.length) { @@ -734,6 +742,8 @@ function cmdCarry(rest: string[], json: boolean): number { const carried = carryReceipt(rebound, 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 targetedVerdict = receiptTargetedAtHead(repoRoot, baseRef, carried, head); + if (!targetedVerdict.ok) return refused('RECEIPT_CARRY_REFUSED', `${targetedVerdict.line}: ${targetedVerdict.message}, so the queue refuses it too (#3859); run /ship on ${head.slice(0, 12)}`); const newBody = placeReceiptFirst(body, carried); const line = receiptSummaryLine(carried); if (scopedDryRun) return refused('RECEIPT_CARRY_SCOPED_REVIEW', scopedDryRun); diff --git a/docs/review-policy.md b/docs/review-policy.md index b0b8f2358..3362090f0 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -400,8 +400,19 @@ The brief states the blocking floor in one sentence, and its money block cites the checklist's Forced-CRITICAL rule on uncapped spend, so a money finding can block without new floor words. The reader is read-only, and `NO FINDINGS` is a result. `gstack-review-dispatch render --role targeted --classes ` -renders its brief; `verify --targeted-out ` writes the record that -`gstack-ship-receipt write --review-targeted @` puts in the receipt. +renders its brief; `verify --targeted-out .gstack/tmp/targeted.json` writes the record that +`gstack-ship-receipt write --review-targeted @.gstack/tmp/targeted.json` puts in the receipt +(Step 19, the handoff and the driver pass it when the file exists). The record is `done` when +the reader got its brief verbatim and its reply was collected on a committed diff, whatever +happened to HEAD after, and it names the commit it read (`head`). `write` binds it only while +that commit is an ancestor of HEAD, its product patch is the receipt's, and nothing changed +after it carries a targeted class, with the gate scan run over that range: any product change +after the read refuses with `RECEIPT_INVALID`, docs and TODOS.md commits included, and so +does a test edit the gate scan tags (a skipped case). Every verify +removes `--targeted-out` before it writes a record, so an earlier run's record never reaches +Step 19. `write` and `carry` (which rescans the gate at the new head) also +refuse a receipt whose head needs a class no `done` record covers (`RECEIPT_INVALID`, +`RECEIPT_CARRY_REFUSED`). The queue refuses the same receipts at `ready` (#3859). `gstack-ship-queue ready`, `train build` and `land` print `REVIEW_TARGETED: required= receipt= verdict=ok|missing|failed` and refuse @@ -430,11 +441,11 @@ puts in the receipt: | `review_loop.noted` | below-floor findings listed in the PR body | | `review_loop.fixed` | findings fixed in this PR | | `review_loop.scope` | the `review_scope` the loop ran under | -| `review_loop.fix_checks` | fix-checks this ship ran (never passes) | +| `review_loop.fix_checks` | fix-checks this ship ran (never passes): the larger of the `review-fix-check` rows and the ledgers holding a fix-check reader's review. When fix-checks ran without their rows, a blocker no row disposes counts as fixed once a later fix-check ledger followed it (decision 3.5 runs one only after the fixes) | | `review_loop.rounds` | passes plus fix-checks (R51 caps it at 2; the queue's `REVIEW_DEPTH:` line prints it) | | `review_loop.dispatches` | reviewer dispatches by role: `specialist`, `red-team`, `adversarial-native`, `outside-voice`, `fix-check`, `targeted` | | `review_loop.est_input_tokens` | prompt plus diff bytes the dispatches read, divided by 4: a relative estimate for comparing runs, not a bill | -| `review_targeted` | the [targeted pass](#targeted-pass-on-money-gate-auth-and-secret-code): the `classes` it covered, `rules`, `brief_sha`, the receipt's own `product_patch_id`, `status` (`done` only with its output collected and the committed diff it was rendered on still the one at verify, else `failed`; absent when write got no record, which the queue reports as `legacy`), `findings` and `raised_by`; a carry keeps it while the product patch is unchanged | +| `review_targeted` | the [targeted pass](#targeted-pass-on-money-gate-auth-and-secret-code): the `classes` it covered, `rules`, `brief_sha`, the receipt's own `product_patch_id`, `status` (`done` only with its brief sent verbatim and its output collected on a committed diff, else `failed`; `write` refuses a record when a change after its read commit carries a targeted class or the read commit is not an ancestor, and refuses to write without a record when the head needs a class), `findings` and `raised_by`; a carry keeps it while the product patch is unchanged | Each review row may carry `dispatches: [{role, effort, prompt_bytes, diff_bytes, status}]`, and each finding `raised_by` (the reviewer that raised it, diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 5dfa5ed39..d78dfb536 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -3317,10 +3317,19 @@ for an older PR, write it once by hand with the same command. **Meaning.** The block exists but does not validate against the `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. +checked-out HEAD: the receipt binds the tree the review read. It refuses the +same way when the head touches money, gate, auth or secret code and no done +targeted record covers those classes (`REVIEW_TARGETED: … verdict=missing|failed`), +or when `--review-targeted` names a read commit whose product patch is not +the receipt's and a change after it carries a targeted class (money, gate, +auth or secret code changed after the targeted reader read it, a test edit the +gate scan tags included), or that is not an ancestor of HEAD. **Fix.** Regenerate it with `gstack-ship-receipt write`; never hand-edit the -block. For the `--head` refusal, check out that commit and write there. +block. For the `--head` refusal, check out that commit and write there. For a +targeted refusal, run the targeted reader on HEAD, `gstack-review-dispatch verify +--targeted-out .gstack/tmp/targeted.json`, and write again with +`--review-targeted @.gstack/tmp/targeted.json`. ### `EXECUTION_UNKNOWN` @@ -5818,7 +5827,10 @@ working tree's status. It also refuses when an owner exception the receipt recor 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 +`review_depth` ([ship-policy.md](ship-policy.md#ship-receipt)). It refuses, too, when +HEAD needs a targeted class (money, gate, auth, secret) that the receipt's +`review_targeted` does not cover with a done pass, since the queue would refuse +that receipt. The receipt in the body is unchanged and still names the old head, so `gstack-ship-queue ready` refuses `QUEUE_RECEIPT_STALE`. @@ -5826,7 +5838,7 @@ ready` refuses `QUEUE_RECEIPT_STALE`. `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. For a retargeted stacked -PR whose older receipt lost its parent branch, or a depth or exception +PR whose older receipt lost its parent branch, or a depth, targeted or exception refusal, run /ship on the new head. When `--head` is not the checked-out HEAD, check it out and carry there. diff --git a/lib/review-pass.ts b/lib/review-pass.ts index 365069720..69689091f 100644 --- a/lib/review-pass.ts +++ b/lib/review-pass.ts @@ -16,7 +16,7 @@ import * as path from 'node:path'; import { BriefError, briefFile, checkoutRoot, checkoutTree, promptHash, renderBrief, withWorkspace, type BriefRole, type SectionTexts, type Slots } from './review-brief'; import { appendEvent, readerKey } from './review-ledger'; import { diffCommand, type Dispatch } from './review-depth'; -import { TARGETED_RULES, parseClassList, targetedBriefSha, targetedPatchId } from './review-targeted'; +import { TARGETED_RULES, parseClassList, targetedBriefSha, targetedRead } from './review-targeted'; export const STAGES = ['first', 'specialists', 'red-team', 'voices'] as const; export type Stage = typeof STAGES[number]; @@ -179,7 +179,7 @@ export function renderStage(m: Manifest, stage: Stage, t: SectionTexts, diffFor: let brief: string; try { brief = fs.readFileSync(checklist, 'utf8'); } catch { throw new BriefError(`cannot read the targeted brief ${checklist}`); } Object.assign(slots, { checklist, brief, classes: (r.classes ?? []).join(','), diff_base: r.diff_base ?? m.diff_base }); - targeted = { classes: parseClassList(slots.classes!), rules: TARGETED_RULES, brief_sha: targetedBriefSha(checklist), diff_base: slots.diff_base, patch_id: targetedPatchId(process.cwd(), slots.diff_base!) }; + targeted = { classes: parseClassList(slots.classes!), rules: TARGETED_RULES, brief_sha: targetedBriefSha(checklist), diff_base: slots.diff_base, ...targetedRead(process.cwd(), slots.diff_base!) }; } const text = withWorkspace(renderBrief(role, slots, t), workdir); const file = path.join(briefs, briefFile(role, r.name)); diff --git a/lib/review-scope.ts b/lib/review-scope.ts index 6ab96923f..620c3c11a 100644 --- a/lib/review-scope.ts +++ b/lib/review-scope.ts @@ -14,7 +14,7 @@ * Pure functions here; bin/gstack-review-scope runs git and gh. */ import { SPECIALISTS } from './review-depth'; -import { rank, type LedgerBlocker } from './review-ledger'; +import { rank, readLedger, type LedgerBlocker } from './review-ledger'; import { parseSpecialistOutput } from './state-matrix'; export type Locus = 'diff' | 'fix-range' | 'outside'; @@ -370,6 +370,18 @@ function rowLocus(f: Record): string | null { return m ? `${m[1]!.replace(/^\.\//, '')}:${m[2]}` : null; } +/** + * When a dispatch ledger started (its first event) and whether it holds a fix-check reader's review: a fix-check run through + * gstack-review-dispatch (the pass program's `fix-check` step) leaves this ledger even when no review-fix-check row was logged. + */ +export interface LedgerPass { started: number; fixCheck: boolean } +export function ledgerPass(ledger: string): LedgerPass { + const events = readLedger(ledger); + const started = Date.parse(String(events[0]?.at ?? '')); + const fixCheck = events.some(e => e.event === 'output' && e.reader === 'fix-check' && e.tree_ok !== false && ((Array.isArray(e.ids) && e.ids.length > 0) || e.review === true)); + return { started: Number.isFinite(started) ? started : NaN, fixCheck }; +} + /** * The loop summary. blocking_found counts every blocker found (GST-951). Each CRITICAL the dispatch ledgers hold * (`ledgerBlockers`, one list per ledger) is paired with at most one row finding per ledger at the same path:line, raised @@ -380,8 +392,13 @@ function rowLocus(f: Record): string | null { * A ledger CRITICAL with no locus, or a second one at a paired line, counts on its own. A different defect the same reviewer * raises at the exact line of a finding a row classified below the floor pairs with that finding: path:line is the only * identity rows and ledgers share until rows carry the ledger id. + * Fix-checks (GST-986, #3862; `ledgerPasses`, one per ledger): fix_checks is the larger of the review-fix-check rows and the + * fix-check ledgers, so one logged both ways counts once. When fix-checks ran without their review-fix-check rows (more fix-check + * ledgers than rows), a blocker no row disposes counts as fixed if a fix-check ledger started after the last ledger that raised it: + * decision 3.5 runs a fix-check only after the loop applied that pass's fixes, and a blocker the loop does not fix is deferred + * or noted by a row. A blocker the last pass found stays unrecorded, and with every fix-check logged the rows decide. */ -export function summarizeReviewLoop(rows: readonly Record[], since: string, ledgers: readonly (readonly LedgerBlocker[])[] = []): ReviewLoopSummary { +export function summarizeReviewLoop(rows: readonly Record[], since: string, ledgers: readonly (readonly LedgerBlocker[])[] = [], ledgerPasses: readonly LedgerPass[] = []): 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)); @@ -414,20 +431,28 @@ export function summarizeReviewLoop(rows: readonly Record[], since: } const paired = new Set(); let ledgerOnly = 0; + let ledgerOnlyFixed = 0; const pairedIds = new Set(); + const followedIds = new Set(); + const lastPaired = new Map(); + const fixCheckStarts = ledgerPasses.filter(p => p.fixCheck && Number.isFinite(p.started)).map(p => p.started); + const rowFixChecks = loopRows.filter(r => r.skill === FIX_CHECK_SKILL).length; + const followed = (n: number) => fixCheckStarts.length > rowFixChecks && fixCheckStarts.some(t => t > (ledgerPasses[n]?.started ?? Infinity)); for (const [n, list] of ledgers.entries()) { for (const b of list) { const by = b.id.startsWith('fix-check') ? null : raiserOf(b.id); const id = b.locus ? [...seen].find(([k, s]) => s.candidate && s.locus === b.locus && (!s.raiser || !by || s.raiser === by) && !paired.has(`${n}\0${k}`))?.[0] : undefined; - if (id) { paired.add(`${n}\0${id}`); pairedIds.add(id); } else ledgerOnly++; + if (id) { paired.add(`${n}\0${id}`); pairedIds.add(id); if (!((ledgerPasses[lastPaired.get(id) ?? -1]?.started ?? -Infinity) > (ledgerPasses[n]?.started ?? -Infinity))) lastPaired.set(id, n); } else { ledgerOnly++; if (followed(n)) ledgerOnlyFixed++; } } } + for (const [id, n] of lastPaired) if (followed(n)) followedIds.add(id); const blockers = [...seen].filter(([k, s]) => s.blocks || (pairedIds.has(k) && !s.classified)); - const unrecorded = ledgerOnly + blockers.filter(([, s]) => !s.disposed).length; - const out: ReviewLoopSummary = { passes: passes.length, blocking_found: blockers.length + ledgerOnly, deferred: deferred.size, noted: noted.size, fixed: fixed.size, scope }; + for (const [k, s] of blockers) if (!s.disposed && followedIds.has(k)) { s.disposed = true; fixed.add(k); } + const unrecorded = ledgerOnly - ledgerOnlyFixed + blockers.filter(([, s]) => !s.disposed).length; + const out: ReviewLoopSummary = { passes: passes.length, blocking_found: blockers.length + ledgerOnly, deferred: deferred.size, noted: noted.size, fixed: fixed.size + ledgerOnlyFixed, scope }; if (unrecorded) out.unrecorded = unrecorded; if (ledgers.length) out.ledgers_read = ledgers.length; - const fixChecks = loopRows.filter(r => r.skill === FIX_CHECK_SKILL).length; + const fixChecks = Math.max(rowFixChecks, fixCheckStarts.length); if (fixChecks) out.fix_checks = fixChecks; out.rounds = passes.length + fixChecks; const byRole: Record = {}; diff --git a/lib/review-targeted.ts b/lib/review-targeted.ts index 7fe8c2fe7..3af9e4a55 100644 --- a/lib/review-targeted.ts +++ b/lib/review-targeted.ts @@ -82,6 +82,12 @@ export function targetedPatchId(root: string, mergeBase: string): string { return git(['patch-id', '--verbatim'], diff.stdout).stdout?.trim().split(/\s+/)[0] || 'none'; } +/** What a targeted render records: the patch the reader reads and the commit it sits on, which receipt write binds to its product patch. */ +export function targetedRead(root: string, mergeBase: string): { patch_id: string; head: string | null } { + const head = spawnSync('git', ['rev-parse', '--verify', '--quiet', 'HEAD^{commit}'], { cwd: root, encoding: 'utf8', timeout: 30_000 }).stdout?.trim(); + return { patch_id: targetedPatchId(root, mergeBase), head: head || null }; +} + /** A class list as printed and recorded: comma-joined, or `none`. */ export const classList = (classes: readonly string[]) => (classes.length ? classes.join(',') : 'none'); diff --git a/lib/ship-receipt-depth.ts b/lib/ship-receipt-depth.ts index 66adef3d7..134698330 100644 --- a/lib/ship-receipt-depth.ts +++ b/lib/ship-receipt-depth.ts @@ -131,17 +131,26 @@ export function exceptionProblems(body: string, r: ShipReceipt, isAncestor?: (co export type ReviewTargeted = NonNullable; -/** `write --review-targeted` input (gstack-review-dispatch verify --targeted-out), bound to the receipt's product patch-id here. */ -export function parseReviewTargeted(value: unknown, productPatchId: string): { ok: true; targeted: ReviewTargeted } | { ok: false; message: string } { +/** + * `write --review-targeted` input (gstack-review-dispatch verify --targeted-out), bound to the receipt's product patch-id here. + * `head` is the commit the reader read; `productPatchOf` gives that commit's product patch-id by the receipt's own rules. The + * record binds only on an equal product patch, and with `classesSince` also only when nothing changed after the read carries a + * targeted class (a test change the gate scan tags is gate class though the product patch leaves tests out), so a record never + * vouches for work it did not read (GST-986). + */ +export function parseReviewTargeted(value: unknown, productPatchId: string, productPatchOf: (head: string) => string | undefined, classesSince?: (read: string) => readonly TargetedClass[] | null): { ok: true; targeted: ReviewTargeted } | { ok: false; message: string } { const bad = (message: string) => ({ ok: false as const, message }); - if (!value || typeof value !== 'object' || Array.isArray(value)) return bad('review_targeted must be { classes, rules, brief_sha, status, findings, raised_by }'); + if (!value || typeof value !== 'object' || Array.isArray(value)) return bad('review_targeted must be { classes, rules, brief_sha, status, findings, raised_by, head }'); const v = value as Record; - for (const key of Object.keys(v)) if (!['classes', 'rules', 'brief_sha', 'status', 'findings', 'raised_by'].includes(key)) return bad(`review_targeted: unknown key ${JSON.stringify(key)}${key === 'product_patch_id' ? ' (write binds the pass to the receipt\'s product_patch_id)' : ''}`); + for (const key of Object.keys(v)) if (!['classes', 'rules', 'brief_sha', 'status', 'findings', 'raised_by', 'head'].includes(key)) return bad(`review_targeted: unknown key ${JSON.stringify(key)}${key === 'product_patch_id' ? ' (write binds the pass to the receipt\'s product_patch_id)' : ''}`); if (!Array.isArray(v.classes) || !v.classes.every(c => (TARGETED_CLASSES as readonly unknown[]).includes(c))) return bad(`review_targeted.classes must list ${TARGETED_CLASSES.join(', ')}`); if (!text(v.rules) || !text(v.raised_by)) return bad('review_targeted.rules and raised_by must be non-empty one-line strings'); if (typeof v.brief_sha !== 'string' || !/^[0-9a-f]{64}$/.test(v.brief_sha)) return bad('review_targeted.brief_sha must be the sha256 of review/specialists/targeted.md'); if (!['done', 'failed', 'not-run'].includes(v.status as string)) return bad('review_targeted.status must be done, failed or not-run'); if (typeof v.findings !== 'number' || !Number.isInteger(v.findings) || v.findings < 0) return bad('review_targeted.findings must be an integer >= 0'); + if (typeof v.head !== 'string' || !/^[0-9a-f]{40,64}$/.test(v.head)) return bad('review_targeted.head must be the commit the targeted reader read (rerun gstack-review-dispatch verify --targeted-out with this gstack)'); + const read = productPatchOf(v.head); + if (read !== productPatchId || (classesSince && classesSince(v.head)?.length !== 0)) return bad(`the targeted reader read ${v.head.slice(0, 12)}, whose product patch ${read?.slice(0, 12) ?? 'cannot be read'} is not this receipt's ${productPatchId.slice(0, 12)}: the product changed after the read, so run the targeted reader on HEAD and verify again`); const classes = TARGETED_CLASSES.filter(c => (v.classes as string[]).includes(c)); return { ok: true, targeted: { classes, rules: v.rules as string, brief_sha: v.brief_sha, product_patch_id: productPatchId, status: v.status as ReviewTargeted['status'], findings: v.findings, raised_by: (v.raised_by as string).trim() } }; } @@ -219,3 +228,34 @@ export function bindShippedReceipt(root: string, baseRef: string, body: string, return { ...b, depthLine: d.line, targetedLine: t.line }; } +/** + * The targeted classes of what changed from `from` to the checked-out `head`, with gstack-gate-diff run over that range whenever + * a non-docs path changed (a test edit that skips a case is gate class); null when `from` is not an ancestor of `head`, the gate + * scan did not run, or a class input is unreadable. + */ +export function targetedClassesSince(root: string, from: string, head: string): readonly TargetedClass[] | null { + if (!git(root, ['merge-base', '--is-ancestor', from, head]).ok) return null; + const at = headInputs(root, from, head); + let gateTagged: number | null = 0; + if (gateScanNeeded(at.c.paths, at.c.root)) { + const g = runScriptSync(path.join(BIN_DIR, 'gstack-gate-diff'), [from], { cwd: root, encoding: 'utf8', timeout: 120_000, maxBuffer: 16 * 1024 * 1024 }); + const m = /GATE_SUMMARY: .*?tagged=\{([^}]*)\}/.exec(g.stdout ?? ''); + if (g.status !== 0 || !m) return null; + gateTagged = m[1]!.split(',').reduce((n, kv) => n + (Number(kv.split(':')[1]) || 0), 0); + } + const t = targetedClasses({ ...at.c, scope: at.scope, gateTagged }); + return t.errors.length ? null : t.classes; +} + +/** + * receiptTargetedVerdict at the receipt's head, with the classes the queue computes: what write and carry check before they + * publish (#3854). Classes that failed closed on an input this checkout cannot read (no origin base, no gate scan) are reported, + * never refused on here: the queue reads its own inputs at ready and refuses there if the head needs the reader. + */ +export function receiptTargetedAtHead(root: string, baseRef: string, r: ShipReceipt, head: string): DepthVerdict { + const at = headInputs(root, baseRef, head); + const gateTagged = boundScan(r, head) ?? (gateScanNeeded(at.c.paths, at.c.root) ? null : 0); + const t = targetedClasses({ ...at.c, scope: at.scope, gateTagged }); + const v = receiptTargetedVerdict(t.classes, r); + return v.ok || !t.errors.length ? v : { ok: true, line: `${v.line} (unchecked: ${t.errors.join('; ')}; the queue decides at ready)` }; +} diff --git a/package.json b/package.json index 7a34868dc..720d53155 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.93.21", + "version": "1.93.22", "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/ship/DRIVER.md b/ship/DRIVER.md index ca721a499..41293bab8 100644 --- a/ship/DRIVER.md +++ b/ship/DRIVER.md @@ -88,8 +88,8 @@ Step 8.1, 9.2.1's probes, 8.2 (`Scope Check:`) on the todos and commits files, t one program. Prints `Pre-Landing Review:`, `Gate edits:`. `NEXT: stop` stops; `full` or `scoped`: one program runs the failing test, fix, `--tests-only` and Phase 3, each gated (`diffBase` RANGE_BASE on `full` only; scoped fix-check: Step 3.5); `fix-check`: the pass with `fixCheck: {fix_list, hunks, tests, callers}`; `done` or `voices`: that program runs Step 11 from -`sections/adversarial.md` as written (above), only the voices the last full-pass plan lists (neither -prints `ADVERSARIAL: skipped (review plan)`). +`sections/adversarial.md` as written (above), only the voices the last full-pass plan lists (none: +`ADVERSARIAL: skipped (review plan)`). ## 5. Voices, bind and version: Steps 11, 11.5, 12 @@ -101,7 +101,7 @@ approvals; `VERSION: skipped|kept`: no write. ## 6. Notes and documentation: Steps 13, 14, 14.5 -Step 13: one user-facing entry from the commits file, where `NEXT:` says; +Step 13: one user-facing entry from the commits file where `NEXT:` says; `gstack-prose-lint --base origin/ --gate` until `PROSE_LINT: ok`. Step 14: TODOS.md deferrals and completions, edited from the todos file. Step 14.5, every ship: read `sections/documentation.md` and run its audit as written, as one program (above), then `gstack-ship-receipt docs-caps --base `; print `Documentation:`. @@ -118,8 +118,8 @@ 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" --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; draft while a gate finding is `open`). `SHIP_RECEIPT:` opens your report. -- Step 20: the metrics row via `gstack-review-log`. +- Step 19 body: the receipt block, then the sections `sections/pr-body.md` lists (Pre-Landing Review: `REVIEW_LOOP:`, `Gate edits:`, deferred, notes), led under `CAPY_PR: on` (Step 19-C) by What changed, Why, Blast radius and Approvals (`gstack-approvals render`). `gstack-prose-lint --sections 'Why,Summary,Blast radius' --gate`; `gstack-issue-links`; `gstack-review-scope summary --since "$REVIEW_SINCE" --run && gstack-ship-receipt write --pr --base --gate @ --pregate --issues @ --review-loop @ --docs @ [--review-targeted @] --body `; `gstack-post pr-body` and `pr-title` (new PR: host PR tool; draft while a gate finding is `open`). `SHIP_RECEIPT:` opens your report. +- Step 20: a metrics row via `gstack-review-log`. - Step 22: `gstack-ship-budget close --reading R --credits C` (report `BUDGET_FINAL:`, `BUDGET_TIMELINE:` (tokens, credits), any `METER:` line), then `gstack-ship-receipt result --pr `: `GSTACK_RESULT: skill=ship status=complete run=`. Under a merge queue, `gstack-ship-queue ready --pr [--screen|--no-screen]` at the pushed head; paste its output. ## Where each STOP sends you (paths under `/ship/`) diff --git a/ship/DRIVER.md.tmpl b/ship/DRIVER.md.tmpl index a0f0d8ac1..48726e977 100644 --- a/ship/DRIVER.md.tmpl +++ b/ship/DRIVER.md.tmpl @@ -86,8 +86,8 @@ Step 8.1, 9.2.1's probes, 8.2 (`Scope Check:`) on the todos and commits files, t one program. Prints `Pre-Landing Review:`, `Gate edits:`. `NEXT: stop` stops; `full` or `scoped`: one program runs the failing test, fix, `--tests-only` and Phase 3, each gated (`diffBase` RANGE_BASE on `full` only; scoped fix-check: Step 3.5); `fix-check`: the pass with `fixCheck: {fix_list, hunks, tests, callers}`; `done` or `voices`: that program runs Step 11 from -`sections/adversarial.md` as written (above), only the voices the last full-pass plan lists (neither -prints `ADVERSARIAL: skipped (review plan)`). +`sections/adversarial.md` as written (above), only the voices the last full-pass plan lists (none: +`ADVERSARIAL: skipped (review plan)`). ## 5. Voices, bind and version: Steps 11, 11.5, 12 @@ -99,7 +99,7 @@ approvals; `VERSION: skipped|kept`: no write. ## 6. Notes and documentation: Steps 13, 14, 14.5 -Step 13: one user-facing entry from the commits file, where `NEXT:` says; +Step 13: one user-facing entry from the commits file where `NEXT:` says; `gstack-prose-lint --base origin/ --gate` until `PROSE_LINT: ok`. Step 14: TODOS.md deferrals and completions, edited from the todos file. Step 14.5, every ship: read `sections/documentation.md` and run its audit as written, as one program (above), then `gstack-ship-receipt docs-caps --base `; print `Documentation:`. @@ -116,8 +116,8 @@ 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" --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; draft while a gate finding is `open`). `SHIP_RECEIPT:` opens your report. -- Step 20: the metrics row via `gstack-review-log`. +- Step 19 body: the receipt block, then the sections `sections/pr-body.md` lists (Pre-Landing Review: `REVIEW_LOOP:`, `Gate edits:`, deferred, notes), led under `CAPY_PR: on` (Step 19-C) by What changed, Why, Blast radius and Approvals (`gstack-approvals render`). `gstack-prose-lint --sections 'Why,Summary,Blast radius' --gate`; `gstack-issue-links`; `gstack-review-scope summary --since "$REVIEW_SINCE" --run && gstack-ship-receipt write --pr --base --gate @ --pregate --issues @ --review-loop @ --docs @ [--review-targeted @] --body `; `gstack-post pr-body` and `pr-title` (new PR: host PR tool; draft while a gate finding is `open`). `SHIP_RECEIPT:` opens your report. +- Step 20: a metrics row via `gstack-review-log`. - Step 22: `gstack-ship-budget close --reading R --credits C` (report `BUDGET_FINAL:`, `BUDGET_TIMELINE:` (tokens, credits), any `METER:` line), then `gstack-ship-receipt result --pr `: `GSTACK_RESULT: skill=ship status=complete run=`. Under a merge queue, `gstack-ship-queue ready --pr [--screen|--no-screen]` at the pushed head; paste its output. ## Where each STOP sends you (paths under `/ship/`) diff --git a/ship/sections/adversarial.md b/ship/sections/adversarial.md index 97f66a3d8..396fa3245 100644 --- a/ship/sections/adversarial.md +++ b/ship/sections/adversarial.md @@ -5,7 +5,7 @@ `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. On `DISPATCH: targeted`, also run the targeted reader -(`gstack-review-dispatch render --role targeted --classes --base --diff-base `, the merge base on every pass, never RANGE_BASE) like the voices; `verify --targeted-out` feeds the receipt's `--review-targeted`. Under a token cap, +(`gstack-review-dispatch render --role targeted --classes --base --diff-base `, the merge base on every pass, never RANGE_BASE) like the voices; `verify --targeted-out .gstack/tmp/targeted.json` feeds the receipt's `--review-targeted`. Under a token cap, reserve the planned voices as one batch (`gstack-ship-budget check --next dispatch:`) and add `--reservation ` to the `--start adversarial-review` call below. After `NEXT: voices` (gate class: Step 11 before any stop), Finish the adversarial phase takes neither diff --git a/ship/sections/adversarial.md.tmpl b/ship/sections/adversarial.md.tmpl index 53bd90695..b4b739e0b 100644 --- a/ship/sections/adversarial.md.tmpl +++ b/ship/sections/adversarial.md.tmpl @@ -3,7 +3,7 @@ `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. On `DISPATCH: targeted`, also run the targeted reader -(`gstack-review-dispatch render --role targeted --classes --base --diff-base `, the merge base on every pass, never RANGE_BASE) like the voices; `verify --targeted-out` feeds the receipt's `--review-targeted`. Under a token cap, +(`gstack-review-dispatch render --role targeted --classes --base --diff-base `, the merge base on every pass, never RANGE_BASE) like the voices; `verify --targeted-out .gstack/tmp/targeted.json` feeds the receipt's `--review-targeted`. Under a token cap, reserve the planned voices as one batch (`gstack-ship-budget check --next dispatch:`) and add `--reservation ` to the `--start adversarial-review` call below. After `NEXT: voices` (gate class: Step 11 before any stop), Finish the adversarial phase takes neither diff --git a/ship/sections/handoff.md b/ship/sections/handoff.md index 55ea33fba..83f85320f 100644 --- a/ship/sections/handoff.md +++ b/ship/sections/handoff.md @@ -13,6 +13,7 @@ unattended run may do is docs/unattended.md#what-an-unattended-session-may-do. ~/.claude/skills/gstack/bin/gstack-issue-links --base --out .gstack/tmp/issues.json ~/.claude/skills/gstack/bin/gstack-ship-receipt write --base --gate @.gstack/tmp/ship-gate.json --issues @.gstack/tmp/issues.json --out .gstack/tmp/ship-receipt.md ``` + When Step 11 saved `.gstack/tmp/targeted.json`, add `--review-targeted @.gstack/tmp/targeted.json` to that write. 2. Send the completion message, identifiers first, in this order: the `SHIP_RECEIPT:` line and receipt block; `branch:` and `sha:` (`git rev-parse HEAD`), pushed or `not pushed`; `inputs:` the plan or issue, the base SHA and the spend diff --git a/ship/sections/handoff.md.tmpl b/ship/sections/handoff.md.tmpl index 273bad052..2c38144fc 100644 --- a/ship/sections/handoff.md.tmpl +++ b/ship/sections/handoff.md.tmpl @@ -11,6 +11,7 @@ unattended run may do is docs/unattended.md#what-an-unattended-session-may-do. ~/.claude/skills/gstack/bin/gstack-issue-links --base --out .gstack/tmp/issues.json ~/.claude/skills/gstack/bin/gstack-ship-receipt write --base --gate @.gstack/tmp/ship-gate.json --issues @.gstack/tmp/issues.json --out .gstack/tmp/ship-receipt.md ``` + When Step 11 saved `.gstack/tmp/targeted.json`, add `--review-targeted @.gstack/tmp/targeted.json` to that write. 2. Send the completion message, identifiers first, in this order: the `SHIP_RECEIPT:` line and receipt block; `branch:` and `sha:` (`git rev-parse HEAD`), pushed or `not pushed`; `inputs:` the plan or issue, the base SHA and the spend diff --git a/ship/sections/pr-body.md b/ship/sections/pr-body.md index 79af0c5d9..4b2a9120f 100644 --- a/ship/sections/pr-body.md +++ b/ship/sections/pr-body.md @@ -221,7 +221,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" --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" +~/.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 $([ -s .gstack/tmp/targeted.json ] && echo --review-targeted @.gstack/tmp/targeted.json) --body "$PR_BODY_FILE" ``` A Step 5 red-base pause adds `--base-red ` (a conditional receipt the queue diff --git a/ship/sections/pr-body.md.tmpl b/ship/sections/pr-body.md.tmpl index 611c1e553..c8d0cd9e7 100644 --- a/ship/sections/pr-body.md.tmpl +++ b/ship/sections/pr-body.md.tmpl @@ -204,7 +204,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" --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" +~/.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 $([ -s .gstack/tmp/targeted.json ] && echo --review-targeted @.gstack/tmp/targeted.json) --body "$PR_BODY_FILE" ``` A Step 5 red-base pause adds `--base-red ` (a conditional receipt the queue diff --git a/test/gstack-clean-producers.test.ts b/test/gstack-clean-producers.test.ts index 58781cd0b..daad0df41 100644 --- a/test/gstack-clean-producers.test.ts +++ b/test/gstack-clean-producers.test.ts @@ -36,6 +36,7 @@ const KEPT: Record = { 'ship-gate.json': '/ship handoff gate record', 'issues.json': '/ship issue links', 'docs-audit.json': '/ship documentation audit record', + 'targeted.json': '/ship Step 11 targeted reader record (gstack-review-dispatch verify --targeted-out)', 'tree-receipt.json': 'gstack-tree-receipt output', 'base-red.json': '/ship base-red record', 'review-*': '/ship review loop records', diff --git a/test/review-packets-removed.test.ts b/test/review-packets-removed.test.ts index fa40c3db5..039228592 100644 --- a/test/review-packets-removed.test.ts +++ b/test/review-packets-removed.test.ts @@ -8,7 +8,9 @@ import { describe, test, expect } from 'bun:test'; import { spawnSync } from 'child_process'; import { createHash } from 'crypto'; import * as fs from 'fs'; +import * as os from 'os'; import * as path from 'path'; +import { sectionTexts } from '../lib/review-brief'; const ROOT = path.join(import.meta.dir, '..'); const REMOVED = [ @@ -57,6 +59,7 @@ const NO_LOSS_REPLAYS = { 'L': "GSTA-167 variant G Arm L: live no-loss replay of one batched reader (testing and security plus scoped checklists) and the Red Team on its findings, 2 passes, scored L∘G1 against G1 on the paired rule (part 2)", 'GST-257': "GST-257 PR 1 dispatch-superset no-loss check (test/review-targeted.test.ts): on every fixture diff the branch dispatches main's reviewers at 8be7f21 unchanged plus only the targeted reader, and over the last 100 non-release commits on main it lost none; adds the targeted role and brief to the G1 and L bytes, whose existing reviewer text is unchanged", 'GST-850': "GST-850 dispatch-superset no-loss check: on the 26 GSTA-167 no-loss units the branch plan (wider security and money classes, write-up text in the batched diff) drops no reviewer role main dispatches, so no saved blocker can be lost, and over the last 100 main commits no commit with a gate or risk class drops depth (test/review-risk-routing.test.ts, test/review-risk-linear.test.ts); reviewer text is unchanged", + 'GST-986': "GST-986 brief-identity no-loss check (the brief-body test below): every reviewer brief lib/review-brief.ts cuts from ship/sections/adversarial.md is byte-identical to the bodies at the GST-257 pin; only the lead's Step 11 instruction naming the targeted record's path changed, outside the cut spans", 'GST-911-redos': "GST-911 ReDoS fix verdict-identity no-loss check (wave review-e7b551ebd865): SVG_REFUSED's url() entry, rewritten to run in linear time, refuses exactly the strings #3838's pattern refused (test/review-risk-redos.test.ts: 60,000 seeded strings and the hand-picked edges, and every SVG blob in the repository's history, with zero differences); SVG_MARKUP now refuses a DOCTYPE naming any document but W3C's SVG DTD (test/review-planner-misreads.test.ts), which only keeps more SVGs code and so drops no reviewer; GST-911's no-loss replay holds unchanged; reviewer text is unchanged", 'GST-911': "GST-911 no-loss replay (GSTA-249/13): the planner over the last 100 first-parent main commits to f5cb6ec1e, origin/main's code against this branch, gate scan fixed at 0, changes no commit's depth, class, risk classes, targeted classes or SCOPE_AUTH; the three misreads (a drawing-only SVG, a session-named data file, a bare base) are proven on fixtures (test/review-planner-misreads.test.ts), where every SVG that can run script stays code with the security class; reviewer text is unchanged", 'R51': "R51 risk-tier routing no-loss re-score (GSTA-167): the saved panel findings (349 records over 24 of the 26 no-loss units) re-scored against the branch's plan lose no blocker whose only finders are roles it drops, and every gate, security, user-data and money fixture keeps main's reviewers plus the full panel (test/review-targeted.test.ts, test/review-risk-routing.test.ts); reviewer text is unchanged", @@ -66,7 +69,7 @@ const NO_LOSS_REPLAYS = { type Replay = keyof typeof NO_LOSS_REPLAYS; const REVIEWER_BRIEFS: Record = { 'ship/sections/review-army.md': { sha: '7c58dfd1bf1f16f967186a4872191cb16ebff3be81a50ebbf19067020af3ec53', replay: 'GST-967' }, - 'ship/sections/adversarial.md': { sha: '5d712bb568573931108a79b1bd40b74db6189610f47c547099c13098ef67bef8', replay: 'GST-967' }, + 'ship/sections/adversarial.md': { sha: 'f492e2c1663b81f503872543e1ba616ff092ffeed23e7d993bd2f4d1bfcd5d9e', replay: 'GST-986' }, 'review/sections/review-army.md': { sha: '78329b64640260cd8831ca81a9189f3f8dcade9c406431631d3f2cd798cd3967', replay: '#3638' }, 'review/sections/adversarial.md': { sha: 'e6962fa25b78f362654f2954f79683e524d2ebd452d1494f8a2b099888e8e255', replay: 'G1' }, 'review/checklist.md': { sha: '607788afeb2c83f2dff1dccd58d212616ec72393d28cda2fa18690c1a83df699', replay: '#3638' }, @@ -87,7 +90,34 @@ const REVIEWER_BRIEFS: Record = { 'lib/review-risk.ts': { sha: '126eff0f1de6982e03dc24d8017c67e6807c91dfa518b18d2db3aa910aac911f', replay: 'GST-911' }, }; +/** The reviewer brief bodies lib/review-brief.ts cuts out of the ship sections, at the bytes replays L and GST-257 admitted. */ +const BRIEF_BODIES: Record = { + specialist: '1ebe47ddb3291a9e4fe90b5a260eb57e328144959549bc95c119b68d23652cb5', + framing: 'aad667b5ea4c6d23d930dafffa38178c04be388cd4364fb73759fc92af17bb63', + fixtures: '9517c86befe773e76acbf30a0642ed828e211b8ecb524325702072b26429ef16', + redTeam: '1cca7957d13f9da08434870290b92958f37750207047b92c9aa52a802da789e8', + native: 'ddb1041ec1e4dd373c3e4ab9017e49316e78c230055c1ef3db1bbba518b143a2', + outside: 'bd96406da28b03742384560425a333b6d6d7ed2be95bd2026c90e0039632cee2', +}; + describe('reviewer briefs stay at their no-loss-tested bytes (GSTA-167)', () => { + // Value: protects=the GST-986 repin of ship/sections/adversarial.md admits only a lead-instruction edit: every brief a reviewer + // reads renders byte-identical to the no-loss-tested bodies; fails_when=an edit inside a cut span changes a brief while the + // section pin moves; seam=sectionTexts, the function every render uses; no_claim=the checklists (pinned by file hash below) + test('every reviewer brief cut from the ship sections is byte-identical to its pinned body', () => { + const hashes = (root: string) => Object.fromEntries(Object.entries(sectionTexts(root)).map(([k, v]) => [k, createHash('sha256').update(v).digest('hex')])); + expect(hashes(ROOT)).toEqual(BRIEF_BODIES); + const copy = fs.mkdtempSync(path.join(os.tmpdir(), 'brief-bodies-')); + try { + fs.cpSync(path.join(ROOT, 'ship', 'sections'), path.join(copy, 'ship', 'sections'), { recursive: true }); + const adversarial = path.join(copy, 'ship', 'sections', 'adversarial.md'); + fs.writeFileSync(adversarial, fs.readFileSync(adversarial, 'utf8').replace('Recommendation:', 'Recommendation (short):')); + expect(hashes(copy)).not.toEqual(BRIEF_BODIES); + } finally { + fs.rmSync(copy, { recursive: true, force: true }); + } + }); + test('every pinned reviewer brief, checklist and depth rule hashes to its pin', () => { const drift = Object.entries(REVIEWER_BRIEFS).filter(([file, pin]) => { const text = fs.readFileSync(path.join(ROOT, file), 'utf8').replace(/\r\n/g, '\n'); diff --git a/test/review-scope-ledger-count.test.ts b/test/review-scope-ledger-count.test.ts index 955d57a61..b18d962f6 100644 --- a/test/review-scope-ledger-count.test.ts +++ b/test/review-scope-ledger-count.test.ts @@ -247,3 +247,55 @@ describe('GST-951: the receipt counts every blocker the readers found', () => { expect(both.err).toContain('(REVIEW_SCOPE_INPUT;'); }); }); + +describe('GST-986 item 5: fix-check passes and their findings count from the ledgers', () => { + // #3862 (reconstruction from its PR body; GST-963, pin 0ead1ce50): round 1's red-team CRITICAL sat in pass 1's row unclassified, + // fixed in be9480829; the first granted fix-check found a CRITICAL, fixed in 0ead1ce50; the second fix-check was clean; the + // targeted reader then raised 2 informational findings. Only pass 1 had a review-log row, so the receipt's tool counters read + // blocking_found=0 fixed=0 fix_checks=0 and the lane hand-set blocking_found=2 fixed=2 fix_checks=2 (its third, the docs + // re-check, ran outside the pass program). The outside voice's findings, which GStack Minder ruled below the floor, are left out. + // Value: protects=a fix-check that ran through gstack-review-dispatch counts as a round, and a blocker a later fix-check + // followed counts as fixed, without a review-log row; fails_when=summary counts fix_checks only from rows, or reports such + // blockers as unrecorded; seam=gstack-review-scope summary --run; no_claim=noted (a floor ruling lives only in rows) + test('#3862: two fix-check ledgers count as fix_checks=2, and the blockers they followed count as fixed', () => { + const repo = branchWith([review(0, 1, [{ fingerprint: 'lib/tree-receipt.ts:120:logic', raised_by: 'red-team' }])]); + const run = path.join(repo.root, 'run'); + const step = (dir: string, readers: Array<[string, string]>) => { pass(path.join(run, dir), readers); Bun.sleepSync(5); }; + step('p1', [['specialist-batched', CLEAN], ['red-team', '[CRITICAL] lib/tree-receipt.ts:120 the tree receipt floors the branch\'s own stamp row at the gated VERSION']]); + step('fc1', [['fix-check', '[CRITICAL] lib/ship-queue-train-stamp.ts:88 the train stamp and verify checks pass no baseRef']]); + step('fc2', [['fix-check', CLEAN]]); + step('s11', [['targeted', '- [INFORMATIONAL] lib/stamp-only.ts:14 a comment names the old bound\n- [INFORMATIONAL] lib/stamp-only.ts:30 the side argument has no default test']]); + const s = loop(summary(repo, '--run', run)); + expect(s).toMatchObject({ passes: 1, blocking_found: 2, fixed: 2, fix_checks: 2, rounds: 3, ledgers_read: 4 }); + expect(s.unrecorded).toBeUndefined(); + }); + + // Value: protects=a fix-check with both a row and a ledger counts once, and once every fix-check has its row the rows decide + // dispositions (a blocker no row disposes stays unrecorded); fails_when=the ledger count adds to the row count, or a logged + // fix-check turns undisposed blockers fixed; seam=gstack-review-scope summary --run + test('a fix-check counted by row and ledger counts once; with its row logged, undisposed blockers stay unrecorded', () => { + const repo = branchWith([ + review(0, 1, [{ fingerprint: 'lib/a.ts:5:logic', severity: 'CRITICAL', blocking: true, action: 'deferred', raised_by: 'red-team' }]), + fixCheck(30, []), + ]); + const run = path.join(repo.root, 'run'); + const step = (dir: string, readers: Array<[string, string]>) => { pass(path.join(run, dir), readers); Bun.sleepSync(5); }; + step('p1', [['red-team', '[CRITICAL] lib/a.ts:5 a deferred defect outside the diff\n[CRITICAL] lib/b.ts:9 a fixed defect']]); + step('fc1', [['fix-check', '[CRITICAL] lib/c.ts:3 the fix broke the caller']]); + const s = loop(summary(repo, '--run', run)); + expect(s).toMatchObject({ passes: 1, blocking_found: 3, deferred: 1, fixed: 0, fix_checks: 1, rounds: 2, unrecorded: 2 }); + }); + + // Value: protects=a blocker the last fix-check raises again is not fixed: only a fix-check after its last occurrence follows it + // (round 2 outside voice on this PR: the re-raised blocker read blocking_found=1 fixed=1 with nothing unrecorded); + // fails_when=an earlier occurrence's follow-up marks it fixed; seam=gstack-review-scope summary --run + test('a blocker the last fix-check raises again stays unrecorded', () => { + const repo = branchWith([review(0, 1, [{ fingerprint: 'lib/a.ts:10:logic', raised_by: 'red-team' }])]); + const run = path.join(repo.root, 'run'); + const step = (dir: string, readers: Array<[string, string]>) => { pass(path.join(run, dir), readers); Bun.sleepSync(5); }; + step('p1', [['red-team', '[CRITICAL] lib/a.ts:10 the cap is never enforced']]); + step('fc1', [['fix-check', '[CRITICAL] lib/a.ts:10 the cap is still never enforced']]); + const s = loop(summary(repo, '--run', run)); + expect(s).toMatchObject({ passes: 1, blocking_found: 1, fixed: 0, fix_checks: 1, unrecorded: 1 }); + }); +}); diff --git a/test/review-targeted-record.test.ts b/test/review-targeted-record.test.ts new file mode 100644 index 000000000..3eeb5909f --- /dev/null +++ b/test/review-targeted-record.test.ts @@ -0,0 +1,295 @@ +/** + * GST-986 (GST-966 part 2): the targeted reader's record from verify to the receipt. + * verify records what the reader did and the commit it read; write binds that record only to a receipt whose product patch + * is the one the reader read; write and carry refuse a receipt the queue's targeted verdict would refuse; /ship's receipt + * writes pass the record. Cases come from gstack-evals#140, C5-36/C6-36 item 5 and GSTA-234's outside-voice fixture on #3859. + */ +import { afterEach, describe, expect, setDefaultTimeout, test } from 'bun:test'; +import { spawnSync } from 'node:child_process'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { EXIT } from '../lib/headless-artifacts'; +import { checkoutTree } from '../lib/review-brief'; +import { TARGETED_RULES, targetedBriefSha } from '../lib/review-targeted'; +import { extractReceipt, placeReceiptFirst } from '../lib/ship-receipt'; +import { receiptDepthAtHead } from '../lib/ship-receipt-depth'; +import { cleanup, makeFixtureRepo, runBin, type FixtureRepo } from './helpers/restamp-fixture'; + +setDefaultTimeout(180_000); +const ROOT = path.resolve(import.meta.dir, '..'); +const repos: FixtureRepo[] = []; +afterEach(() => { while (repos.length) cleanup(repos.pop()!); }); + +function moneyBranch(): { repo: FixtureRepo; base: string; head: string } { + const repo = makeFixtureRepo({ prefix: 'gst986-' }); + repos.push(repo); + repo.git('checkout', '-q', '-b', 'feat'); + fs.mkdirSync(path.join(repo.work, 'src'), { recursive: true }); + fs.writeFileSync(path.join(repo.work, 'src', 'charge.ts'), 'export const spend_cap_usd = 5;\n'); + repo.git('add', '-A'); repo.git('commit', '-q', '-m', 'charge'); + return { repo, base: repo.git('merge-base', 'origin/main', 'HEAD'), head: repo.git('rev-parse', 'HEAD') }; +} +const commit = (repo: FixtureRepo, file: string, text: string, msg: string) => { + fs.mkdirSync(path.dirname(path.join(repo.work, file)), { recursive: true }); + fs.writeFileSync(path.join(repo.work, file), text); + repo.git('add', '-A'); repo.git('commit', '-q', '-m', msg); + return repo.git('rev-parse', 'HEAD'); +}; + +/** One targeted read through the real dispatch steps; `reply` is the reader's raw output after its REVIEWED_TREE line. */ +function targetedRead(repo: FixtureRepo, base: string, dir: string, reply: string | null, sent = true, classes = 'money'): string { + const out = path.join(repo.root, dir); + const ledger = path.join(out, 'review-ledger.jsonl'); + const render = runBin(repo, 'gstack-review-dispatch', ['render', '--role', 'targeted', '--classes', classes, '--diff-base', base, '--out', out]); + expect(render.status, render.err).toBe(0); + const file = /file=(\S+)/.exec(render.out)![1]!; + if (sent) expect(runBin(repo, 'gstack-review-dispatch', ['sent', '--ledger', ledger, '--role', 'targeted', '--sent', file]).status).toBe(0); + if (reply === null) return ledger; + const output = path.join(out, 'targeted.out'); + fs.writeFileSync(output, `REVIEWED_TREE: ${checkoutTree(repo.work)}\n${reply}\n`); + const merged = path.join(out, 'merged.jsonl'); + expect(runBin(repo, 'gstack-review-dispatch', ['collect', '--ledger', ledger, '--role', 'targeted', '--output', output, '--merged', merged]).status).toBe(0); + const judged = path.join(out, 'judged.jsonl'); + fs.writeFileSync(judged, fs.readFileSync(merged, 'utf8').split('\n').filter(Boolean).map(l => JSON.stringify({ id: JSON.parse(l).id, severity: JSON.parse(l).label }) + '\n').join('')); + expect(runBin(repo, 'gstack-review-dispatch', ['judge', '--ledger', ledger, '--merged', merged, '--judged', judged]).status).toBe(0); + return ledger; +} +const verify = (repo: FixtureRepo, ledger: string, record?: string) => + runBin(repo, 'gstack-review-dispatch', ['verify', '--ledger', ledger, ...(record ? ['--targeted-out', record] : [])]); +const recordLine = (out: string) => out.split('\n').find(l => l.startsWith('TARGETED_RECORD:')) ?? ''; +const finding = (line: number, summary: string) => JSON.stringify({ severity: 'INFORMATIONAL', confidence: 7, path: 'src/charge.ts', line, category: 'money', summary, fingerprint: `src/charge.ts:${line}:money`, specialist: 'targeted' }); +const write = (repo: FixtureRepo, args: string[]) => runBin(repo, 'gstack-ship-receipt', ['write', '--base', 'main', '--session-kind', 'unattended', ...args, '--json']); + +describe('verify records the targeted reader\'s outcome and the commit it read (item 1)', () => { + // Value: protects=a targeted reader that completed reads as done, with its findings counted, however the head moves after it + // (gstack-evals#140, C5-36 and C6-36 p2: Step 11 fixes moved the head and the clean or 2-finding read printed status=failed + // findings=0); a reader that did not complete stays failed; fails_when=verify fails a completed read because HEAD moved, zeroes + // its findings, or marks a read with no matching dispatch or output done; seam=gstack-review-dispatch verify --targeted-out; + // no_claim=binding the record to the receipt (write, below) + test('a clean read and a 2-finding read stay done after a later fix commit; an unsent or uncollected read is failed', () => { + const { repo, base, head } = moneyBranch(); + const clean = targetedRead(repo, base, 'clean', 'NO FINDINGS'); + const found = targetedRead(repo, base, 'found', `${finding(1, 'the cap is never enforced')}\n${finding(1, 'the cap has no unit test')}`); + const unsent = targetedRead(repo, base, 'unsent', 'NO FINDINGS', false); + const silent = targetedRead(repo, base, 'silent', null); + commit(repo, 'src/charge.ts', 'export const spend_cap_usd = 5; // enforced in run()\n', 'Step 11 fix after the reader'); + + const record = path.join(repo.root, 'targeted.json'); + const v = verify(repo, clean, record); + expect(recordLine(v.out)).toStartWith(`TARGETED_RECORD: classes=money status=done findings=0 brief=${targetedBriefSha().slice(0, 12)} rules=${TARGETED_RULES}`); + expect(JSON.parse(fs.readFileSync(record, 'utf8'))).toEqual({ classes: ['money'], rules: TARGETED_RULES, brief_sha: targetedBriefSha(), status: 'done', findings: 0, raised_by: 'targeted', head }); + expect(recordLine(verify(repo, found).out)).toStartWith('TARGETED_RECORD: classes=money status=done findings=2 '); + expect(recordLine(verify(repo, unsent).out)).toStartWith('TARGETED_RECORD: classes=money status=failed '); + expect(recordLine(verify(repo, silent).out)).toStartWith('TARGETED_RECORD: classes=money status=failed findings=0 '); + }); + + // Value: protects=a read rendered on uncommitted tracked edits never reads as done (nothing proves which patch it read); + // fails_when=the item 1 repair accepts it; seam=gstack-review-dispatch render + verify + test('a read rendered on uncommitted tracked edits is failed', () => { + const { repo, base } = moneyBranch(); + fs.writeFileSync(path.join(repo.work, 'src', 'charge.ts'), 'export const spend_cap_usd = 6;\n'); + const dirty = targetedRead(repo, base, 'dirty', 'NO FINDINGS'); + repo.git('checkout', '--', 'src/charge.ts'); + expect(recordLine(verify(repo, dirty).out)).toStartWith('TARGETED_RECORD: classes=money status=failed '); + }); + + // Value: protects=a run whose ledger holds no targeted render leaves no record for Step 19 to pass, so an earlier run's + // .gstack/tmp/targeted.json never reaches write (pass 1 Red Team note on this PR); fails_when=verify leaves the stale file; + // seam=gstack-ship-receipt write's input via gstack-review-dispatch verify --targeted-out + test('verify with no targeted render removes a stale --targeted-out file', () => { + const { repo, base } = moneyBranch(); + const out = path.join(repo.root, 'plain'); + const render = runBin(repo, 'gstack-review-dispatch', ['render', '--role', 'specialist', '--name', 'testing', '--diff-base', base, '--out', out]); + expect(render.status, render.err).toBe(0); + const record = path.join(repo.root, 'targeted.json'); + fs.writeFileSync(record, '{"stale":true}\n'); + const v = verify(repo, path.join(out, 'review-ledger.jsonl'), record); + expect(recordLine(v.out)).toBe(''); + expect(fs.existsSync(record)).toBe(false); + }); + + // Value: protects=a verify that stops early (an empty ledger: no brief records) still removes the stale record (round 2 Red + // Team and adversarial on this PR); fails_when=the early return leaves it; seam=gstack-review-dispatch verify --targeted-out + test('verify on a ledger with no brief records removes a stale --targeted-out file too', () => { + const { repo } = moneyBranch(); + const ledger = path.join(repo.root, 'empty-ledger.jsonl'); + fs.writeFileSync(ledger, ''); + const record = path.join(repo.root, 'targeted.json'); + fs.writeFileSync(record, '{"stale":true}\n'); + expect(verify(repo, ledger, record).status).toBe(EXIT.refused); + expect(fs.existsSync(record)).toBe(false); + }); +}); + +describe('write binds a targeted record only to the product patch the reader read (item 2)', () => { + // Value: protects=a record verified at H1 never vouches for H2's new product code (GSTA-234's outside-voice CRITICAL on #3859: + // write rebound the H1 record to H2's product patch-id and ready printed verdict=ok); a later commit that leaves the product + // patch alone (a test, the version stamp) still binds; fails_when=write accepts the stale record, rejects the unchanged + // product, or accepts a record that names no commit; seam=gstack-ship-receipt write --review-targeted + test('GSTA-234\'s fixture: an auth commit after verify is refused; a test-only commit binds', () => { + const { repo, base, head } = moneyBranch(); + const record = path.join(repo.root, 'targeted.json'); + expect(verify(repo, targetedRead(repo, base, 'h1', 'NO FINDINGS'), record).status).toBe(0); + + commit(repo, 'test/charge.test.ts', 'test("cap", () => {});\n', 'test only'); + const testOnly = write(repo, ['--review-targeted', `@${record}`]); + expect(testOnly.status, testOnly.err).toBe(EXIT.ok); + expect(JSON.parse(testOnly.out).review_targeted).toMatchObject({ classes: ['money'], status: 'done' }); + + commit(repo, 'src/session.ts', 'export function checkSession(token: string) { return token.length > 0; }\n', 'auth change after verify'); + const body = path.join(repo.root, 'body.md'); + const stale = write(repo, ['--review-targeted', `@${record}`, '--body', body]); + expect(stale.status).toBe(EXIT.fail); + expect(stale.err).toContain(`the targeted reader read ${head.slice(0, 12)}`); + expect(stale.err).toContain('(RECEIPT_INVALID'); + expect(fs.existsSync(body)).toBe(false); + + const { head: _drop, ...headless } = JSON.parse(fs.readFileSync(record, 'utf8')); + const old = write(repo, ['--review-targeted', JSON.stringify(headless)]); + expect(old.status).toBe(EXIT.fail); + expect(old.err).toContain('review_targeted.head'); + }); + + // Value: protects=a record binds only to the product patch the reader read, so a docs or TODOS.md commit after the read (which + // changes the product patch) is refused like a money edit; binding docs-only commits is planned under GST-1031 with a full + // review; fails_when=write binds a record whose read product patch is not the receipt's; seam=gstack-ship-receipt write --review-targeted + test('a docs and TODOS.md commit after the read is refused; a money edit after it is refused', () => { + const { repo, base, head } = moneyBranch(); + const record = path.join(repo.root, 'targeted.json'); + expect(verify(repo, targetedRead(repo, base, 'h1', 'NO FINDINGS'), record).status).toBe(0); + + commit(repo, 'docs/charges.md', '# Charges\n\nThe cap is enforced per run.\n', 'Step 14.5 docs'); + commit(repo, 'TODOS.md', '# TODOS\n\n- [x] enforce the cap\n', 'Step 14 TODOS'); + const docs = write(repo, ['--review-targeted', `@${record}`]); + expect(docs.status).toBe(EXIT.fail); + expect(docs.err).toContain(`the targeted reader read ${head.slice(0, 12)}`); + + commit(repo, 'src/charge.ts', 'export const spend_cap_usd = 50;\n', 'money edit after verify'); + const stale = write(repo, ['--review-targeted', `@${record}`]); + expect(stale.status).toBe(EXIT.fail); + expect(stale.err).toContain(`the targeted reader read ${head.slice(0, 12)}`); + }); + + // Value: protects=a code edit after the read that drops a cap's enforcing line carries no money token, so the classifier tags + // nothing, but the product patch the reader read is gone and the record must not bind (targeted gate reader on this PR at + // 071532752, CRITICAL lib/ship-receipt-depth.ts:153); fails_when=write binds a record whose read product patch is not the + // receipt's; seam=gstack-ship-receipt write --review-targeted + test('a code edit after the read that removes an enforcing line with no money token is refused', () => { + const { repo, base } = moneyBranch(); + commit(repo, 'src/runner.ts', 'export function run(spent: number, limit: number) {\n if (spent > limit) throw new Error("over");\n}\n', 'enforce the limit'); + const head = repo.git('rev-parse', 'HEAD'); + const record = path.join(repo.root, 'targeted.json'); + expect(verify(repo, targetedRead(repo, base, 'h1', 'NO FINDINGS'), record).status).toBe(0); + expect(write(repo, ['--review-targeted', `@${record}`]).status).toBe(EXIT.ok); + commit(repo, 'src/runner.ts', 'export function run(spent: number, limit: number) {\n}\n', 'drop the enforcing line'); + const dropped = write(repo, ['--review-targeted', `@${record}`]); + expect(dropped.status).toBe(EXIT.fail); + expect(dropped.err).toContain(`the targeted reader read ${head.slice(0, 12)}`); + }); + + // Value: protects=a test edit after the read that skips a case is gate-class work the reader never saw, so the record does not + // bind though the product patch (which leaves tests out) is unchanged (round 2 outside voice and adversarial on this PR); + // a record whose read commit is not an ancestor of the head is refused; fails_when=write binds either; + // seam=gstack-ship-receipt write --review-targeted + test('a test skipped after the read is refused though the product patch is unchanged; a rewritten read is refused', () => { + const { repo, base } = moneyBranch(); + const cases = (cap: string) => `import { test, expect } from "bun:test";\ntest.skip("unit", () => { expect(1).toBe(1); });\n${cap}("cap", () => { expect(5).toBe(5); });\n`; + commit(repo, 'test/charge.test.ts', cases('test'), 'cap test, unit case skipped'); + const head = repo.git('rev-parse', 'HEAD'); + const record = path.join(repo.root, 'targeted.json'); + expect(verify(repo, targetedRead(repo, base, 'h1', 'NO FINDINGS', true, 'money,gate'), record).status).toBe(0); + expect(write(repo, ['--review-targeted', `@${record}`]).status).toBe(EXIT.ok); + commit(repo, 'test/charge.test.ts', cases('test.skip'), 'skip the cap test'); + const skipped = write(repo, ['--review-targeted', `@${record}`]); + expect(skipped.status).toBe(EXIT.fail); + expect(skipped.err).toContain(`the targeted reader read ${head.slice(0, 12)}`); + + repo.git('reset', '-q', '--hard', base); + commit(repo, 'src/charge.ts', 'export const spend_cap_usd = 75;\n', 'rewritten money change'); + const rewritten = write(repo, ['--review-targeted', `@${record}`]); + expect(rewritten.status).toBe(EXIT.fail); + expect(rewritten.err).toContain(`the targeted reader read ${head.slice(0, 12)}`); + }); +}); + +describe('write and carry refuse a receipt the queue\'s targeted verdict refuses (item 4, #3854)', () => { + // Value: protects=/ship never publishes a receipt whose head needs a targeted class it did not cover (gstack-evals#140 wrote + // gstack_version with receipt=legacy); a head that needs none writes as before; fails_when=write records no or a failed pass + // on a money head, or refuses a head without classes; seam=gstack-ship-receipt write + receiptTargetedVerdict + test('write: missing and failed records are refused on a money head; a covered or classless head writes', () => { + const { repo, base } = moneyBranch(); + const missing = write(repo, []); + expect(missing.status).toBe(EXIT.fail); + expect(missing.err).toContain('REVIEW_TARGETED: required=money receipt=legacy verdict=missing'); + expect(missing.err).toMatch(/requires the targeted reader on money, but the receipt records no targeted pass.*\(RECEIPT_INVALID/); + const record = path.join(repo.root, 'targeted.json'); + verify(repo, targetedRead(repo, base, 'h1', 'NO FINDINGS'), record); + const failed = write(repo, ['--review-targeted', JSON.stringify({ ...JSON.parse(fs.readFileSync(record, 'utf8')), status: 'failed' })]); + expect(failed.status).toBe(EXIT.fail); + expect(failed.err).toContain('targeted pass failed'); + expect(write(repo, ['--review-targeted', `@${record}`]).status).toBe(EXIT.ok); + + const plain = makeFixtureRepo({ prefix: 'gst986-plain-' }); + repos.push(plain); + plain.git('checkout', '-q', '-b', 'feat'); + commit(plain, 'docs/notes.md', '# notes\n', 'docs'); + const none = write(plain, []); + expect(none.status, none.err).toBe(EXIT.ok); + expect(JSON.parse(none.out)).not.toHaveProperty('review_targeted'); + }); + + // Value: protects=carry never rebinds a receipt the queue would refuse on targeted coverage (#3854: carry checked depth only); + // a legacy receipt and a covered one carry as before; fails_when=carry prints CARRY: carried for an uncovered money head, or + // refuses a covered or legacy one; seam=gstack-ship-receipt carry + test('carry: an uncovered versioned receipt is refused; a covered or legacy one carries', () => { + const { repo, head } = moneyBranch(); + const saved = { HOME: process.env.HOME, GSTACK_HOME: process.env.GSTACK_HOME, GSTACK_STATE_DIR: process.env.GSTACK_STATE_DIR }; + Object.assign(process.env, { HOME: repo.home, GSTACK_HOME: repo.home, GSTACK_STATE_DIR: repo.home }); + let depth: ReturnType; + try { depth = receiptDepthAtHead(repo.work, 'main', path.join(ROOT, 'bin'), head); } finally { + for (const [k, v] of Object.entries(saved)) { if (v === undefined) delete process.env[k]; else process.env[k] = v; } + } + const versioned = { schema_version: 1, pr: 5, head, base: 'main', gstack_version: '1.92.75.0', review_depth: depth }; + const covered = { ...versioned, review_targeted: { classes: ['money'], rules: TARGETED_RULES, brief_sha: targetedBriefSha(), product_patch_id: 'p', status: 'done', findings: 0, raised_by: 'targeted' } }; + commit(repo, 'test/charge.test.ts', 'test("cap", () => {});\n', 'test only'); + let n = 0; + const carry = (receipt: object) => { + const body = path.join(repo.root, `body-${++n}.md`); + fs.writeFileSync(body, placeReceiptFirst('## Summary\n\nProse.\n', receipt as any)); + return { ...runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body], { GSTACK_STATE_ROOT: path.join(repo.root, 'state') }), body }; + }; + const uncovered = carry(versioned); + expect(uncovered.status).toBe(EXIT.refused); + expect(uncovered.err).toMatch(/requires the targeted reader on money, but the receipt records no targeted pass.*\(RECEIPT_CARRY_REFUSED/); + expect(uncovered.err).toContain('the queue refuses it too (#3859)'); + expect(extractReceipt(fs.readFileSync(uncovered.body, 'utf8')).receipt!.head).toBe(head); + const ok = carry(covered); + expect(ok.status, ok.err).toBe(EXIT.ok); + expect(ok.out).toContain('CARRY: carried from'); + const legacy = carry({ schema_version: 1, pr: 5, head, base: 'main' }); + expect(legacy.status, legacy.err).toBe(EXIT.ok); + }); +}); + +describe('/ship\'s receipt writes pass the targeted record (item 3, #3853)', () => { + // Value: protects=the record Step 11's verify writes reaches every receipt write /ship runs (Step 19, the handoff, the driver); + // fails_when=a write command omits --review-targeted or reads another path than verify writes; seam=the generated sections' + // command text expanded by bash; no_claim=the agent following it (prose) + test('verify writes .gstack/tmp/targeted.json and the Step 19, handoff and driver writes read it', () => { + const section = (name: string) => fs.readFileSync(path.join(ROOT, 'ship', 'sections', `${name}.md`), 'utf8'); + const recordPath = /verify --targeted-out (\S+?)`/.exec(section('adversarial'))?.[1]; + expect(recordPath).toBe('.gstack/tmp/targeted.json'); + const writeLine = section('pr-body').split('\n').find(l => l.includes('gstack-ship-receipt write --pr'))!; + const optional = /\$\(\[ -s (\S+) \] && echo (--review-targeted @\S+)\)/.exec(writeLine); + expect(optional?.slice(1)).toEqual([recordPath!, `--review-targeted @${recordPath}`]); + expect(section('handoff')).toContain(`When Step 11 saved \`${recordPath}\`, add \`--review-targeted @${recordPath}\` to that write.`); + expect(fs.readFileSync(path.join(ROOT, 'ship', 'DRIVER.md'), 'utf8')).toContain('[--review-targeted @] --body '); + + const { repo } = moneyBranch(); + const expand = () => spawnSync('bash', ['-c', `printf '%s\\n' ${optional![0]}`], { cwd: repo.work, encoding: 'utf8', timeout: 10_000 }).stdout.split('\n').filter(Boolean); + expect(expand()).toEqual([]); + fs.mkdirSync(path.join(repo.work, '.gstack', 'tmp'), { recursive: true }); + fs.writeFileSync(path.join(repo.work, recordPath!), '{}\n'); + expect(expand()).toEqual(['--review-targeted', `@${recordPath}`]); + }); +}); diff --git a/test/review-targeted.test.ts b/test/review-targeted.test.ts index 8cf6ca497..6c64e288d 100644 --- a/test/review-targeted.test.ts +++ b/test/review-targeted.test.ts @@ -298,14 +298,14 @@ describe('the targeted role end to end on a fixture repository', () => { expect(verify.status, verify.out).toBe(0); expect(verify.out).toContain(`TARGETED_RECORD: classes=money status=done findings=1 brief=${targetedBriefSha().slice(0, 12)} rules=${TARGETED_RULES}`); expect(verify.out).toContain('VERIFY: clean'); + expect(JSON.parse(fs.readFileSync(record, 'utf8')).head).toBe(head); fs.writeFileSync(path.join(repo.work, 'src', 'charge.ts'), 'export const spend_cap_usd = 50;\n'); repo.git('commit', '-qam', 'later product change the reader never read'); - expect(runBin(repo, 'gstack-review-dispatch', ['verify', '--ledger', ledger]).out).toContain('TARGETED_RECORD: classes=money status=failed findings=0'); + expect(runBin(repo, 'gstack-review-dispatch', ['verify', '--ledger', ledger]).out).toContain(`TARGETED_RECORD: classes=money status=done findings=1 brief=${targetedBriefSha().slice(0, 12)} rules=${TARGETED_RULES} read=${head.slice(0, 12)}`); + const later = runBin(repo, 'gstack-ship-receipt', ['write', '--base', 'main', '--session-kind', 'unattended', '--review-targeted', `@${record}`, '--json']); + expect(later.status).not.toBe(0); + expect(later.err).toContain(`the targeted reader read ${head.slice(0, 12)}`); repo.git('reset', '-q', '--hard', head); - fs.writeFileSync(path.join(repo.work, 'src', 'charge.ts'), 'export const spend_cap_usd = 7;\n'); - expect(runBin(repo, 'gstack-review-dispatch', ['verify', '--ledger', ledger]).out).toContain('TARGETED_RECORD: classes=money status=failed findings=0'); - repo.git('checkout', '--', 'src/charge.ts'); - expect(runBin(repo, 'gstack-review-dispatch', ['verify', '--ledger', ledger]).out).toContain('TARGETED_RECORD: classes=money status=done findings=1'); const body = path.join(repo.root, 'body.md'); const write = runBin(repo, 'gstack-ship-receipt', ['write', '--base', 'main', '--session-kind', 'unattended', '--review-targeted', `@${record}`, '--body', body, '--json']); @@ -316,8 +316,8 @@ describe('the targeted role end to end on a fixture repository', () => { expect(rebound.status).not.toBe(0); expect(rebound.err).toContain("write binds the pass to the receipt's product_patch_id"); const plain = runBin(repo, 'gstack-ship-receipt', ['write', '--base', 'main', '--session-kind', 'unattended', '--json']); - expect(plain.status, plain.err).toBe(0); - expect(JSON.parse(plain.out)).not.toHaveProperty('review_targeted'); + expect(plain.status).not.toBe(0); + expect(plain.err).toContain('REVIEW_TARGETED: required=money receipt=legacy verdict=missing'); const slug = runBin(repo, 'gstack-slug', ['--get', 'SLUG']).out.trim(); const logDir = path.join(repo.home, 'projects', slug); @@ -342,8 +342,11 @@ describe('the targeted role end to end on a fixture repository', () => { describe('the receipt record and the queue line', () => { const record = { classes: ['gate', 'money'], rules: TARGETED_RULES, brief_sha: 'a'.repeat(64), status: 'done', findings: 0, raised_by: 'targeted' }; test('parseReviewTargeted binds the receipt\'s product patch, orders the classes and refuses a malformed record', () => { - expect(parseReviewTargeted(record, 'p1')).toEqual({ ok: true, targeted: { ...record, classes: ['money', 'gate'], product_patch_id: 'p1' } as any }); - for (const bad of [{ ...record, classes: ['cost'] }, { ...record, status: 'skipped' }, { ...record, findings: -1 }, { ...record, brief_sha: 'abc' }, { ...record, extra: 1 }, null]) expect(parseReviewTargeted(bad, 'p1').ok).toBe(false); + const read = { ...record, head: 'c'.repeat(40) }; + const patchOf = (head: string) => (head === read.head ? 'p1' : undefined); + expect(parseReviewTargeted(read, 'p1', patchOf)).toEqual({ ok: true, targeted: { ...record, classes: ['money', 'gate'], product_patch_id: 'p1' } as any }); + for (const bad of [{ ...read, classes: ['cost'] }, { ...read, status: 'skipped' }, { ...read, findings: -1 }, { ...read, brief_sha: 'abc' }, { ...read, extra: 1 }, record, { ...read, head: 'abc' }, null]) expect(parseReviewTargeted(bad, 'p1', patchOf).ok).toBe(false); + expect(parseReviewTargeted(read, 'p2', patchOf)).toMatchObject({ ok: false, message: expect.stringContaining(`the targeted reader read ${'c'.repeat(12)}, whose product patch p1 is not this receipt's p2`) }); }); // Value: protects=the queue refuses an uncovered required class (GST-966); fails_when=the verdict misreads a legacy, failed or partial // receipt as covered, or a receipt without classes as required; seam=receiptTargetedVerdict (what ready, train build and land refuse on) diff --git a/test/ship-receipt-depth.test.ts b/test/ship-receipt-depth.test.ts index 2fd996f70..a3052bd06 100644 --- a/test/ship-receipt-depth.test.ts +++ b/test/ship-receipt-depth.test.ts @@ -19,6 +19,8 @@ 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]); +/** A done targeted record read at HEAD on every class: since GST-986, write refuses a money, gate, auth or secret head without one. */ +const targetedAtHead = (repo: FixtureRepo) => ['--review-targeted', JSON.stringify({ classes: ['money', 'gate', 'auth', 'secret'], rules: 'r', brief_sha: 'a'.repeat(64), status: 'done', findings: 0, raised_by: 'targeted', head: repo.git('rev-parse', 'HEAD') })]; 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'); @@ -62,11 +64,11 @@ describe('the queue refuses a receipt below the depth its head needs', () => { const repo = makeFixtureRepo(); repos.push(repo); const head = commit(repo, { 'lib/billing.js': lines(80) }); const body = path.join(repo.root, 'body.md'); - expect(writeReceipt(repo, body).status).toBe(EXIT.ok); + expect(writeReceipt(repo, body, targetedAtHead(repo)).status).toBe(EXIT.ok); expect(receiptIn(body).review_depth).toMatchObject({ depth: 'full', runtime_lines: 80, risk: ['money'] }); const base = repo.git('rev-parse', 'origin/main').slice(0, 12); const bind = (text: string) => bindShippedReceipt(repo.work, 'origin/main', text, '7', [head]); - const untargeted = bind(fs.readFileSync(body, 'utf8')); + const untargeted = bind(rewrite(body, r => { delete r.review_targeted; })); expect(!untargeted.ok && untargeted.code).toBe('QUEUE_RECEIPT_TARGETED_MISSING'); expect(!untargeted.ok && untargeted.depthLine).toContain('REVIEW_DEPTH: receipt=full required=full'); const good = bind(rewrite(body, r => { r.review_targeted = { classes: ['money'], rules: 'r', brief_sha: 'a'.repeat(64), product_patch_id: r.product_patch_id, status: 'done', findings: 0, raised_by: 'targeted' }; })); @@ -103,7 +105,7 @@ describe('the queue refuses a receipt below the depth its head needs', () => { 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(writeReceipt(repo, body, targetedAtHead(repo)).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'; })); diff --git a/test/size-ledger/ship/gst-986-targeted-recorder.json b/test/size-ledger/ship/gst-986-targeted-recorder.json new file mode 100644 index 000000000..14644dd14 --- /dev/null +++ b/test/size-ledger/ship/gst-986-targeted-recorder.json @@ -0,0 +1,4 @@ +{ + "bytes": 232, + "why": "GST-986: Step 11 verify writes the targeted record to .gstack/tmp/targeted.json (adversarial.md lead instruction); merged over GST-967" +}