From 0a1e4f0929bb6f6a6ef21296f52dc4434fead50e Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 05:06:23 +0000 Subject: [PATCH 1/7] fix(ship): receipt counts bind by finding id, every run kept, scoped re-check after a refused carry, READY tail, head= on the result line (GSTA-258 L4; GST-1136, GST-1168, GST-1179) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- bin/gstack-review-scope | 16 +++- bin/gstack-ship-receipt | 46 +++++++++- bin/gstack-skill-start | 2 +- changelog.d/gsta258-l4-ready-receipts.md | 17 ++++ docs/review-policy.md | 3 + docs/ship-policy.md | 6 ++ docs/troubleshooting.md | 23 +++++ docs/unattended.md | 28 +++--- lib/headless-artifacts.ts | 16 ++-- lib/review-ledger.ts | 11 ++- lib/review-scope.ts | 67 +++++++++++--- lib/ship-preflight.ts | 4 + lib/ship-queue.ts | 3 +- lib/ship-receipt.ts | 56 ++++++++++-- lib/ship-recheck.ts | 63 +++++++++++++ ship/DRIVER.md | 2 +- ship/DRIVER.md.tmpl | 2 +- ship/SKILL.md | 2 +- ship/SKILL.md.tmpl | 2 +- ship/sections/review-army.md | 11 ++- ship/sections/review-army.md.tmpl | 11 ++- test/gstack-ship-receipt-docs.test.ts | 2 +- test/gstack-ship-receipt.test.ts | 15 ++-- test/gstack-skill-start.test.ts | 4 +- test/review-ledger-verdict-line.test.ts | 38 ++++++++ test/review-loop-binding.test.ts | 76 ++++++++++++++++ test/review-scope.test.ts | 3 +- test/ship-receipt-result-head.test.ts | 33 +++++++ test/ship-receipt-runs.test.ts | 58 ++++++++++++ test/ship-recheck.test.ts | 88 +++++++++++++++++++ .../ship/gsta258-l4-ready-receipts.json | 4 + 31 files changed, 642 insertions(+), 70 deletions(-) create mode 100644 changelog.d/gsta258-l4-ready-receipts.md create mode 100644 lib/ship-recheck.ts create mode 100644 test/review-ledger-verdict-line.test.ts create mode 100644 test/review-loop-binding.test.ts create mode 100644 test/ship-receipt-result-head.test.ts create mode 100644 test/ship-receipt-runs.test.ts create mode 100644 test/ship-recheck.test.ts create mode 100644 test/size-ledger/ship/gsta258-l4-ready-receipts.json diff --git a/bin/gstack-review-scope b/bin/gstack-review-scope index bd8280e29..baf2e7ae2 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 { - ledgerPass, parseHunks, renderDeferredIssue, renderDeferredSection, reviewLoopLine, scopeFindings, scopeLine, summarizeReviewLoop, + ledgerPass, parseFixTrailers, 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'; @@ -48,7 +48,9 @@ const HELP = `gstack-review-scope — finding scope for /ship's review loop (doc blocking_found=… deferred=… noted=… (the ship receipt's --review-loop input); blocking_found also counts every CRITICAL the dispatch ledgers collected or judged (--run finds each review-ledger.jsonl under the run directory); unrecorded=N names - blockers no row gives a disposition. An unreadable ledger: REVIEW_SCOPE_LEDGER + blockers no row disposes; a row finding's ledger_id (/) and each + fix commit's Review-Finding: reader= trailer bind by id, and what cannot bind + prints REVIEW_SCOPE_UNBOUND: : (unbound=N). An unreadable ledger: REVIEW_SCOPE_LEDGER Defaults: --policy from the base branch's .gstack/ship-policy.json review_scope (frozen); an unresolvable base refuses (POLICY_SOURCE_UNAVAILABLE) unless --policy is passed; --findings - reads stdin; --branch the current branch. @@ -267,9 +269,15 @@ function cmdSummary(rest: string[], json: boolean): number { if (noLedger && files.length) return refuse('summary', 'REVIEW_SCOPE_INPUT', '--no-ledger contradicts --run and --ledger'); // The ledgers hold every reader's findings, the voices' too, before any row classifies them; a summary without them can undercount. if (!noLedger && !files.length) return refuse('summary', 'REVIEW_SCOPE_LEDGER', 'name the review\'s dispatch ledgers (--run or --ledger ), or --no-ledger when no reader ran through gstack-review-dispatch'); + // A row's ledger_id and a fix trailer name a finding as /: the ids restart in every pass's ledger. + const keyOf = (file: string) => (id: string) => `${path.basename(path.dirname(path.resolve(file)))}/${id}`; 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, files.map(ledgerPass)); + try { ledgers = files.map(f => ledgerBlockers(f).map(b => ({ ...b, key: keyOf(f)(b.id) }))); } catch (e: any) { return refuse('summary', 'REVIEW_SCOPE_LEDGER', e.message); } + const log = spawnSync('git', ['log', '--first-parent', '--no-merges', `--since=${since}`, '--format=%H%x00%(trailers:key=Review-Finding,valueonly,separator=%x1f)%x1e', 'HEAD'], { encoding: 'utf8', timeout: 60_000, maxBuffer: 64 * 1024 * 1024 }); + if (log.status !== 0) return refuse('summary', 'REVIEW_SCOPE_INPUT', `git log for Review-Finding trailers failed: ${(log.stderr ?? '').trim().split('\n')[0] ?? ''}`); + const trailers = log.stdout.split('\x1e').flatMap(rec => { const [sha, values] = rec.trim().split('\0'); return sha && values ? parseFixTrailers(sha, values.split('\x1f').map(v => v.trim()).filter(Boolean)) : []; }); + const summary = summarizeReviewLoop(rows, since, ledgers, files.map(f => ledgerPass(f, keyOf(f))), trailers); + for (const u of summary.unbound ?? []) process.stderr.write(`REVIEW_SCOPE_UNBOUND: ${u}\n`); 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 cc4e80dae..527160383 100755 --- a/bin/gstack-ship-receipt +++ b/bin/gstack-ship-receipt @@ -49,6 +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 { planRecheck, readRecheck, recheckPacket } from '../lib/ship-recheck'; import { deeper, depthBelow, exceptionProblems, parseReviewLoopException, parseReviewTargeted, receiptExclude, receiptReviewAtHead, receiptTargetedAtHead, reviewLoopExceptionLine, targetedClassesSince } from '../lib/ship-receipt-depth'; import { checkReceiptProse, proseLine } from '../lib/ship-receipt-prose'; import { resolveStateRoot } from '../lib/state-root'; @@ -66,7 +67,7 @@ const HELP = `gstack-ship-receipt — the \`\`\`gstack-ship-receipt\`\`\` block result --pr [--repo ] | --body [--head ] [--out ] /ship's terminal line: binds the receipt to HEAD (QUEUE_RECEIPT_MISSING / QUEUE_RECEIPT_STALE), writes /ship/runs/-/{ship-run.json,ship-receipt.md} and prints - GSTACK_RESULT: skill=ship status=complete run= (refused: status=refused run=-) + GSTACK_RESULT: skill=ship status=complete head= run= (refused: status=refused run=-) census --repo --since [--cohort ] the P7 adoption number over merged PRs (deduplicated by run and thread) carry --pr [--repo ] | --body [--base ] [--head ] [--pregate ] [--dry-run] [--out ] @@ -77,6 +78,8 @@ const HELP = `gstack-ship-receipt — the \`\`\`gstack-ship-receipt\`\`\` block prints GSTACK_RESULT: skill=ship status=complete; RECEIPT_CARRY_PRODUCT_CHANGED / RECEIPT_CARRY_TREE_MISMATCH / RECEIPT_CARRY_REFUSED leave the body unchanged (exit 3); a receipt that records a pregate needs --pregate current for HEAD (RECEIPT_CARRY_PREGATE_STALE); + a product change on linear history whose range-diff is within the re-check cap prints + RECHECK: eligible … packet=: one reader reads it, then --recheck (J1); needs git >= 2.39 (RECEIPT_GIT_TOO_OLD) docs-caps [--base ] /ship Step 14.5: one DOCS_CAP: / ok|over line per docs.caps entry of the reviewed policy (origin/'s .gstack/ship-policy.json; --policy reads that file @@ -99,6 +102,8 @@ write flags: --pr --head --base --version --gated-tree --predecessor --gate (the digest= on gstack-review-dispatch verify's REVIEW_EXCLUDE_RECORD: line, which the reviewers' briefs name; none without that line): must equal the digest HEAD has under the base policy's review_exclude_paths, else RECEIPT_INVALID field=review_exclude and nothing is written (rerun the review; write alone cannot clear it); recorded as review_exclude. Carry and the queue compare that digest. + --no-stop (a JSON array of {class, reason, evidence}: each routine stop the run recorded and went on past, class one of + scoped-fix-check, test-timeout-fix, pregate-flake-rerun, stamp-only-carry, autoplan-regate); recorded as no_stop. --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 @@ -126,6 +131,13 @@ function jsonArg(raw: string | undefined): Record | undefined { if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) throw new Error('expected a JSON object'); return parsed; } +/** --no-stop : the routine stops this run recorded and went on past (K1), a JSON array of {class, reason, evidence}. */ +function noStopArg(raw: string | undefined): ShipReceipt['no_stop'] { + if (raw === undefined) return undefined; + const parsed = JSON.parse(raw.startsWith('@') ? fs.readFileSync(raw.slice(1), 'utf8') : raw); + if (!Array.isArray(parsed)) throw new Error('--no-stop expects a JSON array of {class, reason, evidence}'); + return parsed.length ? parsed : undefined; +} function git(args: string[], cwd?: string): string | undefined { const r = spawnSync('git', args, { cwd, encoding: 'utf8', timeout: 30_000, maxBuffer: 256 * 1024 * 1024 }); return r.status === 0 ? r.stdout.trim() : undefined; @@ -277,6 +289,7 @@ function cmdWrite(rest: string[], json: boolean): number { gstack_version: runningGstackVersion(), review_depth: reviewDepth, review_loop_exception: loopException?.ok ? loopException.exception : undefined, review_targeted: targeted?.ok ? targeted.targeted : undefined, base_red: pause ? receiptBaseRed(pause) : undefined, review_exclude: receiptExclude(review.exclude), + no_stop: noStopArg(flag(rest, '--no-stop')), }; } catch (e: any) { process.stderr.write(`gstack-ship-receipt write: ${e.message}\n`); return EXIT.usage; } if (input.pr !== undefined && !Number.isInteger(input.pr)) { process.stderr.write('gstack-ship-receipt write: --pr must be an integer\n'); return EXIT.usage; } @@ -349,7 +362,7 @@ function cmdRead(rest: string[], json: boolean): number { * /ship's terminal line (docs/unattended.md#the-terminal-line). The receipt in * the PR body (or the handoff file) must bind to HEAD: then the run record * goes under /ship/runs/ with the head, the PR and the receipt, and - * the last stdout line is GSTACK_RESULT: skill=ship status=complete run=. + * the last stdout line is GSTACK_RESULT: skill=ship status=complete head= run=. * A missing or stale receipt is status=refused run=- (exit 3), so a parent * never has to parse the PR body to know whether /ship finished at this head. */ @@ -416,7 +429,7 @@ function writeRunRecord(a: { receipt: ShipReceipt; head: string; pr: string | un }; try { fs.writeFileSync(path.join(dir, 'ship-run.json'), JSON.stringify(run, null, 2) + '\n'); } catch (e) { return a.incomplete(`cannot write ${path.join(dir, 'ship-run.json')}: ${e instanceof Error ? e.message : String(e)}`); } process.stderr.write(a.line + '\n'); - process.stdout.write(a.json ? JSON.stringify({ ...run, run: dir }) + '\n' : `GSTACK_RESULT: skill=ship status=${status} run=${dir}\n`); + process.stdout.write(a.json ? JSON.stringify({ ...run, run: dir }) + '\n' : `GSTACK_RESULT: skill=ship status=${status} head=${a.head} run=${dir}\n`); return status === 'complete' ? EXIT.ok : EXIT.fail; } @@ -648,6 +661,7 @@ function cmdCarry(rest: string[], json: boolean): number { const retarget = reviewedBase !== base.replace(/^origin\//, ''); let plan: ReturnType; let scopedDryRun: string | undefined; + let recheck: string | undefined; if (!ancestor) { // Rewritten history: the receipt follows only onto a tree it already covers, with the product diff it recorded. const recorded = receipt.product_patch_id !== undefined ? (receipt.product_patch_id === 'none' ? '' : receipt.product_patch_id) : fromKnown ? productPatchAgainstBase(repoRoot, rules, baseRef, from) : { error: 'the receipt records no product patch-id and its head is not in this clone' }; @@ -745,7 +759,29 @@ function cmdCarry(rest: string[], json: boolean): number { // Evidence for the record, never the decision: a slow range-diff is reported unavailable rather than waited for. const rangeRaw = spawnSync('git', ['range-diff', '--no-color', `${fromBase}..${from}`, `${headBase}..${head}`], { cwd: repoRoot, encoding: 'utf8', timeout: 15_000, maxBuffer: 64 * 1024 * 1024 }); const rangeDiff = rangeRaw.status === 0 ? (rangeRaw.stdout.trim().split('\n').slice(0, 20).join('\n') || '(empty: identical ranges)') : `unavailable: ${(rangeRaw.stderr ?? '').trim().split('\n')[0] || (rangeRaw.signal ? `range-diff killed by ${rangeRaw.signal} after 15 s` : 'range-diff failed')}`; - plan = planCarry({ from, head, changed, productPatchId: { from: reviewed as string, head: current as string }, testPatchId: { from: testFrom, head: testHead }, changedProduct, mergeCommits: Number(mergeCommits) || 0, rangeDiff, ...(retarget ? { retargetedFrom: reviewedBase } : {}), ...(scopedReview ? { scopedReview } : {}), ...(stampedRows.length ? { stampedRows } : {}) }); + // J1: a bounded product change since the reviewed head gets one reader on the range-diff instead of a full /ship (lib/ship-recheck.ts). + if (changedProduct.length) { + const named = changedProduct.filter(p => !p.startsWith('(') && !p.includes(' (')); + const inter = named.length ? spawnSync('git', ['diff', '--no-color', '--no-ext-diff', from, head, '--', ...named.map(p => `:(literal)${p}`)], { cwd: repoRoot, encoding: 'utf8', timeout: 30_000, maxBuffer: 64 * 1024 * 1024 }) : null; + const fullRange = rangeRaw.status === 0 && inter?.status === 0 ? `${rangeRaw.stdout.trim()}\n${inter.stdout.trim()}` : null; + const r = planRecheck({ code: 'RECEIPT_CARRY_PRODUCT_CHANGED', ancestor, retarget, rangeDiff: fullRange, unnamedPaths: named.length !== changedProduct.length }); + const record = flag(rest, '--recheck'); + if (!r.ok) process.stderr.write(`RECHECK: not eligible (${r.reason}); run /ship on ${head.slice(0, 12)}\n`); + else if (record) { + const read = readRecheck(record, { from, head, sha256: r.sha256 }); + if (!read.ok) return refused('RECEIPT_CARRY_REFUSED', `--recheck: ${read.message}`); + recheck = `scoped re-check by ${read.record.reader} (range-diff ${r.lines} lines)`; + } else { + let where = 'not written (dry run)'; + if (!rest.includes('--dry-run')) { + const packet = path.resolve(repoRoot, git(['rev-parse', '--git-path', `gstack/recheck-${head.slice(0, 12)}.md`], repoRoot) ?? `.git/gstack/recheck-${head.slice(0, 12)}.md`); + try { fs.mkdirSync(path.dirname(packet), { recursive: true }); fs.writeFileSync(packet, recheckPacket({ from, head, plan: r, rangeDiff: fullRange! })); where = packet; } + catch (e) { where = `unwritable: ${(e as Error).message.split('\n')[0]}`; } + } + process.stderr.write(`RECHECK: eligible range_diff_lines=${r.lines} range_diff_sha256=${r.sha256} packet=${where}; one reader reads it, then carry --recheck ({"from":"${from}","head":"${head}","range_diff_sha256":"${r.sha256}","reader":"","verdict":"clean"}); a blocking finding means /ship on ${head.slice(0, 12)}\n`); + } + } + plan = planCarry({ from, head, changed, productPatchId: { from: reviewed as string, head: current as string }, testPatchId: { from: testFrom, head: testHead }, changedProduct, mergeCommits: Number(mergeCommits) || 0, rangeDiff, ...(retarget ? { retargetedFrom: reviewedBase } : {}), ...(scopedReview ? { scopedReview } : {}), ...(stampedRows.length ? { stampedRows } : {}), ...(recheck ? { recheck } : {}) }); } if (!plan.ok) return refused(plan.code, plan.message); // Eligibility is not publication: a receipt that recorded a pregate is rebound only with a pregate current for HEAD, @@ -774,6 +810,8 @@ function cmdCarry(rest: string[], json: boolean): number { if (review.exclude.state === 'refused') return refused('RECEIPT_CARRY_REFUSED', `${review.exclude.lines.join('; ')}; run /ship on ${head.slice(0, 12)} once the base policy reads`); const digest = review.exclude.state === 'active' ? review.exclude.record.digest : 'none'; if (digest !== (receipt.review_exclude?.digest ?? 'none')) return refused('RECEIPT_CARRY_REFUSED', `review_exclude changed: recorded=${receipt.review_exclude?.digest ?? 'none'} now=${digest} source=${review.exclude.state === 'active' ? review.exclude.record.source : `${base} (no review_exclude_paths)`}; the reviewers read another selection than HEAD ${head.slice(0, 12)} has under the base policy, so carry does not apply; run /ship on ${head.slice(0, 12)}`); + const riskOf = (d: { class: string; risk?: readonly string[] }) => `${d.class}:${[...(d.risk ?? [])].sort().join(',')}`; + if (recheck && riskOf(now) !== riskOf(receipt.review_depth)) return refused('RECEIPT_CARRY_REFUSED', `HEAD ${head.slice(0, 12)} is risk class ${riskOf(now)}, not the reviewed ${riskOf(receipt.review_depth)}, so a scoped re-check cannot stand in for review; run /ship on ${head.slice(0, 12)}`); if (depthBelow(receipt.review_depth.depth, now.depth)) return refused('RECEIPT_CARRY_REFUSED', `HEAD ${head.slice(0, 12)} needs ${now.depth} review depth (${now.reasons.join(', ')}), above the receipt's ${receipt.review_depth.depth}; run /ship on ${head.slice(0, 12)}`); depthField = { review_depth: { ...now, depth: deeper(receipt.review_depth.depth, now.depth) }, review_exclude: receiptExclude(review.exclude) }; } diff --git a/bin/gstack-skill-start b/bin/gstack-skill-start index 16b1c9133..e07e5d2e9 100755 --- a/bin/gstack-skill-start +++ b/bin/gstack-skill-start @@ -743,7 +743,7 @@ if [ "$_SESSION_KIND" = "unattended" ]; then case "$SKILL_NAME" in autoplan) _UA_RECORD="This skill writes a run directory through gstack-autoplan (plan.md, review-record.md, tasks.jsonl, decisions.jsonl, findings.jsonl, timing.json, run.json, gate.json): append each recommended choice to decisions.jsonl with kind \"auto\" and each pending one with kind \"approval\" and status \"pending\". The final approval gate is written as a gate list (gstack-gate render) and this run ENDS there with \`GSTACK_RESULT: skill=autoplan status=gate_pending run=\`." ;; eval-plan) _UA_RECORD="This run ENDS at the Step 4 spend approval, left pending, with \`GSTACK_RESULT: skill=eval-plan status=gate_pending run=\`, where is the pilot's --out directory. No helper writes a gate.json for this skill yet, so the parent reads the pilot report and spend ledger in ." ;; - ship) _UA_RECORD="This skill ends with \`gstack-ship-receipt result\` (Step 22), which writes /ship/runs/-/ship-run.json (a /ship record, not a run manifest) and prints \`GSTACK_RESULT: skill=ship status=complete run=\`; a run that stops earlier prints \`GSTACK_RESULT: skill=ship status=refused run=-\` itself. The one publication allowed here: when an open PR for this branch exists and HEAD is its pushed head, update that PR's body and title with the receipt through the pr-body section's existing-PR path (never pr-create, never a push, and never \`gh pr ready\`: the draft-state items there concern an early PR this run never opened); otherwise the handoff section writes the receipt to .gstack/tmp/ship-receipt.md and the parent reads it there." ;; + ship) _UA_RECORD="This skill ends with \`gstack-ship-receipt result\` (Step 22), which writes /ship/runs/-/ship-run.json (a /ship record, not a run manifest) and prints \`GSTACK_RESULT: skill=ship status=complete head= run=\`; a run that stops earlier prints \`GSTACK_RESULT: skill=ship status=refused run=-\` itself. The one publication allowed here: when an open PR for this branch exists and HEAD is its pushed head, update that PR's body and title with the receipt through the pr-body section's existing-PR path (never pr-create, never a push, and never \`gh pr ready\`: the draft-state items there concern an early PR this run never opened); otherwise the handoff section writes the receipt to .gstack/tmp/ship-receipt.md and the parent reads it there." ;; review) _UA_RECORD="$_UA_NONE The parent reads the review-log rows (\`gstack-review-read\`)." ;; qa) _UA_RECORD="$_UA_NONE The parent reads the \`qa-report-*.md\` file." ;; *) _UA_RECORD="$_UA_NONE The parent reads this skill's own record (docs/unattended.md)." ;; diff --git a/changelog.d/gsta258-l4-ready-receipts.md b/changelog.d/gsta258-l4-ready-receipts.md new file mode 100644 index 000000000..0790d9a30 --- /dev/null +++ b/changelog.d/gsta258-l4-ready-receipts.md @@ -0,0 +1,17 @@ +**/ship's receipt counts its review findings by id, keeps every run, and a small change after review gets one reader instead of a second /ship (GSTA-258 L4; GST-1136, GST-1168, GST-1179).** The receipt's `REVIEW_LOOP` counts were wrong in a repeatable way: gbrain-evals#310's receipt said 3 blockers found and 2 fixed when 2 were found, 1 fixed and 1 deferred; gstack-evals#174 counted 16 blocking with 8 unrecorded against 9 found and 9 fixed; #3906 and #3911 showed `fixed=0` after real fixes. The summary paired findings by path:line only, so a native finding with no path:line, a row logged after its fix moved the line, or a late defer re-logged under another fingerprint all broke the count, and rows it couldn't place were dropped without a word. + +### Changed +- **`gstack-review-scope summary` binds by finding id.** A row finding carries `ledger_id` (`/`), each fix commit names what it fixed in a `Review-Finding: reader=` trailer, rows that record their head dedupe by kind, pass, head and finding ids, and a late defer counts once. Rows and findings without an id keep the path:line pairing. +- **What can't bind is named, never a silent zero.** A row with no start token or readable time, a `pN` ledger with no review row, a `ledger_id` no ledger holds and a trailer naming nothing each print `REVIEW_SCOPE_UNBOUND: : `, and the line and receipt carry `unbound=N`. +- **The native reader's own verdict line no longer collects as a blocker.** "There are three low-severity findings, all FIXABLE, and none blocks." (#3901) collected as CRITICAL beside the findings it summarized; it now collects at FIXABLE. A line that says something blocks, names a file:line or leads with FIXABLE still collects as CRITICAL. +- **`gstack-ship-receipt carry` offers a scoped re-check (J1).** On `RECEIPT_CARRY_PRODUCT_CHANGED` with linear history against the reviewed base and a range-diff (plus the diff of the changed product paths) of at most 400 lines, carry prints `RECHECK: eligible … packet=`. One reader on the packet and `carry --recheck ` rebind the receipt, unless HEAD needs a deeper review depth or another risk class. Rebased branches, retargeted stacks and anything over the cap still need /ship. +- **Every /ship run's receipt is kept.** When `write` or `carry` replaces a different receipt, the old one's identifiers and counters move to an append-only `gstack-receipt-runs` block in the PR body (newest 30). +- **`gstack-ship-queue ready` prints the whole READY.** It adds the receipt's `REVIEW_LOOP:` line, a `TRIAL_MERGES:` line for the lane to fill and an `Attention:` stub that opens with `reviewed by ` on risk-class PRs and `self-audited` on every other PR. docs/unattended.md now asks for the cross-family trail read only on risk-class PRs. +- **The `GSTACK_RESULT:` line names the full head** (`status=complete head= run=`), so a leftover line from an earlier ship can't pass for a new head (GST-1179). `parseResult` reads the optional token. +- **The receipt schema has `no_stop`**, one `{class, reason, evidence}` record per routine stop a run went on past (`write --no-stop`), for the five no-stop defaults that come next. + +### Tests +- `test/review-loop-binding.test.ts`: the GBEV-3, GEVA-223, #3906, #3892/#3903/#3911 and #3911 duplicate-row cases. On main the GBEV-3 case reproduces its receipt exactly (3 found, 2 fixed) and GEVA-223's pattern (18 found, 9 unrecorded). +- `test/review-ledger-verdict-line.test.ts`: the captured #3901 output, with negative controls. +- `test/ship-recheck.test.ts`: every refusal that does and doesn't qualify, the record binding, and the carry end to end. +- `test/ship-receipt-runs.test.ts`, `test/ship-receipt-result-head.test.ts`: the runs block, the READY tail, `no_stop` validation and the `head=` token. diff --git a/docs/review-policy.md b/docs/review-policy.md index 89e4681d0..c39b61b94 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -461,7 +461,10 @@ puts in the receipt: | `review_loop.passes` | full review passes this ship ran (1 or 2; up to 3 under `review_scope: all`) | | `review_loop.blocking_found` | every blocker found (GST-951): the rows' blocking findings (classified, or an unclassified CRITICAL), plus each ledger CRITICAL (as collected or judged) that no row finding from the same reviewer at the same path:line accounts for (each row finding pairs with one per ledger), so an adversarial, outside-voice or not-yet-classified blocker counts and one whose newest classification is below the floor does not. An unreadable ledger, merged file or judged set, or a judged set changed since `judge`, is `REVIEW_SCOPE_LEDGER`, never a count | | `review_loop.unrecorded` | blockers found that no row gives a disposition (fixed, deferred, noted, skipped, a gate answer); absent when 0 | +| `review_loop.unbound` | what could not bind (GST-1168), each printed as `REVIEW_SCOPE_UNBOUND: : `: a row with no start token and no readable time, a `pN` ledger with no review row of pass N, a row finding's `ledger_id` no ledger collected, a `Review-Finding` trailer naming nothing; absent when 0 | | `review_loop.ledgers_read` | dispatch ledgers the count read; absent under `--no-ledger` | + +Findings bind by id first (GST-1168). A row finding that came from a dispatched reader carries `ledger_id` (`/`, such as `p2/adversarial-native#3`) and pairs with that ledger entry whatever its path:line says, so a native finding with no path:line, or a row logged after its fix moved the line, still pairs. Each fix commit names what it fixed in a `Review-Finding: reader=` trailer, and a trailer counts the finding as fixed. A late defer re-logs its finding under the same id, so it counts once. Rows that record their head dedupe by kind, pass, head and finding ids, so a logger command run twice counts once. Rows and findings without an id keep the path:line pairing above. The native reader's own verdict line ("all FIXABLE, and none blocks", with no file:line, list mark, lead label or Severity/Blocks field) collects at FIXABLE, not CRITICAL; the findings it summarizes collect on their own lines. | `review_loop.deferred` | blocking findings in unchanged code filed as issues (a Step 11 late defer, logged as a `review-late-defer` row so it never displaces the newest `review` pass, counts here and is not a pass) | | `review_loop.noted` | below-floor findings listed in the PR body | | `review_loop.fixed` | findings fixed in this PR | diff --git a/docs/ship-policy.md b/docs/ship-policy.md index a6fcabd18..ec6a2b47c 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -1591,6 +1591,12 @@ base context next to the branch's unchanged hunks, which is [`RECEIPT_CARRY_SCOPED_REVIEW`](troubleshooting.md#receipt-carry-scoped-review) until one fix-check reviewer clears the packet; a working tree with uncommitted changes is [`RECEIPT_CARRY_REFUSED`](troubleshooting.md#receipt-carry-refused). +A `RECEIPT_CARRY_PRODUCT_CHANGED` on linear history against the reviewed base +whose range-diff, plus the diff of the changed product paths, is at most 400 +lines also prints `RECHECK: eligible … packet=`: one reader on that +packet and `carry --recheck ` rebind the receipt instead of a +full second /ship, unless HEAD needs a deeper review depth or another risk +class ([scoped re-check](troubleshooting.md#receipt-carry-recheck), J1). **Rewritten history.** `write` records `head_tree` (the reviewed head's tree) and `product_patch_id` (its product patch against the base), and every carry diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 7ae72bf86..799443897 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -6025,6 +6025,29 @@ none of carry's other checks would refuse; carry without `--dry-run` writes it. In /ship Step 1, `gstack-ship-preflight` prints the route as `NEXT: carry route (Step 1.4)`. + +### Scoped re-check after `RECEIPT_CARRY_PRODUCT_CHANGED` (J1) + +**Meaning.** The product diff changed since the receipt's head, so the receipt +can't carry as it is. When the history is linear, the base is the one the +receipt was reviewed against, and `git range-diff` of the reviewed range +against the current one is at most 400 lines (`RECHECK_RANGE_DIFF_CAP` in +`lib/ship-recheck.ts`), carry also prints `RECHECK: eligible +range_diff_lines=N range_diff_sha256= packet=`: one reader on that +range-diff can clear it instead of a full second /ship. A rebased branch +(`RECEIPT_CARRY_TREE_MISMATCH`), a retargeted stack, an empty or unreadable +range-diff and anything over the cap print `RECHECK: not eligible ()` +instead. + +**Fix.** Dispatch one reader on the packet (`.git/gstack/recheck-.md`; +`--dry-run` writes none). When it comes back clean, write +`{"from":"","head":"","range_diff_sha256":"","reader":"","verdict":"clean"}` +and rerun `gstack-ship-receipt carry ... --recheck `. The carry +reason records the reader and the range-diff size. Carry still refuses when +HEAD needs a deeper review depth or a different risk class than the receipt +recorded; that, or a blocking finding, means /ship on the new head. In /ship +Step 1, `gstack-ship-preflight` prints this as `NEXT: carry route (Step 1.4, J1)`. + ### `RECEIPT_PROSE_MISMATCH` diff --git a/docs/unattended.md b/docs/unattended.md index 08703d6b6..c4cd4d4f0 100644 --- a/docs/unattended.md +++ b/docs/unattended.md @@ -87,7 +87,7 @@ Not every workflow writes this line or a run directory today: - **`/ship`** ends with `gstack-ship-receipt result --pr ` (or `--body .gstack/tmp/ship-receipt.md` after the handoff section), which binds the receipt to the branch head, writes the run record and prints - `GSTACK_RESULT: skill=ship status=complete run=`. That line and that + `GSTACK_RESULT: skill=ship status=complete head= run=`. That line and that record are the machine-readable READY evidence; [What /ship leaves for a parent](#ship-evidence) below is the whole contract (the refused and incomplete lines, the one publication an unattended run may make, the @@ -131,7 +131,7 @@ in the PR body (or the handoff file) to HEAD, writes `/ship/runs/-/` and prints the terminal line: ``` -GSTACK_RESULT: skill=ship status=complete run= +GSTACK_RESULT: skill=ship status=complete head= run= ``` The run directory holds `ship-receipt.md` (the block, copied) and @@ -255,7 +255,7 @@ When the parent merges the base in or fixes a test after the receipt was written, it runs `gstack-ship-receipt carry --pr --base --dry-run` instead of another /ship; on `CARRY: would carry` it runs `gstack-pregate` at HEAD and reruns carry with `--pregate .gstack/tmp/pregate.json`. `CARRY: carried from to ()` plus the -`GSTACK_RESULT: skill=ship status=complete run=` line means the receipt +`GSTACK_RESULT: skill=ship status=complete head= run=` line means the receipt now names HEAD with a `carry` object holding the `git range-diff` evidence, and `ship-run.json` gains `carried_from` and `carry_reason`; the `review_loop` counters are kept, because the reviewed product diff is unchanged or its only @@ -893,15 +893,19 @@ decision or action, every evidence pointer resolves and shows what the row claims, and a fork or abandoned approach that shaped the result but is not logged gets a row. The audit corrects the log, never the story: no row is edited or removed, an invented or wrong row stays and a new row supersedes it -with what actually happened. Then a reviewer on a different model family from -the one that did the work (a Capy subagent on another family, or the outside -voice) reads the trail and the transcript and flags what the owner should look -at: decisions with weak or missing evidence, verification claimed without -proof, choices that look risky in hindsight, gaps a skim would miss. The run's -final report ends with an `Attention` section that opens with `reviewed by -` on its own line and lists each flag against its rows; `No flags` is a -valid body, a missing model line is not. Self-review does not stand in for -this. `decisions.jsonl` stays the typed record `gstack-artifact` validates; the +with what actually happened. On a risk-class PR (the receipt's `review_depth` +has class `gate` or a `risk` class: security, money, auth, secrets, user +data), a reviewer on a different model family from the one that did the work +(a Capy subagent on another family, or the outside voice) then reads the trail +and the transcript and flags what the owner should look at: decisions with +weak or missing evidence, verification claimed without proof, choices that +look risky in hindsight, gaps a skim would miss. Self-review does not stand in +for that read. Every other PR skips it: the run's own audit is the review. The +run's final report ends with an `Attention` section that opens with `reviewed +by ` (risk class) or `self-audited` (every other PR) on its own line +and lists each flag against its rows; `No flags` is a valid body, a missing +opening line is not. `gstack-ship-queue ready` prints the stub with the right +opening line for the receipt it bound. `decisions.jsonl` stays the typed record `gstack-artifact` validates; the trail is the plain one a person reads, and `column -s$'\t' -t decisions.tsv` renders it in a terminal. diff --git a/lib/headless-artifacts.ts b/lib/headless-artifacts.ts index 1749944fc..3b1e88531 100644 --- a/lib/headless-artifacts.ts +++ b/lib/headless-artifacts.ts @@ -110,7 +110,7 @@ export interface ShipReceipt { // Related open PRs by other authors (Lane D): `supersedes` / `credit` only on the owner's confirmation. issues?: { fixes: number[]; refs: number[]; flakes: Array<{ issue: number; run: string }>; related?: Array<{ pr: number; repo?: string; author: string; closes: number[]; supersedes?: boolean; credit?: boolean }> }; /** Harness lane E: the review loop's counters (gstack-review-scope summary) and the carry record when the receipt followed a head whose product diff was unchanged (gstack-ship-receipt carry). */ - review_loop?: { passes: number; blocking_found: number; deferred: number; noted?: number; fixed?: number; scope?: string; fix_checks?: number; rounds?: number; dispatches?: Record; est_input_tokens?: number; unrecorded?: number; ledgers_read?: number }; + review_loop?: { passes: number; blocking_found: number; deferred: number; noted?: number; fixed?: number; scope?: string; fix_checks?: number; rounds?: number; dispatches?: Record; est_input_tokens?: number; unrecorded?: number; ledgers_read?: number; unbound?: number }; carry?: { from: string; reason: string; product_patch_id: string; changed_paths: string[]; merge_commits: number; range_diff: string; covers: string[]; reruns: string[]; trees?: string[]; ancestry?: 'linear' | 'rewritten' }; /** The reviewed head's tree and its product patch-id against the base (gstack-ship-receipt write; carry rebinds both): what a carry across rewritten history compares. */ head_tree?: string; product_patch_id?: string; @@ -125,7 +125,11 @@ export interface ShipReceipt { base_red?: { cases: string[]; files: string[]; base: string; lane: string; expires_at: string }; /** The base policy's review_exclude_paths at the reviewed head (lib/review-exclude.ts); absent when the base declares none. Carry and the queue compare `digest`. */ review_exclude?: { source: string; patterns: string[]; applied: string[]; excluded: number; excluded_bytes: number; kept: number; gate_tagged: string[]; patterns_sha256: string; list_sha256: string; digest: string }; + /** Routine stops the run recorded and went on past instead of waiting for a Captain (plan K1; GSTA-258 L5 writes them): one record per stop. */ + no_stop?: Array<{ class: NoStopClass; reason: string; evidence: string }>; } +export const NO_STOP_CLASSES = ['scoped-fix-check', 'test-timeout-fix', 'pregate-flake-rerun', 'stamp-only-carry', 'autoplan-regate'] as const; +export type NoStopClass = typeof NO_STOP_CLASSES[number]; export const RESULT_STATUSES: readonly ResultStatus[] = ['complete', 'reviewed', 'gate_pending', 'amend_pending', 'incomplete', 'interrupted', 'refused']; const TERMINAL_RUN: readonly string[] = ['complete', 'reviewed', 'gate_pending', 'amend_pending', 'refused', 'incomplete']; @@ -142,11 +146,12 @@ export function exitTableHelp(): string { export function formatResult(skill: string, status: ResultStatus, run: string): string { return `GSTACK_RESULT: skill=${skill} status=${status} run=${run}`; } -export function parseResult(text: string): { skill: string; status: ResultStatus; run: string } | undefined { +/** A ship line may name the full head it bound (`head=`, GST-1179) between status and run. */ +export function parseResult(text: string): { skill: string; status: ResultStatus; run: string; head?: string } | undefined { const lines = text.split('\n').filter(l => l.startsWith('GSTACK_RESULT: ')); - const m = /^GSTACK_RESULT: skill=(\S+) status=(\S+) run=(.+)$/.exec(lines[lines.length - 1] ?? ''); + const m = /^GSTACK_RESULT: skill=(\S+) status=(\S+)(?: head=([0-9a-f]{40,64}))? run=(.+)$/.exec(lines[lines.length - 1] ?? ''); if (!m || !RESULT_STATUSES.includes(m[2] as ResultStatus)) return; - return { skill: m[1]!, status: m[2] as ResultStatus, run: m[3]! }; + return { skill: m[1]!, status: m[2] as ResultStatus, run: m[4]!, ...(m[3] ? { head: m[3] } : {}) }; } // --------------------------------------------------------------------------- @@ -246,7 +251,7 @@ export const ARTIFACT_SCHEMAS = { thread: str, tree: { enum: ['same-modulo-stamps', 'changed', 'not-compared'] }, gate_reuse: str, policy: str, queue_mode: str, preregistration: str, history: str, pregate: { ...str, description: 'pass|warn|requires-remote cleared … @ from pregate.json, current for this head' }, requires_remote: strList, approval: { type: 'object', description: 'approval classes: { source, status: checked|unavailable, classes: [{ class, scope, effects, citation: verified|unverified, approved_by, date, via, binding, evidence }], report_only }' }, - review_loop: { type: 'object', required: ['passes', 'blocking_found', 'deferred'], properties: { passes: { type: 'integer', minimum: 0 }, blocking_found: { type: 'integer', minimum: 0 }, deferred: { type: 'integer', minimum: 0 }, noted: { type: 'integer', minimum: 0 }, fixed: { type: 'integer', minimum: 0 }, scope: str, fix_checks: { type: 'integer', minimum: 0 }, rounds: { type: 'integer', minimum: 0 }, dispatches: { type: 'object', description: 'reviewer dispatches by role' }, est_input_tokens: { type: 'integer', minimum: 0 }, unrecorded: { type: 'integer', minimum: 0 }, ledgers_read: { type: 'integer', minimum: 0 } } }, + review_loop: { type: 'object', required: ['passes', 'blocking_found', 'deferred'], properties: { passes: { type: 'integer', minimum: 0 }, blocking_found: { type: 'integer', minimum: 0 }, deferred: { type: 'integer', minimum: 0 }, noted: { type: 'integer', minimum: 0 }, fixed: { type: 'integer', minimum: 0 }, scope: str, fix_checks: { type: 'integer', minimum: 0 }, rounds: { type: 'integer', minimum: 0 }, dispatches: { type: 'object', description: 'reviewer dispatches by role' }, est_input_tokens: { type: 'integer', minimum: 0 }, unrecorded: { type: 'integer', minimum: 0 }, ledgers_read: { type: 'integer', minimum: 0 }, unbound: { type: 'integer', minimum: 0, description: 'rows, ledger ids or fix trailers gstack-review-scope summary could not bind (GST-1168)' } } }, carry: { type: 'object', required: ['from', 'reason', 'product_patch_id'], properties: { from: str, reason: str, product_patch_id: str, changed_paths: strList, merge_commits: { type: 'integer', minimum: 0 }, range_diff: str, covers: strList, reruns: strList, trees: strList, ancestry: { enum: ['linear', 'rewritten'] } } }, head_tree: str, product_patch_id: str, gstack_version: str, base_red: { type: 'object', required: ['cases', 'files', 'base', 'lane', 'expires_at'], properties: { cases: strList, files: strList, base: str, lane: str, expires_at: str } }, @@ -254,6 +259,7 @@ export const ARTIFACT_SCHEMAS = { review_targeted: { type: 'object', required: ['classes', 'rules', 'brief_sha', 'product_patch_id', 'status', 'findings', 'raised_by'], properties: { classes: { type: 'array', items: { enum: ['money', 'gate', 'auth', 'secret'] } }, rules: str, brief_sha: str, product_patch_id: str, status: { enum: ['done', 'failed', 'not-run'] }, findings: { type: 'integer', minimum: 0 }, raised_by: str } }, review_exclude: { type: 'object', required: ['source', 'patterns', 'applied', 'excluded', 'excluded_bytes', 'kept', 'gate_tagged', 'patterns_sha256', 'list_sha256', 'digest'], properties: { source: str, patterns: strList, applied: strList, excluded: { type: 'integer', minimum: 0 }, excluded_bytes: { type: 'integer', minimum: 0 }, kept: { type: 'integer', minimum: 0 }, gate_tagged: strList, patterns_sha256: str, list_sha256: str, digest: str } }, review_loop_exception: { type: 'object', required: ['accepted_by', 'date', 'via', 'reason', 'fix_check'], properties: { accepted_by: str, date: str, via: str, reason: str, fix_check: { type: 'object', required: ['head', 'findings'], properties: { head: str, findings: { type: 'integer', minimum: 0 }, files: strList } } } }, + no_stop: { type: 'array', items: { type: 'object', required: ['class', 'reason', 'evidence'], properties: { class: { enum: [...NO_STOP_CLASSES] }, reason: str, evidence: str } } }, issues: { type: 'object', properties: { fixes: { type: 'array', items: { type: 'integer' } }, refs: { type: 'array', items: { type: 'integer' } }, flakes: { type: 'array', items: { type: 'object', required: ['issue', 'run'], properties: { issue: { type: 'integer' }, run: str } } }, related: { type: 'array', items: { type: 'object', required: ['pr', 'author', 'closes'], properties: { pr: { type: 'integer' }, repo: str, author: str, closes: { type: 'array', items: { type: 'integer' } }, supersedes: { type: 'boolean' }, credit: { type: 'boolean' } } } } } }, }, }, diff --git a/lib/review-ledger.ts b/lib/review-ledger.ts index 7ace70d5b..e1fa339be 100644 --- a/lib/review-ledger.ts +++ b/lib/review-ledger.ts @@ -100,6 +100,14 @@ const DENIED_OR_FIXED = new RegExp(`(?:(? ANY_LABEL.test(t) && !ANY_LABEL.test(t.replace(DENIED_OR_FIXED, ' ')) && !t.startsWith('{') && !t.startsWith('**') && !LIST_ITEM.test(t) && !HEADING.test(t) && !LEAD_LABEL.test(t) && !t.match(SEVERITY_FIELD) && !t.match(STATED_BLOCKS) && [...t.matchAll(ALL_LABELS)].every(m => !!rank(m[1])) && t.split(SENTENCE).filter(s => ANY_LABEL.test(s)).every(s => !PATH_LOCUS.test(s) && !EXCEPTION.test(s)); +/** + * The native reader's own verdict line (GSTA-258 L4: #3901 "There are three low-severity findings, all FIXABLE, and none blocks."): a prose + * line, no list item, heading, lead label, file:line, Severity or Blocks field, whose only labels are FIXABLE or INVESTIGATE and which says + * nothing blocks. It collects at FIXABLE as written, never raised to CRITICAL; the findings it summarizes collect on their own lines. + */ +const NONE_BLOCKS = /(? ANY_LABEL.test(t) && NONE_BLOCKS.test(t) && !LOCUS.test(t) && !PATH_LOCUS.test(t) && !LIST_ITEM.test(t) && !HEADING.test(t) && !LEAD_LABEL.test(t) + && !t.match(SEVERITY_FIELD) && !t.match(STATED_BLOCKS) && [...t.matchAll(ALL_LABELS)].every(m => rank(m[1]) === 0); export function stated(raw: string): Stated | undefined { if (raw.startsWith('{')) return undefined; const severity = [...raw.matchAll(STATED_SEVERITY)].at(-1)?.[1]; @@ -143,6 +151,7 @@ export function parseFindings(reader: string, output: string, fixableRaises = tr const critical = (t: string) => BLOCKS_YES.test(t) || (fixableRaises && role === 'adversarial-native' && /(? { const l = LEAD_LABEL.exec(t)?.[1]; return l === undefined || !fixableRaises ? l : named(l); }; const labelOf = (t: string) => { + if (role === 'adversarial-native' && quietSummary(t)) return 'FIXABLE'; const field = fieldOf(t); if (field === undefined) return strongest(t); return critical(t) ? 'CRITICAL' : strongestOf([leadOf(t), field])!; @@ -342,7 +351,7 @@ const realPath = (f: string) => { try { return fs.realpathSync(f); } catch { ret * included) so the summary can find its row. * A ledger, merged file or judged set it cannot read, or a judged set changed since judge, throws: the count is never a guess. */ -export interface LedgerBlocker { id: string; locus: string | null } +export interface LedgerBlocker { id: string; locus: string | null; /** The run-unique `/` a row's ledger_id and a fix trailer name (GST-1168); the caller sets it. */ key?: string } function lineLocus(raw: string): string | null { try { const o = JSON.parse(raw); diff --git a/lib/review-scope.ts b/lib/review-scope.ts index f40d49884..7822fd958 100644 --- a/lib/review-scope.ts +++ b/lib/review-scope.ts @@ -13,6 +13,7 @@ * * Pure functions here; bin/gstack-review-scope runs git and gh. */ +import * as path from 'node:path'; import { SPECIALISTS } from './review-depth'; import { rank, readLedger, type LedgerBlocker } from './review-ledger'; import { parseSpecialistOutput } from './state-matrix'; @@ -313,6 +314,8 @@ export interface ReviewLoopSummary { unrecorded?: number; /** Dispatch ledgers whose collected and judged findings were counted (gstack-review-dispatch). */ ledgers_read?: number; + /** What could not bind (GST-1168), each named with its reason: a row with no start token or readable time, a row finding's ledger_id no ledger holds, a Review-Finding trailer naming no finding. Never counted as zero silently. */ + unbound?: string[]; } /** Review-log rows of one /ship invocation (skill review, via ship, at or after `since`) → the loop summary. */ @@ -376,12 +379,22 @@ function rowLocus(f: Record): string | 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 { +export interface LedgerPass { started: number; fixCheck: boolean; ids?: string[]; /** The ledger's pass directory (p1, p2, fc): a pN with no review row of pass N is named unbound. */ name?: string } +export function ledgerPass(ledger: string, key = (id: string) => id): 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 }; + const ids = events.flatMap(e => (e.event === 'output' && Array.isArray(e.ids) ? (e.ids as string[]) : e.event === 'judgment' && e.kind === 'added' && typeof e.id === 'string' ? [e.id] : [])).map(key); + return { started: Number.isFinite(started) ? started : NaN, fixCheck, ids, name: path.basename(path.dirname(path.resolve(ledger))) }; +} + +/** A fix commit's `Review-Finding: reader=` trailer (receipt attribution): the finding that commit fixed. */ +export interface FixTrailer { finding: string; reader: string; commit: string } +export function parseFixTrailers(commit: string, values: readonly string[]): FixTrailer[] { + return values.flatMap(v => { + const m = /^\s*(\S+)\s+reader=([a-z][a-z0-9-]*)\s*$/.exec(v); + return m ? [{ finding: m[1]!, reader: m[2]!, commit }] : []; + }); } /** @@ -400,12 +413,17 @@ export function ledgerPass(ledger: string): LedgerPass { * 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[])[] = [], ledgerPasses: readonly LedgerPass[] = []): ReviewLoopSummary { +export function summarizeReviewLoop(rows: readonly Record[], since: string, ledgers: readonly (readonly LedgerBlocker[])[] = [], ledgerPasses: readonly LedgerPass[] = [], trailers: readonly FixTrailer[] = []): ReviewLoopSummary { const sinceMs = Date.parse(since); + const loopSkill = (r: Record) => r.skill === 'review' || r.skill === LATE_DEFER_SKILL || r.skill === FIX_CHECK_SKILL; const inRun = (r: Record) => r && r.via === 'ship' && Number.isFinite(rowTime(r)) && (!Number.isFinite(sinceMs) || rowTime(r) >= sinceMs); - const loopRows = rows.filter(r => inRun(r) && (r.skill === 'review' || r.skill === LATE_DEFER_SKILL || r.skill === FIX_CHECK_SKILL)); + const unbound: string[] = rows.filter(r => r && r.via === 'ship' && loopSkill(r) && !Number.isFinite(rowTime(r))) + .map(r => `row ${String(r.skill)} pass=${String(r.pass ?? '?')}: no start token and no readable timestamp, so it cannot be placed in this run`); + // A repeated logger command writes the same row twice (#3911: one fix-check counted as two): rows that record their head dedupe by kind, pass, head and finding ids. + const rowKey = (r: Record) => JSON.stringify([r.skill, r.pass ?? null, r.commit_full ?? r.commit ?? '', (Array.isArray(r.findings) ? r.findings : []).map((f: any) => (f && typeof f === 'object' ? (typeof f.ledger_id === 'string' ? f.ledger_id : findingIdentity(f)) : '')).sort()]); + const loopRows = [...new Map(rows.filter(r => inRun(r) && loopSkill(r)).map((r, i) => [r.commit_full || r.commit ? rowKey(r) : `#${i}`, r] as const)).values()]; const passes = loopRows.filter(r => r.skill === 'review'); - type Seen = { locus: string | null; raiser: string | null; classified: boolean; blocks: boolean; candidate: boolean; disposed: boolean }; + type Seen = { locus: string | null; raiser: string | null; classified: boolean; blocks: boolean; candidate: boolean; disposed: boolean; ledgerId?: string }; const seen = new Map(); const deferred = new Set(); const noted = new Set(); @@ -415,7 +433,10 @@ export function summarizeReviewLoop(rows: readonly Record[], since: if (REVIEW_SCOPE_POLICIES.includes(row.review_scope)) scope = row.review_scope; for (const f of Array.isArray(row.findings) ? row.findings : []) { if (!f || typeof f !== 'object') continue; - const id = findingIdentity(f); + const ledgerId = typeof f.ledger_id === 'string' && f.ledger_id ? f.ledger_id : undefined; + // A late defer re-logs a finding an earlier row holds; without a ledger_id it is that finding when the locus and reviewer agree (GBEV-3). + const same = !ledgerId && row.skill === LATE_DEFER_SKILL ? [...seen].find(([, s]) => s.locus !== null && s.locus === rowLocus(f) && s.raiser === raiserOf(f.raised_by))?.[0] : undefined; + const id = ledgerId ?? same ?? findingIdentity(f); const prior = seen.get(id); const classified = typeof f.blocking === 'boolean'; seen.set(id, { @@ -425,15 +446,21 @@ export function summarizeReviewLoop(rows: readonly Record[], since: blocks: classified ? f.blocking : prior?.classified ? prior.blocks : !!prior?.blocks || rowBlocks(f), candidate: !!prior?.candidate || f.severity === undefined || rank(f.severity) === 4 || rank(f.raw_severity) === 4 || f.blocking === true, disposed: !!prior?.disposed || DISPOSITIONS.includes(f.action), + ...(ledgerId ?? prior?.ledgerId ? { ledgerId: ledgerId ?? prior?.ledgerId } : {}), }); if (f.action === 'deferred') deferred.add(id); if (f.action === 'noted') noted.add(id); if (f.action === 'fixed' || f.action === 'auto-fixed') fixed.add(id); } } + for (const p of ledgerPasses) { + const n = /^p(\d+)$/.exec(p.name ?? '')?.[1]; + if (n && !passes.some(r => String(r.pass) === n)) unbound.push(`pass ${n} (ledger ${p.name}): no review-log row, so its fixes and dispositions cannot be counted`); + } + const known = ledgerPasses.some(p => p.ids) ? new Set(ledgerPasses.flatMap(p => p.ids ?? [])) : null; + if (known) for (const [, s] of seen) if (s.ledgerId && !known.has(s.ledgerId)) unbound.push(`ledger_id ${s.ledgerId}: no dispatch ledger of this run collected it`); const paired = new Set(); - let ledgerOnly = 0; - let ledgerOnlyFixed = 0; + const ledgerOnlyKeys: Array<{ key: string; id: string; fixed: boolean }> = []; const pairedIds = new Set(); const followedIds = new Set(); const lastPaired = new Map(); @@ -443,17 +470,29 @@ export function summarizeReviewLoop(rows: readonly Record[], since: 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); if (!((ledgerPasses[lastPaired.get(id) ?? -1]?.started ?? -Infinity) > (ledgerPasses[n]?.started ?? -Infinity))) lastPaired.set(id, n); } else { ledgerOnly++; if (followed(n)) ledgerOnlyFixed++; } + const key = b.key ?? b.id; + const open = ([k]: [string, Seen]) => !paired.has(`${n}\0${k}`); + const id = [...seen].find(e => open(e) && (e[1].ledgerId === key || e[1].ledgerId === b.id))?.[0] + ?? (b.locus ? [...seen].find(e => open(e) && !e[1].ledgerId && e[1].candidate && e[1].locus === b.locus && (!e[1].raiser || !by || e[1].raiser === by))?.[0] : undefined); + 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 ledgerOnlyKeys.push({ key, id: b.id, fixed: followed(n) }); } } for (const [id, n] of lastPaired) if (followed(n)) followedIds.add(id); + for (const t of trailers) { + const hit = [...seen].find(([k, s]) => k === t.finding || s.ledgerId === t.finding); + if (hit) { hit[1].disposed = true; fixed.add(hit[0]); continue; } + const only = ledgerOnlyKeys.find(l => l.key === t.finding || l.id === t.finding); + if (only) { only.fixed = true; continue; } + unbound.push(`trailer ${t.commit.slice(0, 12)} Review-Finding: ${t.finding} reader=${t.reader}: names no finding this run's rows or ledgers hold`); + } const blockers = [...seen].filter(([k, s]) => s.blocks || (pairedIds.has(k) && !s.classified)); 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 }; + const ledgerOnlyFixed = ledgerOnlyKeys.filter(l => l.fixed).length; + const unrecorded = ledgerOnlyKeys.length - ledgerOnlyFixed + blockers.filter(([, s]) => !s.disposed).length; + const out: ReviewLoopSummary = { passes: passes.length, blocking_found: blockers.length + ledgerOnlyKeys.length, deferred: deferred.size, noted: noted.size, fixed: fixed.size + ledgerOnlyFixed, scope }; if (unrecorded) out.unrecorded = unrecorded; if (ledgers.length) out.ledgers_read = ledgers.length; + if (unbound.length) out.unbound = unbound; const fixChecks = Math.max(rowFixChecks, fixCheckStarts.length); if (fixChecks) out.fix_checks = fixChecks; out.rounds = passes.length + fixChecks; @@ -474,6 +513,6 @@ export function dispatchTotal(d: Record | undefined): number | u export function reviewLoopLine(s: ReviewLoopSummary): string { const total = dispatchTotal(s.dispatches); return `REVIEW_LOOP: passes=${s.passes} blocking_found=${s.blocking_found} deferred=${s.deferred} noted=${s.noted} fixed=${s.fixed} scope=${s.scope}` - + (s.unrecorded ? ` unrecorded=${s.unrecorded}` : '') + (s.ledgers_read ? ` ledgers_read=${s.ledgers_read}` : '') + + (s.unrecorded ? ` unrecorded=${s.unrecorded}` : '') + (s.unbound?.length ? ` unbound=${s.unbound.length}` : '') + (s.ledgers_read ? ` ledgers_read=${s.ledgers_read}` : '') + (s.fix_checks ? ` fix_checks=${s.fix_checks}` : '') + (total !== undefined ? ` dispatches=${total}` : '') + (s.est_input_tokens !== undefined ? ` est_input_tokens=${s.est_input_tokens}` : ''); } diff --git a/lib/ship-preflight.ts b/lib/ship-preflight.ts index 9f9b28156..52b03bc5a 100644 --- a/lib/ship-preflight.ts +++ b/lib/ship-preflight.ts @@ -58,6 +58,10 @@ function integrate(cwd: string, base: string, r: Report): boolean { r.next(`carry route (Step 1.4): fix what the base merge broke (CHANGELOG, size caps), gstack-pregate --allow-repo-commands, then gstack-ship-receipt carry --pr ${n} --base ${base} --pregate .gstack/tmp/pregate.json`); return false; } + if (line === 'CARRY: refused (RECEIPT_CARRY_PRODUCT_CHANGED)' && /^RECHECK: eligible /m.test(carry.stderr ?? '')) { + r.next(`carry route (Step 1.4, J1): the product changed since the receipt within the scoped re-check cap: gstack-ship-receipt carry --pr ${n} --base ${base} writes the re-check packet; one reader reads it; gstack-pregate --allow-repo-commands, then carry again with --recheck --pregate .gstack/tmp/pregate.json; a blocking finding runs Steps 2-16`); + return false; + } if (line === 'CARRY: refused (RECEIPT_CARRY_SCOPED_REVIEW)') { r.next(`carry route (Step 1.4): the base edited context next to reviewed hunks: fix what the base merge broke (CHANGELOG, size caps), commit and push; then gstack-ship-receipt carry --pr ${n} --base ${base} writes the packet for that head; one fix-check reviewer reads it; gstack-pregate --allow-repo-commands, then carry again with --scoped-review --pregate .gstack/tmp/pregate.json; a blocking finding runs Steps 2-16`); return false; diff --git a/lib/ship-queue.ts b/lib/ship-queue.ts index 0a95e7d66..2f2220d5e 100644 --- a/lib/ship-queue.ts +++ b/lib/ship-queue.ts @@ -31,7 +31,7 @@ import type { PregateFile } from './headless-artifacts'; import { CHECKS, pregateMirrorPath, readPregateFile, verifyPregate, PREGATE_OUT_REL } from './pregate/index'; import { ensureTmpExcluded, restamp, versionAtRef } from './restamp'; import type { ResultCodeName } from './result-codes'; -import { extractReceipt } from './ship-receipt'; +import { extractReceipt, readyTailLines } from './ship-receipt'; import { bindShippedReceipt } from './ship-receipt-depth'; import { isUnavailable, loadPolicyFromBase, type LoadedPolicy } from './ship-policy'; import { @@ -272,6 +272,7 @@ export function ready(ctx: QueueContext, o: { pr: string; pregate?: string; gate receipt.line, ...(receipt.depthLine ? [receipt.depthLine] : []), ...(receipt.targetedLine ? [receipt.targetedLine] : []), + ...readyTailLines(receipt.ok ? receipt.receipt : undefined), ]; const gate = policy.policy.full_e2e ? gateEvidence(ctx, o.pr, head, attempt, o.gateExit, o.gateLog) : null; if (gate) lines.push(gate.line); diff --git a/lib/ship-receipt.ts b/lib/ship-receipt.ts index b1baa4b61..bc5e37cce 100644 --- a/lib/ship-receipt.ts +++ b/lib/ship-receipt.ts @@ -19,7 +19,7 @@ const KEY_ORDER: Array = [ 'schema_version', 'pr', 'head', 'version', 'base', 'gated_tree', 'predecessor', 'tree', 'gate_reuse', 'gate', 'ci', 'ci_carried', 'spend_usd', 'session_kind', 'artifacts_consumed', 'run', 'thread', 'policy', 'queue_mode', 'preregistration', 'history', 'head_tree', 'product_patch_id', 'gstack_version', 'review_depth', 'pregate', 'requires_remote', 'issues', 'approval', 'docs', 'review_loop', - 'review_loop_exception', 'review_targeted', 'review_exclude', 'carry', 'base_red', + 'review_loop_exception', 'review_targeted', 'review_exclude', 'carry', 'base_red', 'no_stop', ]; /** Keys in the fixed order so identifiers lead; undefined values are dropped. */ @@ -92,10 +92,32 @@ export function bindReceipt(body: string, pr: string | undefined, accepted: read return { ok: true, receipt: r, head, line }; } -/** The block becomes the first section of the body: an existing block is replaced in place at the top, prose follows. */ +/** + * Every /ship run's receipt (GSTA-258 L4, receipt attribution): when a write or carry replaces a different receipt, the old one's + * identifiers and counters move to an append-only `gstack-receipt-runs` block (one JSON line per run, oldest first, the + * newest RECEIPT_RUNS_KEPT kept) right after the current block, so a later replay sees each run's review loop, not only the last. + */ +export const RUNS_FENCE = 'gstack-receipt-runs'; +export const RECEIPT_RUNS_KEPT = 30; +const RUNS_RE = /```gstack-receipt-runs[ \t]*\r?\n([\s\S]*?)\r?\n?```\n?/; +export type ReceiptRun = Pick & { depth?: string; carried_from?: string }; +export function receiptRuns(body: string): ReceiptRun[] { + const m = RUNS_RE.exec(body); + return m ? m[1]!.split('\n').flatMap(l => { try { const o = JSON.parse(l); return o && typeof o.head === 'string' ? [o as ReceiptRun] : []; } catch { return []; } }) : []; +} +export function receiptRun(r: ShipReceipt): ReceiptRun { + const { head, version, gstack_version, run, review_loop, review_loop_exception } = r; + return JSON.parse(JSON.stringify({ head, version, gstack_version, run, review_loop, review_loop_exception, depth: r.review_depth?.depth, carried_from: r.carry?.from })); +} + +/** The block becomes the first section of the body: an existing block is replaced in place at the top, prose follows; a replaced receipt joins the runs block. */ export function placeReceiptFirst(body: string, receipt: ShipReceipt): string { - const block = renderReceiptBlock(receipt); - const stripped = body.replace(BLOCK_RE, '').replace(/^\s+/, ''); + const prior = extractReceipt(body).receipt; + const runs = receiptRuns(body); + if (prior && typeof prior.head === 'string' && JSON.stringify(prior) !== JSON.stringify(receipt)) runs.push(receiptRun(prior)); + const kept = runs.slice(-RECEIPT_RUNS_KEPT); + const block = renderReceiptBlock(receipt) + (kept.length ? '\n```' + RUNS_FENCE + '\n' + kept.map(r => JSON.stringify(r)).join('\n') + '\n```\n' : ''); + const stripped = body.replace(BLOCK_RE, '').replace(RUNS_RE, '').replace(/^\s+/, ''); return block + (stripped ? '\n' + stripped : ''); } @@ -136,6 +158,25 @@ export function receiptSummaryLine(r: ShipReceipt): string { return `SHIP_RECEIPT: ${oneLine(parts.join(' '))}`; } +/** A risk-class PR (R51): the receipt's review depth has class gate or a risk class. Only these get the cross-family trail read. */ +export const riskClassReceipt = (r: ShipReceipt | undefined) => r?.review_depth?.class === 'gate' || !!r?.review_depth?.risk?.length; + +/** + * The rest of the READY block (GSTA-258 E1: the ready output is the whole READY): the receipt's review loop, the trial-merge + * line the lane fills, and the Attention stub, whose opening line follows the receipt's risk class (docs/unattended.md). + */ +export function readyTailLines(r: ShipReceipt | undefined): string[] { + const loop = parseReviewLoop(r?.review_loop); + const opt = (k: keyof ReviewLoopReceipt) => (loop?.[k] ? ` ${k}=${loop[k]}` : ''); + const risk = riskClassReceipt(r); + return [ + loop ? `REVIEW_LOOP: passes=${loop.passes} blocking_found=${loop.blocking_found} deferred=${loop.deferred} noted=${loop.noted ?? 0} fixed=${loop.fixed ?? 0}${opt('unrecorded')}${opt('unbound')}${opt('fix_checks')}` : 'REVIEW_LOOP: none in the receipt', + 'TRIAL_MERGES: ', + 'Attention:', + risk ? `reviewed by (risk class: ${oneLine([r?.review_depth?.class === 'gate' ? 'gate' : '', ...(r?.review_depth?.risk ?? [])].filter(Boolean).join(', '))}; rule 27 cross-family trail read)` : 'self-audited (not a risk class: no cross-family trail read)', + '', + ]; +} // --------------------------------------------------------------------------- // carry: the receipt follows a head whose product diff is unchanged @@ -181,6 +222,8 @@ export interface CarryPlanInput { scopedReview?: string; /** Stamp files the branch changed as product whose only change since the reviewed head is a stamp on the branch's own rows, the base untouched, and without which the two product patches match (GST-980). */ stampedRows?: string[]; + /** A clean scoped re-check of the range-diff since the reviewed head (lib/ship-recheck.ts, J1): the carry reason it adds. */ + recheck?: string; } export interface CarryRecord { from: string; reason: string; product_patch_id: string; changed_paths: string[]; merge_commits: number; range_diff: string; covers: string[]; reruns: string[]; trees?: string[]; ancestry?: 'linear' | 'rewritten' } export type CarryPlan = { ok: true; carry: CarryRecord } | { ok: false; code: 'RECEIPT_CARRY_PRODUCT_CHANGED' | 'RECEIPT_CARRY_REFUSED' | 'RECEIPT_CARRY_TREE_MISMATCH'; message: string }; @@ -249,12 +292,13 @@ export function classifyCarryPath(rel: string, extra: { metadata?: readonly stri * branch's own rows in a stamp file the branch also changed as product (`stampedRows`, GST-980). */ export function planCarry(input: CarryPlanInput): CarryPlan { - if (input.productPatchId.from !== input.productPatchId.head && (input.changedProduct.length || (!input.scopedReview && !input.stampedRows?.length))) { + if (input.productPatchId.from !== input.productPatchId.head && !input.recheck && (input.changedProduct.length || (!input.scopedReview && !input.stampedRows?.length))) { const named = input.changedProduct.slice(0, CARRY_NAMED_PATHS); return { ok: false, code: 'RECEIPT_CARRY_PRODUCT_CHANGED', message: `product diff changed between ${input.from.slice(0, 12)} and ${input.head.slice(0, 12)}: ${named.join(', ') || 'patch-id differs'}${input.changedProduct.length > named.length ? ` (+${input.changedProduct.length - named.length} more)` : ''}` }; } const reasons: string[] = input.retargetedFrom ? [`stack parent ${input.retargetedFrom} landed`] : []; if (input.mergeCommits > 0) reasons.push(input.mergeCommits === 1 ? 'base merge' : `${input.mergeCommits} base merges`); + if (input.recheck && input.productPatchId.from !== input.productPatchId.head) reasons.push(input.recheck); if (input.scopedReview && input.productPatchId.from !== input.productPatchId.head) reasons.push(input.scopedReview); if (input.stampedRows?.length && input.productPatchId.from !== input.productPatchId.head) reasons.push(`a stamp on the branch's own rows (${input.stampedRows.join(', ')})`); if (input.testPatchId.from !== input.testPatchId.head) reasons.push('test-only change'); @@ -337,6 +381,8 @@ export function parseReviewLoop(value: unknown): ReviewLoopReceipt | undefined { if (n('est_input_tokens') !== undefined) out.est_input_tokens = n('est_input_tokens'); if (n('unrecorded')) out.unrecorded = n('unrecorded'); if (n('ledgers_read')) out.ledgers_read = n('ledgers_read'); + const unbound = Array.isArray(v.unbound) ? v.unbound.length : n('unbound'); + if (unbound) out.unbound = unbound; if (v.dispatches && typeof v.dispatches === 'object' && !Array.isArray(v.dispatches)) { const d = Object.fromEntries(Object.entries(v.dispatches as Record) .filter(([k, c]) => /^[a-z][a-z0-9-]{0,39}$/.test(k) && typeof c === 'number' && Number.isInteger(c) && c >= 0).slice(0, 12)) as Record; diff --git a/lib/ship-recheck.ts b/lib/ship-recheck.ts new file mode 100644 index 000000000..aa62ff37d --- /dev/null +++ b/lib/ship-recheck.ts @@ -0,0 +1,63 @@ +/** + * J1 (GSTA-258 plan H, relabeled J1): a scoped re-check after a refused receipt carry. When `gstack-ship-receipt carry` refuses + * because the product patch changed since the reviewed head, and the change is bounded, one reader reads `git range-diff` of + * the reviewed range against the current one instead of a full second /ship (gbrain-evals #246/#247: about 100k credits per + * full rerun; #300's scoped reader: about 300). A clean verdict bound to that exact range-diff rebinds the receipt; the carry's + * later checks still refuse a deeper review depth or a changed risk class, which need the full /ship. + * + * Qualifies: RECEIPT_CARRY_PRODUCT_CHANGED on linear history (the receipt's head is an ancestor of HEAD) against the same base. + * Does not qualify: RECEIPT_CARRY_TREE_MISMATCH (rewritten history: the reviewed commits are gone, so there is no range to + * diff), a retargeted stack (the reviewed range was against another base), an unreadable or empty range-diff, a range-diff + * over RECHECK_RANGE_DIFF_CAP lines, and every RECEIPT_CARRY_REFUSED / RECEIPT_CARRY_PREGATE_STALE (those need their inputs + * repaired, not a review). + */ +import { createHash } from 'node:crypto'; +import * as fs from 'node:fs'; + +export const RECHECK_RANGE_DIFF_CAP = 400; +/** rangeDiff: \`git range-diff\` of the two ranges followed by the diff of the changed product paths between the heads (a range-diff names added commits without their content). */ +export type RecheckInput = { code: string; ancestor: boolean; retarget: boolean; rangeDiff: string | null; unnamedPaths?: boolean }; +export type RecheckPlan = { ok: true; lines: number; sha256: string } | { ok: false; reason: string }; + +const sha = (s: string) => createHash('sha256').update(s).digest('hex'); + +/** Whether a carry refusal may take the scoped re-check, and the range-diff a verdict must bind to. */ +export function planRecheck(i: RecheckInput): RecheckPlan { + if (i.code !== 'RECEIPT_CARRY_PRODUCT_CHANGED') return { ok: false, reason: `${i.code} is not a product change a review can clear` }; + if (!i.ancestor) return { ok: false, reason: 'rewritten history: the reviewed head is not an ancestor of HEAD, so there is no range to diff' }; + if (i.retarget) return { ok: false, reason: 'the receipt was reviewed against another base (a retargeted stack)' }; + if (i.unnamedPaths) return { ok: false, reason: 'a changed product path could not be named, so the packet would miss it' }; + if (i.rangeDiff === null) return { ok: false, reason: 'git range-diff failed or timed out' }; + const text = i.rangeDiff.trim(); + if (!text) return { ok: false, reason: 'git range-diff is empty, so it shows nothing for a reader to check' }; + const lines = text.split('\n').length; + if (lines > RECHECK_RANGE_DIFF_CAP) return { ok: false, reason: `git range-diff is ${lines} lines, over the scoped re-check cap of ${RECHECK_RANGE_DIFF_CAP}` }; + return { ok: true, lines, sha256: sha(text) }; +} + +/** The packet the one reader reads: what to judge, the record to write, then the range-diff verbatim. */ +export function recheckPacket(o: { from: string; head: string; plan: Extract; rangeDiff: string }): string { + return [ + `# Scoped re-check: the product patch changed since the reviewed head\n`, + `reviewed ${o.from}; current ${o.head}. Below is \`git range-diff\` of the reviewed range against the current one, then the diff of the changed product paths between the two heads (${o.plan.lines} lines).`, + 'Review only what changed between the two ranges, against the blocking floor: would fail a test, break production, or lose data or secrets.', + `Reply NO FINDINGS, or one finding per line with path:line, Severity and Blocks. A clean reply is recorded as {"from":"${o.from}","head":"${o.head}","range_diff_sha256":"${o.plan.sha256}","reader":"","verdict":"clean"}.`, + 'A blocking finding means the full /ship on the current head.\n', + '```diff', o.rangeDiff.trim(), '```', '', + ].join('\n'); +} + +export interface RecheckRecord { from: string; head: string; range_diff_sha256: string; reader: string; verdict: 'clean' } + +/** A clean re-check record bound to this carry's from, head and range-diff, or why it does not stand. */ +export function readRecheck(file: string, bind: { from: string; head: string; sha256: string }): { ok: true; record: RecheckRecord } | { ok: false; message: string } { + let v: Record; + try { v = JSON.parse(fs.readFileSync(file, 'utf8')); } catch (e) { return { ok: false, message: `${file} is not readable JSON (${(e as Error).message.split('\n')[0]})` }; } + if (!v || typeof v !== 'object') return { ok: false, message: `${file} is not a JSON object` }; + if (v.verdict !== 'clean') return { ok: false, message: `the re-check is ${JSON.stringify(v.verdict)}, not clean: run /ship on ${bind.head.slice(0, 12)}` }; + if (typeof v.reader !== 'string' || !v.reader.trim()) return { ok: false, message: 'the re-check names no reader' }; + for (const [k, want] of [['from', bind.from], ['head', bind.head], ['range_diff_sha256', bind.sha256]] as const) { + if (v[k] !== want) return { ok: false, message: `the re-check is for ${k} ${String(v[k] ?? 'none').slice(0, 12)}, not ${want.slice(0, 12)}` }; + } + return { ok: true, record: v as unknown as RecheckRecord }; +} diff --git a/ship/DRIVER.md b/ship/DRIVER.md index 235eecec8..7616e5031 100644 --- a/ship/DRIVER.md +++ b/ship/DRIVER.md @@ -118,7 +118,7 @@ repo gates run beside CI. Unattended, push defers the pre-push offer; else its S - 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 @ [--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. +- 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 head= 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 2b451755f..7d8111837 100644 --- a/ship/DRIVER.md.tmpl +++ b/ship/DRIVER.md.tmpl @@ -116,7 +116,7 @@ repo gates run beside CI. Unattended, push defers the pre-push offer; else its S - 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 @ [--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. +- 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 head= 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/SKILL.md b/ship/SKILL.md index 125947cba..47496e238 100644 --- a/ship/SKILL.md +++ b/ship/SKILL.md @@ -956,7 +956,7 @@ The last line printed is the result line a parent greps (docs/unattended.md): ``` It binds the receipt to HEAD, writes the run record and prints `GSTACK_RESULT: skill=ship -status=complete run=`; `QUEUE_RECEIPT_STALE` means HEAD moved after the receipt (rewrite, +status=complete head= run=`; `QUEUE_RECEIPT_STALE` means HEAD moved after the receipt (rewrite, repost, rerun). A run that STOPs earlier ends with `GSTACK_RESULT: skill=ship status=refused run=-`. --- diff --git a/ship/SKILL.md.tmpl b/ship/SKILL.md.tmpl index 1dc0ce129..15d30bbda 100644 --- a/ship/SKILL.md.tmpl +++ b/ship/SKILL.md.tmpl @@ -675,7 +675,7 @@ The last line printed is the result line a parent greps (docs/unattended.md): ``` It binds the receipt to HEAD, writes the run record and prints `GSTACK_RESULT: skill=ship -status=complete run=`; `QUEUE_RECEIPT_STALE` means HEAD moved after the receipt (rewrite, +status=complete head= run=`; `QUEUE_RECEIPT_STALE` means HEAD moved after the receipt (rewrite, repost, rerun). A run that STOPs earlier ends with `GSTACK_RESULT: skill=ship status=refused run=-`. --- diff --git a/ship/sections/review-army.md b/ship/sections/review-army.md index 76abde9ed..938f0b08c 100644 --- a/ship/sections/review-army.md +++ b/ship/sections/review-army.md @@ -687,7 +687,8 @@ but missing dispatched output still blocks continuation, even with a QA exceptio (all sources, queued Steps 10–11 included) as a JSON array to `.gstack/tmp/review-findings-p$PASS.json` with your file-write tool, each with `raised_by` (`core`, `qa`, a specialist, `red-team`, `adversarial-native`, - `outside-voice` or `fix-check`), then: + `outside-voice` or `fix-check`) and, for a dispatched reader's finding, `ledger_id` + (`/`, e.g. `p2/adversarial-native#3`), then: `~/.claude/skills/gstack/bin/gstack-review-scope classify --base --pass $PASS --range-base $RANGE_BASE --findings .gstack/tmp/review-findings-p$PASS.json --json > .gstack/tmp/review-classified-p$PASS.json` (omit `--range-base` on pass 1). Every Step 9.4 scratch file lives under `.gstack/tmp/` (gitignored): `` below is this pass's classified @@ -754,7 +755,8 @@ but missing dispatched output still blocks continuation, even with a QA exceptio row under its own skill (a tokenless `review` row would read UNVERIFIED): `~/.claude/skills/gstack/bin/gstack-review-log '{"skill":"review-late-defer","via":"ship","timestamp":"TIMESTAMP","status":"issues_found","issues_found":0,"critical":0,"informational":0,"pass":PASS,"review_scope":"SCOPE","findings":FINDINGS_JSON}'` with `findings` holding only the deferred records (`action: "deferred"`, `issue`, - `locus`, `kind`, `blocking` as in item 6) and no `--finish` token. + `locus`, `kind`, `blocking`, `ledger_id` as in item 6) and no `--finish` token. Log it in the + program that decides the defer, with that moment's full ISO 8601 TIMESTAMP, never at Step 19. 4. **Finish and log this pass before choosing the next step.** Apply each A) Restore by reverting only the relaxation. After any fix or restore in this pass, run item @@ -765,7 +767,8 @@ but missing dispatched output still blocks continuation, even with a QA exceptio REVIEW_START. Missing dispatched output uses `status:"unavailable"`, `completed:false` and `converged:false`; fixes also require `converged:false`. Then commit named fixed files, if any - (`git add && git commit -m "fix: pre-landing review fixes"`). + (`git add && git commit -m "fix: pre-landing review fixes" --trailer "Review-Finding: reader="`, + one trailer per fixed finding: the receipt counts fixes by it). 5. Output summary: `Pre-Landing Review: N issues — M auto-fixed, K asked (J fixed, L skipped), D deferred, O noted` @@ -804,7 +807,7 @@ but missing dispatched output still blocks continuation, even with a QA exceptio `{"fingerprint":"path:line:category","severity":"CRITICAL|INFORMATIONAL","action":"ACTION"}`. ACTION: `"auto-fixed"`, `"fixed"` (approved), `"skipped"` (explicit Skip), `"deferred"` (with `issue`, the filed number or `null`) or `"noted"`. Copy - `locus`, `kind`, `blocking` and `raised_by` from the classified output onto every finding. + `locus`, `kind`, `blocking`, `raised_by` and `ledger_id` from the classified output onto every finding. Merge revalidated invocation decisions by identity and advisory/defect kind; preserve `advisory`, `evidence_paths` and `helper_target`. Gate findings add `gate` (RH tag), `gate_id` (the tool's hunk identity), `reason` diff --git a/ship/sections/review-army.md.tmpl b/ship/sections/review-army.md.tmpl index c7d1e33d1..914eeb60a 100644 --- a/ship/sections/review-army.md.tmpl +++ b/ship/sections/review-army.md.tmpl @@ -96,7 +96,8 @@ but missing dispatched output still blocks continuation, even with a QA exceptio (all sources, queued Steps 10–11 included) as a JSON array to `.gstack/tmp/review-findings-p$PASS.json` with your file-write tool, each with `raised_by` (`core`, `qa`, a specialist, `red-team`, `adversarial-native`, - `outside-voice` or `fix-check`), then: + `outside-voice` or `fix-check`) and, for a dispatched reader's finding, `ledger_id` + (`/`, e.g. `p2/adversarial-native#3`), then: `~/.claude/skills/gstack/bin/gstack-review-scope classify --base --pass $PASS --range-base $RANGE_BASE --findings .gstack/tmp/review-findings-p$PASS.json --json > .gstack/tmp/review-classified-p$PASS.json` (omit `--range-base` on pass 1). Every Step 9.4 scratch file lives under `.gstack/tmp/` (gitignored): `` below is this pass's classified @@ -163,7 +164,8 @@ but missing dispatched output still blocks continuation, even with a QA exceptio row under its own skill (a tokenless `review` row would read UNVERIFIED): `~/.claude/skills/gstack/bin/gstack-review-log '{"skill":"review-late-defer","via":"ship","timestamp":"TIMESTAMP","status":"issues_found","issues_found":0,"critical":0,"informational":0,"pass":PASS,"review_scope":"SCOPE","findings":FINDINGS_JSON}'` with `findings` holding only the deferred records (`action: "deferred"`, `issue`, - `locus`, `kind`, `blocking` as in item 6) and no `--finish` token. + `locus`, `kind`, `blocking`, `ledger_id` as in item 6) and no `--finish` token. Log it in the + program that decides the defer, with that moment's full ISO 8601 TIMESTAMP, never at Step 19. 4. **Finish and log this pass before choosing the next step.** Apply each A) Restore by reverting only the relaxation. After any fix or restore in this pass, run item @@ -174,7 +176,8 @@ but missing dispatched output still blocks continuation, even with a QA exceptio REVIEW_START. Missing dispatched output uses `status:"unavailable"`, `completed:false` and `converged:false`; fixes also require `converged:false`. Then commit named fixed files, if any - (`git add && git commit -m "fix: pre-landing review fixes"`). + (`git add && git commit -m "fix: pre-landing review fixes" --trailer "Review-Finding: reader="`, + one trailer per fixed finding: the receipt counts fixes by it). 5. Output summary: `Pre-Landing Review: N issues — M auto-fixed, K asked (J fixed, L skipped), D deferred, O noted` @@ -213,7 +216,7 @@ but missing dispatched output still blocks continuation, even with a QA exceptio `{"fingerprint":"path:line:category","severity":"CRITICAL|INFORMATIONAL","action":"ACTION"}`. ACTION: `"auto-fixed"`, `"fixed"` (approved), `"skipped"` (explicit Skip), `"deferred"` (with `issue`, the filed number or `null`) or `"noted"`. Copy - `locus`, `kind`, `blocking` and `raised_by` from the classified output onto every finding. + `locus`, `kind`, `blocking`, `raised_by` and `ledger_id` from the classified output onto every finding. Merge revalidated invocation decisions by identity and advisory/defect kind; preserve `advisory`, `evidence_paths` and `helper_target`. Gate findings add `gate` (RH tag), `gate_id` (the tool's hunk identity), `reason` diff --git a/test/gstack-ship-receipt-docs.test.ts b/test/gstack-ship-receipt-docs.test.ts index db89a68fb..b398d1d44 100644 --- a/test/gstack-ship-receipt-docs.test.ts +++ b/test/gstack-ship-receipt-docs.test.ts @@ -49,7 +49,7 @@ describe('E18: a repository without a version source or CHANGELOG', () => { const out = path.join(repo.root, 'run'); const result = runBin(repo, 'gstack-ship-receipt', ['result', '--body', body, '--out', out]); expect(result.status).toBe(EXIT.ok); - expect(result.out.trim()).toBe(`GSTACK_RESULT: skill=ship status=complete run=${out}`); + expect(result.out.trim()).toBe(`GSTACK_RESULT: skill=ship status=complete head=${head} run=${out}`); expect(JSON.parse(fs.readFileSync(path.join(out, 'ship-run.json'), 'utf8')).version).toBe('none'); }); diff --git a/test/gstack-ship-receipt.test.ts b/test/gstack-ship-receipt.test.ts index 155fee501..eff3d9175 100644 --- a/test/gstack-ship-receipt.test.ts +++ b/test/gstack-ship-receipt.test.ts @@ -165,9 +165,10 @@ describe('gstack-ship-receipt result (/ship terminal line, GBRA-108)', () => { const ok = runBin(repo, 'gstack-ship-receipt', ['result', '--pr', '5'], env); expect(ok.status).toBe(EXIT.ok); const last = ok.out.trim().split('\n').at(-1)!; - const m = /^GSTACK_RESULT: skill=ship status=complete run=(.+)$/.exec(last); + const m = /^GSTACK_RESULT: skill=ship status=complete head=([0-9a-f]{40}) run=(.+)$/.exec(last); expect(m).not.toBeNull(); - const dir = m![1]!; + expect(m![1]).toBe(head); + const dir = m![2]!; expect(dir.startsWith(path.join(state, 'ship', 'runs'))).toBe(true); expect(dir.endsWith(`-${head.slice(0, 12)}`)).toBe(true); const run = JSON.parse(fs.readFileSync(path.join(dir, 'ship-run.json'), 'utf8')); @@ -202,13 +203,13 @@ describe('gstack-ship-receipt result (/ship terminal line, GBRA-108)', () => { fs.writeFileSync(bodyFile, renderReceiptBlock({ schema_version: 1, head, base: 'main' } as any)); const handoff = runBin(repo, 'gstack-ship-receipt', ['result', '--body', bodyFile, '--out', path.join(repo.root, 'run-out')], env); expect(handoff.status).toBe(EXIT.ok); - expect(handoff.out.trim()).toBe(`GSTACK_RESULT: skill=ship status=complete run=${path.join(repo.root, 'run-out')}`); + expect(handoff.out.trim()).toBe(`GSTACK_RESULT: skill=ship status=complete head=${head} run=${path.join(repo.root, 'run-out')}`); expect(JSON.parse(fs.readFileSync(path.join(repo.root, 'run-out', 'ship-run.json'), 'utf8'))).toMatchObject({ head, pr: null, receipt_source: bodyFile, session_kind: 'unattended' }); expect(runBin(repo, 'gstack-ship-receipt', ['result', '--body', path.join(repo.root, 'absent.md')], env).status).toBe(EXIT.refused); expect(runBin(repo, 'gstack-ship-receipt', ['result'], env).status).toBe(EXIT.usage); const relative = runBin(repo, 'gstack-ship-receipt', ['result', '--body', bodyFile, '--out', 'rel-out'], env); expect(relative.status).toBe(EXIT.ok); - expect(relative.out.trim()).toBe(`GSTACK_RESULT: skill=ship status=complete run=${path.join(repo.work, 'rel-out')}`); + expect(relative.out.trim()).toBe(`GSTACK_RESULT: skill=ship status=complete head=${head} run=${path.join(repo.work, 'rel-out')}`); expect(fs.existsSync(path.join(repo.work, 'rel-out', 'ship-run.json'))).toBe(true); const blocker = path.join(repo.root, 'not-a-dir'); fs.writeFileSync(blocker, 'file\n'); @@ -251,7 +252,7 @@ describe('gstack-ship-receipt result (/ship terminal line, GBRA-108)', () => { const pinned = path.join(dir, 'pinned.md'); fs.writeFileSync(pinned, renderReceiptBlock({ schema_version: 1, head: 'c'.repeat(40), base: 'main' } as any)); const override = runBin(repo, 'gstack-ship-receipt', ['result', '--body', pinned, '--head', 'c'.repeat(40), '--out', path.join(repo.root, 'pinned-out')], env); - expect([override.status, override.out.trim()]).toEqual([EXIT.ok, `GSTACK_RESULT: skill=ship status=complete run=${path.join(repo.root, 'pinned-out')}`]); + expect([override.status, override.out.trim()]).toEqual([EXIT.ok, `GSTACK_RESULT: skill=ship status=complete head=${'c'.repeat(40)} run=${path.join(repo.root, 'pinned-out')}`]); expect(JSON.parse(fs.readFileSync(path.join(repo.root, 'pinned-out', 'ship-run.json'), 'utf8')).head).toBe('c'.repeat(40)); const badHead = runBin(repo, 'gstack-ship-receipt', ['result', '--body', pinned, '--head', '../../etc'], env); expect([badHead.status, badHead.out]).toEqual([EXIT.usage, '']); @@ -260,7 +261,7 @@ describe('gstack-ship-receipt result (/ship terminal line, GBRA-108)', () => { expect([shortHead.status, shortHead.out]).toEqual([EXIT.usage, '']); expect(shortHead.err).toContain('--head must be a full hex commit id'); const upper = runBin(repo, 'gstack-ship-receipt', ['result', '--body', pinned, '--head', 'C'.repeat(40), '--out', path.join(repo.root, 'upper-out')], env); - expect([upper.status, upper.out.trim()]).toEqual([EXIT.ok, `GSTACK_RESULT: skill=ship status=complete run=${path.join(repo.root, 'upper-out')}`]); + expect([upper.status, upper.out.trim()]).toEqual([EXIT.ok, `GSTACK_RESULT: skill=ship status=complete head=${'c'.repeat(40)} run=${path.join(repo.root, 'upper-out')}`]); const noGit = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-receipt-nogit-')); const outside = runBin(repo, 'gstack-ship-receipt', ['result', '--body', pinned], env, noGit); expect([outside.status, outside.err.trim()]).toEqual([EXIT.usage, 'gstack-ship-receipt result: --head is required outside a git repository']); @@ -312,7 +313,7 @@ describe('gstack-ship-receipt carry (harness lane E row 3: receipt follows a hea expect(carried.status).toBe(EXIT.ok); expect(carried.out.trim().split('\n')).toEqual([ `CARRY: carried from ${reviewed.slice(0, 12)} to ${head.slice(0, 12)} (base merge + test-only change)`, - `GSTACK_RESULT: skill=ship status=complete run=${path.join(repo.root, 'carry-run')}`, + `GSTACK_RESULT: skill=ship status=complete head=${head} run=${path.join(repo.root, 'carry-run')}`, ]); expect(carried.err.trim()).toBe(`SHIP_RECEIPT: pr=5 head=${head.slice(0, 12)} version=- base=main passes=2 blocking_found=1 deferred=1 carried_from=${reviewed.slice(0, 12)}`); const after = extractReceipt(fs.readFileSync(body, 'utf8')); diff --git a/test/gstack-skill-start.test.ts b/test/gstack-skill-start.test.ts index 3764ba518..2cc14c5a6 100644 --- a/test/gstack-skill-start.test.ts +++ b/test/gstack-skill-start.test.ts @@ -413,7 +413,7 @@ describe('gstack-skill-start behavior', () => { expect(evalPlan).toContain('No helper writes a gate.json'); expect(evalPlan).not.toContain('gate list'); const ship = blocks.ship; - expect(ship).toContain('`GSTACK_RESULT: skill=ship status=complete run=`'); + expect(ship).toContain('`GSTACK_RESULT: skill=ship status=complete head= run=`'); expect(ship).toContain('`GSTACK_RESULT: skill=ship status=refused run=-`'); expect(ship).not.toContain('writes no run directory'); const records: Record = { @@ -437,7 +437,7 @@ describe('gstack-skill-start behavior', () => { const ship = runStart(['--skill', 'ship'], { GSTACK_SESSION_KIND: 'unattended' }); expect(ship.match(line)).toHaveLength(1); expect((ship.match(/^GSTACK_INSTRUCTION_BEGIN: (\S+)/gm) ?? []).map((h) => h.replace(/^GSTACK_INSTRUCTION_BEGIN: /, ''))).toEqual(['unattended-session']); - expect(ship).toContain('`GSTACK_RESULT: skill=ship status=complete run=`'); + expect(ship).toContain('`GSTACK_RESULT: skill=ship status=complete head= run=`'); expect(ship).toContain("update that PR's body and title with the receipt through the pr-body section's existing-PR path (never pr-create, never a push, and never `gh pr ready`"); expect(ship).toContain('the handoff section writes the receipt to .gstack/tmp/ship-receipt.md'); for (const skill of ['review', 'qa', 'autoplan']) { diff --git a/test/review-ledger-verdict-line.test.ts b/test/review-ledger-verdict-line.test.ts new file mode 100644 index 000000000..14582acd9 --- /dev/null +++ b/test/review-ledger-verdict-line.test.ts @@ -0,0 +1,38 @@ +/** + * GSTA-258 L4 addendum: the collector raised the native reader's own verdict line to CRITICAL. On #3901 (pass 2, gstack 1.94 + * branch) "There are three low-severity findings, all FIXABLE, and none blocks." collected as a blocker beside the three + * findings it summarizes, each of which already said "Severity: Low. Blocks: no". Excerpt of that captured output below. + */ +import { describe, expect, test } from 'bun:test'; +import { parseFindings } from '../lib/review-ledger'; + +const CAPTURED_3901_P2 = [ + 'REVIEWED_TREE: 7be72149fbd6d1b8e713c7c42ef4cd9ba6373e20 /workspace/gstack', + '', + 'I found no blocking defects in this range. There are three low-severity findings, all FIXABLE, and none blocks.', + '', + '### Findings', + '', + '1. **The remote-gate route hint ignores `--no-ci-carry`** (`lib/pregate/checks/lanes.ts`, the `route` condition). The hint prints whenever `remote_gate_runs` is declared. FIXABLE. Severity: Low. Blocks: no.', + '', + 'Recommendation: Ship as-is.', +].join('\n'); + +const labels = (out: string, reader = 'adversarial-native') => parseFindings(reader, out, true).map(f => f.label); + +describe('native verdict line (collector severity inflation)', () => { + test('the captured #3901 verdict line stays FIXABLE; the finding keeps its own Severity: Low', () => { + expect(labels(CAPTURED_3901_P2)).toEqual(['FIXABLE', 'Low']); + }); + + test('negative controls: a line that says something blocks, names a file:line, or leads with FIXABLE is still CRITICAL', () => { + expect(labels('Three findings are FIXABLE, and one blocks.')).toEqual(['CRITICAL']); + expect(labels('The token leak in log.ts:3 is FIXABLE, and none of the others blocks.')).toEqual(['CRITICAL']); + expect(labels('FIXABLE: none blocks the merge but the cache key drops the tree.')).toEqual(['CRITICAL']); + expect(labels('- FIXABLE, and nothing blocks')).toEqual(['CRITICAL']); + }); + + test('a stated Blocks field on the line wins: Blocks: yes stays CRITICAL', () => { + expect(labels('All FIXABLE, none blocks. Blocks: yes')).toEqual(['CRITICAL']); + }); +}); diff --git a/test/review-loop-binding.test.ts b/test/review-loop-binding.test.ts new file mode 100644 index 000000000..9e1a26eb8 --- /dev/null +++ b/test/review-loop-binding.test.ts @@ -0,0 +1,76 @@ +/** + * GST-1168: the receipt's review counts bind by finding id. Each case is a shape a real /ship run produced + * (GBEV-3 gbrain-evals#310, GEVA-223 gstack-evals#174, #3906 at 9417c7d, #3892/#3903/#3911); before the fix + * the summary paired rows and ledgers by path:line only and dropped rows it could not place without a word. + */ +import { describe, expect, test } from 'bun:test'; +import { parseFixTrailers, reviewLoopLine, summarizeReviewLoop, type LedgerPass } from '../lib/review-scope'; +import type { LedgerBlocker } from '../lib/review-ledger'; + +const SINCE = '2026-11-02T10:00:00.000Z'; +const at = (min: number) => new Date(Date.parse(SINCE) + min * 60_000).toISOString(); +const row = (skill: string, min: number, findings: Record[], extra: Record = {}) => + ({ skill, via: 'ship', review_scope: 'frozen', review_binding: { started_at: at(min) }, findings, ...extra }); +const blocker = (pass: string, id: string, locus: string | null): LedgerBlocker => ({ id, locus, key: `${pass}/${id}` }); +const pass = (min: number, ids: string[], fixCheck = false): LedgerPass => ({ started: Date.parse(at(min)), fixCheck, ids }); + +describe('review loop counts bind by finding id (GST-1168)', () => { + test('GBEV-3: a deferred finding logged late counts once and a fix-check never marks it fixed', () => { + const rows = [ + row('review', 1, [{ fingerprint: 'src/a.ts:10:correctness', severity: 'CRITICAL', action: 'fixed', blocking: true, raised_by: 'adversarial-native', ledger_id: 'p1/adversarial-native#1' }]), + row('review-late-defer', 30, [{ fingerprint: 'src/b.ts:20:security', severity: 'CRITICAL', action: 'deferred', issue: 311, blocking: true, raised_by: 'adversarial-native', ledger_id: 'p1/adversarial-native#2' }]), + ]; + const ledgers = [[blocker('p1', 'adversarial-native#1', 'src/a.ts:10'), blocker('p1', 'adversarial-native#2', 'src/b.ts:21')], []]; + const s = summarizeReviewLoop(rows, SINCE, ledgers, [pass(0, ['p1/adversarial-native#1', 'p1/adversarial-native#2']), pass(20, ['fc/fix-check#1'], true)]); + expect({ found: s.blocking_found, fixed: s.fixed, deferred: s.deferred, unrecorded: s.unrecorded }).toEqual({ found: 2, fixed: 1, deferred: 1, unrecorded: undefined }); + }); + + test('GEVA-223: native findings with no path:line pair with their rows by ledger_id', () => { + const ids = Array.from({ length: 9 }, (_, i) => `adversarial-native#${i + 1}`); + const findings = [ + ...ids.map((id, i) => ({ fingerprint: `lib/x.ts:${10 * (i + 1)}:correctness`, severity: 'CRITICAL', action: 'fixed', blocking: true, raised_by: 'adversarial-native', ledger_id: `p1/${id}` })), + ...Array.from({ length: 6 }, (_, i) => ({ fingerprint: `lib/y.ts:${i + 1}:style`, severity: 'INFORMATIONAL', action: 'noted', blocking: false, raised_by: 'specialist' })), + ]; + const s = summarizeReviewLoop([row('review', 1, findings)], SINCE, [ids.map(id => blocker('p1', id, null))], [pass(0, ids.map(id => `p1/${id}`))]); + expect({ found: s.blocking_found, fixed: s.fixed, noted: s.noted, unrecorded: s.unrecorded }).toEqual({ found: 9, fixed: 9, noted: 6, unrecorded: undefined }); + }); + + test('#3906: fixes named by Review-Finding trailers count even when no row recorded the findings', () => { + const ledger = [1, 2, 3].map(n => blocker('p1', `red-team#${n}`, `lib/auth.ts:${25 + 15 * n}`)); + const trailers = parseFixTrailers('9417c7d0000000000000000000000000000000000', ['p1/red-team#1 reader=red-team', 'p1/red-team#2 reader=red-team', 'p1/red-team#3 reader=red-team']); + const s = summarizeReviewLoop([row('review', 1, [])], SINCE, [ledger], [pass(0, ledger.map(b => b.key!))], trailers); + expect({ found: s.blocking_found, fixed: s.fixed, unrecorded: s.unrecorded }).toEqual({ found: 3, fixed: 3, unrecorded: undefined }); + expect(reviewLoopLine(s)).toContain('fixed=3'); + }); + + test('#3892/#3903/#3911: a row with no start token and no readable time is named unbound, never dropped silently', () => { + const tokenless = { skill: 'review', via: 'ship', pass: 2, timestamp: 'TIMESTAMP', findings: [{ fingerprint: 'lib/q.ts:7:correctness', severity: 'CRITICAL', action: 'fixed', blocking: true }] }; + const s = summarizeReviewLoop([row('review', 1, []), tokenless], SINCE); + expect(s.unbound).toEqual(['row review pass=2: no start token and no readable timestamp, so it cannot be placed in this run']); + expect(reviewLoopLine(s)).toContain('unbound=1'); + }); + + test('a ledger_id no ledger collected and a trailer naming nothing are unbound with their names', () => { + const rows = [row('review', 1, [{ fingerprint: 'lib/z.ts:3:correctness', severity: 'CRITICAL', action: 'fixed', blocking: true, raised_by: 'specialist', ledger_id: 'p9/specialist#4' }])]; + const s = summarizeReviewLoop(rows, SINCE, [[]], [pass(0, [])], parseFixTrailers('abc1234def567', ['p1/targeted#2 reader=targeted', 'no-reader-field'])); + expect(s.unbound).toEqual([ + 'ledger_id p9/specialist#4: no dispatch ledger of this run collected it', + "trailer abc1234def56 Review-Finding: p1/targeted#2 reader=targeted: names no finding this run's rows or ledgers hold", + ]); + }); + + test('#3911 at 4340ed4c: a fix-check row written twice counts once, and pass 1 with no row is named', () => { + const fc = { skill: 'review-fix-check', via: 'ship', pass: 2, commit: '4340ed4c', review_binding: { started_at: at(40) }, findings: [{ fingerprint: 'lib/p.ts:9:correctness', severity: 'CRITICAL', action: 'fixed', blocking: true, raised_by: 'fix-check', ledger_id: 'fc/fix-check#1' }] }; + const rows = [row('review', 20, [], { pass: 2, commit: '1111111' }), fc, { ...fc }]; + const passes = [{ ...pass(0, []), name: 'p1' }, { ...pass(20, []), name: 'p2' }, { ...pass(40, ['fc/fix-check#1'], true), name: 'fc' }]; + const s = summarizeReviewLoop(rows, SINCE, [[], [], [blocker('fc', 'fix-check#1', 'lib/p.ts:9')]], passes); + expect({ passes: s.passes, fix_checks: s.fix_checks, found: s.blocking_found, fixed: s.fixed }).toEqual({ passes: 1, fix_checks: 1, found: 1, fixed: 1 }); + expect(s.unbound).toEqual(['pass 1 (ledger p1): no review-log row, so its fixes and dispositions cannot be counted']); + }); + + test('rows without ledger_id keep pairing by path:line, as before', () => { + const rows = [row('review', 1, [{ fingerprint: 'src/a.ts:10:correctness', severity: 'CRITICAL', action: 'fixed', blocking: true, raised_by: 'specialist' }])]; + const s = summarizeReviewLoop(rows, SINCE, [[blocker('p1', 'specialist/security#1', 'src/a.ts:10')]]); + expect({ found: s.blocking_found, fixed: s.fixed, unrecorded: s.unrecorded, unbound: s.unbound }).toEqual({ found: 1, fixed: 1, unrecorded: undefined, unbound: undefined }); + }); +}); diff --git a/test/review-scope.test.ts b/test/review-scope.test.ts index 8b40ad3af..b166db1e7 100644 --- a/test/review-scope.test.ts +++ b/test/review-scope.test.ts @@ -223,7 +223,8 @@ describe('loop summary', () => { { skill: 'review', via: 'ship', review_binding: { state: 'changed', started_at: '2026-10-26T09:00:00.000Z' }, findings: [{ fingerprint: 'old:1:q', action: 'noted' }] }, { skill: 'review', via: 'ship', findings: [{ fingerprint: 'undated:1:q', action: 'noted' }] }, ]; - expect(summarizeReviewLoop(rows, '2026-10-26T09:30:00Z')).toEqual({ passes: 3, blocking_found: 1, deferred: 0, noted: 2, fixed: 1, scope: 'frozen', rounds: 3 }); + expect(summarizeReviewLoop(rows, '2026-10-26T09:30:00Z')).toEqual({ passes: 3, blocking_found: 1, deferred: 0, noted: 2, fixed: 1, scope: 'frozen', rounds: 3, + unbound: ['row review pass=?: no start token and no readable timestamp, so it cannot be placed in this run'] }); }); }); diff --git a/test/ship-receipt-result-head.test.ts b/test/ship-receipt-result-head.test.ts new file mode 100644 index 000000000..81adfc001 --- /dev/null +++ b/test/ship-receipt-result-head.test.ts @@ -0,0 +1,33 @@ +/** + * GST-1179: gstack-ship-receipt's terminal line names the full head it bound, so a leftover line from an earlier ship can't + * pass for a new head (harness-evals' merge lane requires it from 2026-11-09T08:00Z). + */ +import { afterEach, describe, expect, test } from 'bun:test'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { EXIT, parseResult } from '../lib/headless-artifacts'; +import { renderReceiptBlock } from '../lib/ship-receipt'; +import { cleanup, makeFixtureRepo, runBin, type FixtureRepo } from './helpers/restamp-fixture'; + +const repos: FixtureRepo[] = []; +afterEach(() => { while (repos.length) cleanup(repos.pop()!); }); + +describe('GSTACK_RESULT head= token (GST-1179)', () => { + test('result prints head= between status and run, and parseResult reads it', () => { + const repo = makeFixtureRepo({ prefix: 'ship-result-head-' }); repos.push(repo); + const head = repo.git('rev-parse', 'HEAD'); + const body = path.join(repo.root, 'body.md'); + fs.writeFileSync(body, renderReceiptBlock({ schema_version: 1, head, base: 'main' } as any)); + const out = path.join(repo.root, 'run'); + const r = runBin(repo, 'gstack-ship-receipt', ['result', '--body', body, '--out', out], { GSTACK_STATE_ROOT: path.join(repo.root, 'state') }); + expect(r.status).toBe(EXIT.ok); + const line = r.out.trim().split('\n').at(-1)!; + expect(line).toBe(`GSTACK_RESULT: skill=ship status=complete head=${head} run=${out}`); + expect(head).toMatch(/^[0-9a-f]{40}$/); + expect(parseResult(r.out)).toEqual({ skill: 'ship', status: 'complete', head, run: out }); + }); + + test('a line without head= still parses, with no head', () => { + expect(parseResult('GSTACK_RESULT: skill=pregate status=complete run=.gstack/tmp/pregate.json')).toEqual({ skill: 'pregate', status: 'complete', run: '.gstack/tmp/pregate.json' }); + }); +}); diff --git a/test/ship-receipt-runs.test.ts b/test/ship-receipt-runs.test.ts new file mode 100644 index 000000000..52a33c941 --- /dev/null +++ b/test/ship-receipt-runs.test.ts @@ -0,0 +1,58 @@ +/** + * GSTA-258 L4 receipt attribution: a PR body keeps every /ship run's receipt (append-only runs block), and the receipt + * schema carries K1's no_stop records for L5. + */ +import { describe, expect, test } from 'bun:test'; +import { extractReceipt, orderReceipt, placeReceiptFirst, readyTailLines, receiptRuns, RECEIPT_RUNS_KEPT, validateReceipt } from '../lib/ship-receipt'; +import type { ShipReceipt } from '../lib/headless-artifacts'; + +const receipt = (head: string, loop: Partial> = {}): ShipReceipt => + orderReceipt({ schema_version: 1, head, base: 'main', version: '1.94.1.0', review_loop: { passes: 1, blocking_found: 0, deferred: 0, ...loop } }); + +describe('receipt runs block', () => { + test('a write over a different receipt keeps the old run, oldest first, after the current block', () => { + let body = placeReceiptFirst('## Summary\nprose\n', receipt('a'.repeat(40), { passes: 2, blocking_found: 3, fixed: 3 })); + body = placeReceiptFirst(body, receipt('b'.repeat(40))); + body = placeReceiptFirst(body, receipt('c'.repeat(40), { passes: 1 })); + expect(extractReceipt(body).receipt!.head).toBe('c'.repeat(40)); + expect(receiptRuns(body).map(r => [r.head[0], r.review_loop?.blocking_found])).toEqual([['a', 3], ['b', 0]]); + expect(body.indexOf('gstack-receipt-runs')).toBeLessThan(body.indexOf('## Summary')); + expect(body.match(/## Summary/g)!.length).toBe(1); + }); + + test('rewriting the same receipt adds no run, and the block keeps the newest RECEIPT_RUNS_KEPT', () => { + const same = receipt('d'.repeat(40)); + expect(receiptRuns(placeReceiptFirst(placeReceiptFirst('', same), same))).toEqual([]); + let body = ''; + for (let i = 0; i < RECEIPT_RUNS_KEPT + 5; i++) body = placeReceiptFirst(body, receipt(i.toString(16).padStart(40, '0'))); + const runs = receiptRuns(body); + expect(runs.length).toBe(RECEIPT_RUNS_KEPT); + expect(runs.at(-1)!.head).toBe((RECEIPT_RUNS_KEPT + 3).toString(16).padStart(40, '0')); + }); +}); + +describe('READY tail (E1: the ready output is the whole READY)', () => { + const depth = (cls: 'docs' | 'code' | 'gate', risk?: Array<'money' | 'auth' | 'secret' | 'security' | 'data'>) => + ({ depth: 'full' as const, class: cls, runtime_lines: 10, rules: 'r', base: 'b', registry: null, gate_scan: { tagged: 0, head: 'h' }, reasons: [], ...(risk ? { risk } : {}) }); + test('a risk-class receipt asks for the cross-family read; any other PR is self-audited', () => { + expect(readyTailLines({ ...receipt('f'.repeat(40)), review_depth: depth('gate') })[3]).toStartWith('reviewed by (risk class: gate;'); + expect(readyTailLines({ ...receipt('f'.repeat(40)), review_depth: depth('code', ['money']) })[3]).toContain('risk class: money'); + expect(readyTailLines({ ...receipt('f'.repeat(40)), review_depth: depth('docs') })[3]).toBe('self-audited (not a risk class: no cross-family trail read)'); + }); + test('the review loop line carries fixed, unrecorded and unbound from the receipt', () => { + const lines = readyTailLines(receipt('f'.repeat(40), { passes: 2, blocking_found: 3, fixed: 2, unrecorded: 1, unbound: 1 })); + expect(lines[0]).toBe('REVIEW_LOOP: passes=2 blocking_found=3 deferred=0 noted=0 fixed=2 unrecorded=1 unbound=1'); + expect(lines.slice(1, 3)).toEqual([expect.stringMatching(/^TRIAL_MERGES: /), 'Attention:']); + expect(readyTailLines(undefined)[0]).toBe('REVIEW_LOOP: none in the receipt'); + }); +}); + +describe('no_stop receipt field (K1)', () => { + test('a record per routine stop validates; an unknown class or a missing field does not', () => { + const ok = orderReceipt({ ...receipt('e'.repeat(40)), no_stop: [{ class: 'pregate-flake-rerun', reason: 'one rerun of test/x.test.ts at the same tree', evidence: 'run 41562764513' }] }); + expect(validateReceipt(ok)).toEqual([]); + expect(Object.keys(ok).at(-1)).toBe('no_stop'); + expect(validateReceipt({ ...ok, no_stop: [{ class: 'merge-anyway', reason: 'x', evidence: 'y' }] } as any).length).toBeGreaterThan(0); + expect(validateReceipt({ ...ok, no_stop: [{ class: 'stamp-only-carry', reason: 'x' }] } as any).length).toBeGreaterThan(0); + }); +}); diff --git a/test/ship-recheck.test.ts b/test/ship-recheck.test.ts new file mode 100644 index 000000000..4722eb300 --- /dev/null +++ b/test/ship-recheck.test.ts @@ -0,0 +1,88 @@ +/** + * J1 (GSTA-258 L4): a scoped re-check after a refused receipt carry. Each refusal reason that does or does not qualify, + * the record binding, and the carry end to end: a bounded product change prints RECHECK: eligible and writes the packet, + * and a clean record bound to that range-diff carries the receipt with the reader named. + */ +import { afterEach, describe, expect, test } from 'bun:test'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { EXIT } from '../lib/headless-artifacts'; +import { extractReceipt, placeReceiptFirst } from '../lib/ship-receipt'; +import { planRecheck, readRecheck, RECHECK_RANGE_DIFF_CAP } from '../lib/ship-recheck'; +import { cleanup, makeFixtureRepo, runBin, type FixtureRepo } from './helpers/restamp-fixture'; + +const repos: FixtureRepo[] = []; +afterEach(() => { while (repos.length) cleanup(repos.pop()!); }); +const RANGE = '1: aaaaaaa ! 1: bbbbbbb feat: a=2\n @@ src/app.ts\n -export const a = 2;\n +export const a = 3;'; + +describe('planRecheck: which refusals qualify', () => { + test('a bounded product change on linear history against the same base qualifies', () => { + const p = planRecheck({ code: 'RECEIPT_CARRY_PRODUCT_CHANGED', ancestor: true, retarget: false, rangeDiff: RANGE }); + expect(p).toMatchObject({ ok: true, lines: 4 }); + }); + test.each([ + ['tree mismatch', { code: 'RECEIPT_CARRY_TREE_MISMATCH', ancestor: false, retarget: false, rangeDiff: RANGE }, 'is not a product change'], + ['refused inputs', { code: 'RECEIPT_CARRY_REFUSED', ancestor: true, retarget: false, rangeDiff: RANGE }, 'is not a product change'], + ['stale pregate', { code: 'RECEIPT_CARRY_PREGATE_STALE', ancestor: true, retarget: false, rangeDiff: RANGE }, 'is not a product change'], + ['rewritten history', { code: 'RECEIPT_CARRY_PRODUCT_CHANGED', ancestor: false, retarget: false, rangeDiff: RANGE }, 'rewritten history'], + ['retargeted stack', { code: 'RECEIPT_CARRY_PRODUCT_CHANGED', ancestor: true, retarget: true, rangeDiff: RANGE }, 'another base'], + ['unnamed product path', { code: 'RECEIPT_CARRY_PRODUCT_CHANGED', ancestor: true, retarget: false, rangeDiff: RANGE, unnamedPaths: true }, 'could not be named'], + ['range-diff failed', { code: 'RECEIPT_CARRY_PRODUCT_CHANGED', ancestor: true, retarget: false, rangeDiff: null }, 'failed or timed out'], + ['empty range-diff', { code: 'RECEIPT_CARRY_PRODUCT_CHANGED', ancestor: true, retarget: false, rangeDiff: '\n' }, 'empty'], + ['over the cap', { code: 'RECEIPT_CARRY_PRODUCT_CHANGED', ancestor: true, retarget: false, rangeDiff: Array.from({ length: RECHECK_RANGE_DIFF_CAP + 1 }, (_, i) => `l${i}`).join('\n') }, 'over the scoped re-check cap'], + ])('%s does not qualify', (_name, input, why) => { + const p = planRecheck(input); + expect(p.ok).toBe(false); + expect(!p.ok && p.reason).toContain(why); + }); +}); + +describe('readRecheck binds to from, head and the range-diff', () => { + test('only a clean record for this exact range-diff stands', () => { + const dir = fs.mkdtempSync(path.join(require('node:os').tmpdir(), 'recheck-')); + const bind = { from: 'a'.repeat(40), head: 'b'.repeat(40), sha256: 'c'.repeat(64) }; + const write = (o: object) => { const f = path.join(dir, `${Math.random()}.json`); fs.writeFileSync(f, JSON.stringify(o)); return f; }; + const good = { from: bind.from, head: bind.head, range_diff_sha256: bind.sha256, reader: 'anthropic/claude-opus-5-5', verdict: 'clean' }; + expect(readRecheck(write(good), bind).ok).toBe(true); + expect(readRecheck(write({ ...good, verdict: 'blocking' }), bind)).toMatchObject({ ok: false, message: expect.stringContaining('not clean') }); + expect(readRecheck(write({ ...good, range_diff_sha256: 'd'.repeat(64) }), bind)).toMatchObject({ ok: false, message: expect.stringContaining('range_diff_sha256') }); + expect(readRecheck(write({ ...good, reader: '' }), bind)).toMatchObject({ ok: false, message: 'the re-check names no reader' }); + fs.rmSync(dir, { recursive: true, force: true }); + }); +}); + +describe('carry with a scoped re-check (CLI)', () => { + test('a bounded product change prints RECHECK: eligible, keeps the refusal, and a clean record carries with the reader named', () => { + const repo = makeFixtureRepo({ prefix: 'ship-recheck-' }); repos.push(repo); + fs.mkdirSync(path.join(repo.work, 'src'), { recursive: true }); + fs.writeFileSync(path.join(repo.work, 'src', 'app.ts'), 'export const a = 1;\n'); + repo.git('add', '.'); repo.git('commit', '-qm', 'seed app'); repo.git('push', '-q', 'origin', 'main'); + repo.git('checkout', '-qb', 'feat'); + fs.writeFileSync(path.join(repo.work, 'src', 'app.ts'), 'export const a = 2;\n'); + repo.git('commit', '-qam', 'feat: a=2'); + const reviewed = repo.git('rev-parse', 'HEAD'); + fs.writeFileSync(path.join(repo.work, 'src', 'app.ts'), 'export const a = 3;\n'); + repo.git('commit', '-qam', 'fix: a=3'); + const head = repo.git('rev-parse', 'HEAD'); + const body = path.join(repo.root, 'body.md'); + fs.writeFileSync(body, placeReceiptFirst('## Summary\n', { schema_version: 1, pr: 5, head: reviewed, base: 'main', review_loop: { passes: 1, blocking_found: 0, deferred: 0 } } as any)); + const env = { GSTACK_STATE_ROOT: path.join(repo.root, 'state'), GSTACK_SESSION_KIND: 'unattended' }; + const refused = runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body], env); + expect(refused.status).toBe(EXIT.refused); + expect(refused.out.trim().split('\n')[0]).toBe('CARRY: refused (RECEIPT_CARRY_PRODUCT_CHANGED)'); + const m = /^RECHECK: eligible range_diff_lines=(\d+) range_diff_sha256=([0-9a-f]{64}) packet=(\S+);/m.exec(refused.err); + expect(m).not.toBeNull(); + expect(fs.readFileSync(m![3]!, 'utf8')).toContain('export const a = 3;'); + const record = path.join(repo.root, 'recheck.json'); + fs.writeFileSync(record, JSON.stringify({ from: reviewed, head, range_diff_sha256: m![2], reader: 'anthropic/claude-opus-5-5', verdict: 'clean' })); + const carried = runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--recheck', record, '--out', path.join(repo.root, 'run')], env); + expect(carried.status).toBe(EXIT.ok); + expect(carried.out.trim().split('\n')[0]).toBe(`CARRY: carried from ${reviewed.slice(0, 12)} to ${head.slice(0, 12)} (scoped re-check by anthropic/claude-opus-5-5 (range-diff ${m![1]} lines))`); + expect(extractReceipt(fs.readFileSync(body, 'utf8')).receipt).toMatchObject({ head, carry: { from: reviewed } }); + const stale = path.join(repo.root, 'stale.json'); + fs.writeFileSync(stale, JSON.stringify({ from: reviewed, head, range_diff_sha256: 'f'.repeat(64), reader: 'x', verdict: 'clean' })); + fs.writeFileSync(path.join(repo.work, 'src', 'app.ts'), 'export const a = 4;\n'); + repo.git('commit', '-qam', 'fix: a=4'); + expect(runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--recheck', stale], env).status).toBe(EXIT.refused); + }); +}); diff --git a/test/size-ledger/ship/gsta258-l4-ready-receipts.json b/test/size-ledger/ship/gsta258-l4-ready-receipts.json new file mode 100644 index 000000000..4a2a9e77c --- /dev/null +++ b/test/size-ledger/ship/gsta258-l4-ready-receipts.json @@ -0,0 +1,4 @@ +{ + "bytes": 412, + "why": "GSTA-258 L4: review fixes carry ledger_id and a Review-Finding trailer, late defers log at Step 11 (GST-1168), the result line names head= (GST-1179)" +} From 819fc25ad4dbc89c81482872b6a72c8b8ef3a666 Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 06:12:49 +0000 Subject: [PATCH 2/7] fix(ship): goldens, head= in the base-red line test, Severity field after a closing quote (ggstack#71); repin ship/sections/review-army.md under GST-508+GST-464+GST-967+GSTA-258-L4 (brief-identity no-loss check: every cut brief unchanged) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- changelog.d/gsta258-l4-ready-receipts.md | 4 ++-- lib/review-ledger.ts | 2 +- test/base-red.test.ts | 2 +- test/fixtures/golden/agy-ship-SKILL.md | 2 +- test/fixtures/golden/claude-ship-SKILL.md | 2 +- test/fixtures/golden/codex-ship-SKILL.md | 2 +- test/fixtures/golden/factory-ship-SKILL.md | 2 +- test/review-ledger-verdict-line.test.ts | 10 ++++++++++ test/review-packets-removed.test.ts | 3 ++- 9 files changed, 20 insertions(+), 9 deletions(-) diff --git a/changelog.d/gsta258-l4-ready-receipts.md b/changelog.d/gsta258-l4-ready-receipts.md index 0790d9a30..2b8b4845d 100644 --- a/changelog.d/gsta258-l4-ready-receipts.md +++ b/changelog.d/gsta258-l4-ready-receipts.md @@ -3,7 +3,7 @@ ### Changed - **`gstack-review-scope summary` binds by finding id.** A row finding carries `ledger_id` (`/`), each fix commit names what it fixed in a `Review-Finding: reader=` trailer, rows that record their head dedupe by kind, pass, head and finding ids, and a late defer counts once. Rows and findings without an id keep the path:line pairing. - **What can't bind is named, never a silent zero.** A row with no start token or readable time, a `pN` ledger with no review row, a `ledger_id` no ledger holds and a trailer naming nothing each print `REVIEW_SCOPE_UNBOUND: : `, and the line and receipt carry `unbound=N`. -- **The native reader's own verdict line no longer collects as a blocker.** "There are three low-severity findings, all FIXABLE, and none blocks." (#3901) collected as CRITICAL beside the findings it summarized; it now collects at FIXABLE. A line that says something blocks, names a file:line or leads with FIXABLE still collects as CRITICAL. +- **The native reader's own verdict line no longer collects as a blocker.** "There are three low-severity findings, all FIXABLE, and none blocks." (#3901) collected as CRITICAL beside the findings it summarized; it now collects at FIXABLE. A line that says something blocks, names a file:line or leads with FIXABLE still collects as CRITICAL. A finding's `Severity: Low. Blocks: no` after a closing quote (`… its base." Severity: Low`) now counts as its own field: on ggstack#71 that one quote made one of four identical findings CRITICAL. - **`gstack-ship-receipt carry` offers a scoped re-check (J1).** On `RECEIPT_CARRY_PRODUCT_CHANGED` with linear history against the reviewed base and a range-diff (plus the diff of the changed product paths) of at most 400 lines, carry prints `RECHECK: eligible … packet=`. One reader on the packet and `carry --recheck ` rebind the receipt, unless HEAD needs a deeper review depth or another risk class. Rebased branches, retargeted stacks and anything over the cap still need /ship. - **Every /ship run's receipt is kept.** When `write` or `carry` replaces a different receipt, the old one's identifiers and counters move to an append-only `gstack-receipt-runs` block in the PR body (newest 30). - **`gstack-ship-queue ready` prints the whole READY.** It adds the receipt's `REVIEW_LOOP:` line, a `TRIAL_MERGES:` line for the lane to fill and an `Attention:` stub that opens with `reviewed by ` on risk-class PRs and `self-audited` on every other PR. docs/unattended.md now asks for the cross-family trail read only on risk-class PRs. @@ -12,6 +12,6 @@ ### Tests - `test/review-loop-binding.test.ts`: the GBEV-3, GEVA-223, #3906, #3892/#3903/#3911 and #3911 duplicate-row cases. On main the GBEV-3 case reproduces its receipt exactly (3 found, 2 fixed) and GEVA-223's pattern (18 found, 9 unrecorded). -- `test/review-ledger-verdict-line.test.ts`: the captured #3901 output, with negative controls. +- `test/review-ledger-verdict-line.test.ts`: the captured #3901 output and ggstack#71's findings, with negative controls. - `test/ship-recheck.test.ts`: every refusal that does and doesn't qualify, the record binding, and the carry end to end. - `test/ship-receipt-runs.test.ts`, `test/ship-receipt-result-head.test.ts`: the runs block, the READY tail, `no_stop` validation and the `head=` token. diff --git a/lib/review-ledger.ts b/lib/review-ledger.ts index e1fa339be..67c4d7838 100644 --- a/lib/review-ledger.ts +++ b/lib/review-ledger.ts @@ -73,7 +73,7 @@ const STATED_SEVERITY = new RegExp(`(?]\\s+|\\d+[.)]\\s+)?\\**\\s*Severity\\**\\s*:?\\s*\\**\\s*((?:${LABELS})(?:\\s*[/→]\\s*\\**\\s*(?:${LABELS}))*)(?![A-Za-z0-9])|(?:[.;,|()]\\s*|\\s[—–-]\\s*)\\**\\s*Severity\\**\\s*:\\s*\\**\\s*((?:${LABELS})(?:\\s*[/→]\\s*\\**\\s*(?:${LABELS}))*)(?![A-Za-z0-9])(?=\\**\\s*(?:$|[.;,|()*—–]|-\\s|Blocks)))`, 'gi'); +const SEVERITY_FIELD = new RegExp(`(?:^(?:[-*+>]\\s+|\\d+[.)]\\s+)?\\**\\s*Severity\\**\\s*:?\\s*\\**\\s*((?:${LABELS})(?:\\s*[/→]\\s*\\**\\s*(?:${LABELS}))*)(?![A-Za-z0-9])|(?:[.;,|()]["'”’\`]*\\s*|\\s[—–-]\\s*)\\**\\s*Severity\\**\\s*:\\s*\\**\\s*((?:${LABELS})(?:\\s*[/→]\\s*\\**\\s*(?:${LABELS}))*)(?![A-Za-z0-9])(?=\\**\\s*(?:$|[.;,|()*—–]|-\\s|Blocks)))`, 'gi'); /** * A label leading its line past list, quote, heading, emphasis and emoji marks (one mark per step, so a long divider stays linear) * and an optional file:line, closed by a mark or a separator, or by a space, comma or parenthesis after an all-caps word ("[P0] …", diff --git a/test/base-red.test.ts b/test/base-red.test.ts index c2f868b8d..221957d7c 100644 --- a/test/base-red.test.ts +++ b/test/base-red.test.ts @@ -144,7 +144,7 @@ describe('pause, receipt and wait', () => { fs.writeFileSync(bodyFile, placeReceiptFirst('## Summary\n', { schema_version: 1, pr: 7, head, base: 'main', base_red: receiptBaseRed(pause) } as any)); const res = bin('gstack-ship-receipt', ['result', '--body', bodyFile, '--head', head, '--out', path.join(root, 'run')]); expect(res.status).toBe(EXIT.fail); - expect(res.stdout.trim()).toBe(`GSTACK_RESULT: skill=ship status=incomplete run=${path.join(root, 'run')}`); + expect(res.stdout.trim()).toBe(`GSTACK_RESULT: skill=ship status=incomplete head=${head} run=${path.join(root, 'run')}`); expect(res.stderr).toContain(`SHIP_PAUSED: base-red cases=1 base=${pause.base_sha.slice(0, 12)} lane=tests`); expect(JSON.parse(fs.readFileSync(path.join(root, 'run', 'ship-run.json'), 'utf8')).status).toBe('incomplete'); const carry = bin('gstack-ship-receipt', ['carry', '--body', bodyFile]); diff --git a/test/fixtures/golden/agy-ship-SKILL.md b/test/fixtures/golden/agy-ship-SKILL.md index 45130e661..e7f441d1e 100644 --- a/test/fixtures/golden/agy-ship-SKILL.md +++ b/test/fixtures/golden/agy-ship-SKILL.md @@ -957,7 +957,7 @@ The last line printed is the result line a parent greps (docs/unattended.md): ``` It binds the receipt to HEAD, writes the run record and prints `GSTACK_RESULT: skill=ship -status=complete run=`; `QUEUE_RECEIPT_STALE` means HEAD moved after the receipt (rewrite, +status=complete head= run=`; `QUEUE_RECEIPT_STALE` means HEAD moved after the receipt (rewrite, repost, rerun). A run that STOPs earlier ends with `GSTACK_RESULT: skill=ship status=refused run=-`. --- diff --git a/test/fixtures/golden/claude-ship-SKILL.md b/test/fixtures/golden/claude-ship-SKILL.md index 125947cba..47496e238 100644 --- a/test/fixtures/golden/claude-ship-SKILL.md +++ b/test/fixtures/golden/claude-ship-SKILL.md @@ -956,7 +956,7 @@ The last line printed is the result line a parent greps (docs/unattended.md): ``` It binds the receipt to HEAD, writes the run record and prints `GSTACK_RESULT: skill=ship -status=complete run=`; `QUEUE_RECEIPT_STALE` means HEAD moved after the receipt (rewrite, +status=complete head= run=`; `QUEUE_RECEIPT_STALE` means HEAD moved after the receipt (rewrite, repost, rerun). A run that STOPs earlier ends with `GSTACK_RESULT: skill=ship status=refused run=-`. --- diff --git a/test/fixtures/golden/codex-ship-SKILL.md b/test/fixtures/golden/codex-ship-SKILL.md index bf8e4ddcc..6cac22e78 100644 --- a/test/fixtures/golden/codex-ship-SKILL.md +++ b/test/fixtures/golden/codex-ship-SKILL.md @@ -966,7 +966,7 @@ $GSTACK_ROOT/bin/gstack-ship-receipt result --pr # handoff: --body ``` It binds the receipt to HEAD, writes the run record and prints `GSTACK_RESULT: skill=ship -status=complete run=`; `QUEUE_RECEIPT_STALE` means HEAD moved after the receipt (rewrite, +status=complete head= run=`; `QUEUE_RECEIPT_STALE` means HEAD moved after the receipt (rewrite, repost, rerun). A run that STOPs earlier ends with `GSTACK_RESULT: skill=ship status=refused run=-`. --- diff --git a/test/fixtures/golden/factory-ship-SKILL.md b/test/fixtures/golden/factory-ship-SKILL.md index a34726a90..fdf11bc92 100644 --- a/test/fixtures/golden/factory-ship-SKILL.md +++ b/test/fixtures/golden/factory-ship-SKILL.md @@ -968,7 +968,7 @@ $GSTACK_ROOT/bin/gstack-ship-receipt result --pr # handoff: --body ``` It binds the receipt to HEAD, writes the run record and prints `GSTACK_RESULT: skill=ship -status=complete run=`; `QUEUE_RECEIPT_STALE` means HEAD moved after the receipt (rewrite, +status=complete head= run=`; `QUEUE_RECEIPT_STALE` means HEAD moved after the receipt (rewrite, repost, rerun). A run that STOPs earlier ends with `GSTACK_RESULT: skill=ship status=refused run=-`. --- diff --git a/test/review-ledger-verdict-line.test.ts b/test/review-ledger-verdict-line.test.ts index 14582acd9..4f42cc3eb 100644 --- a/test/review-ledger-verdict-line.test.ts +++ b/test/review-ledger-verdict-line.test.ts @@ -32,6 +32,16 @@ describe('native verdict line (collector severity inflation)', () => { expect(labels('- FIXABLE, and nothing blocks')).toEqual(['CRITICAL']); }); + // ggstack#71 (GGST-15, gstack 1.94.4.0, gate-class plan): four identically shaped native findings, the first labeled CRITICAL + // because its Severity field followed a closing quote ('… its base." Severity: Low'). Excerpt of findings 1 and 2, as returned. + test('ggstack#71: a Severity field after a closing quote is still the finding\'s own field', () => { + const out = ['## Findings', '', "**1. The CHANGELOG describes the \"behind main\" rule wrongly (CHANGELOG.md:10).** It says \"a pull request that doesn't contain main's tip runs the suite here as before.\" gstack's actual rule is different: the run's merge-commit tree must equal the head's tree. That means the head had to contain its base as it was when CI ran, and that base isn't always main (stacked PRs). Nothing in `findCarry` or `verifyRun` compares against main's current tip, so a run from before main moved still carries. /ship's own merge of the base usually changes the tree before this matters, but the text states a guarantee the code doesn't make. FIXABLE: reword it to something like \"a pull_request run made while the branch was behind its base.\" Severity: Low. Blocks: no", '', "**2. `files` is a static file listing, and run.sh skips Godot silently (test.yml:46, pregate.json:7).** tree.json's `files` comes from the same `ls -1 test/*.sh test/*.py test/*.gd` as the inventory, so the coverage check passes by construction. The receipt's `tests=9` counts files, not tests. run.sh prints SKIP and exits 0 when `$GODOT` isn't executable or neither `xvfb-run` nor `DISPLAY` is available. Web has `REQUIRE_WEB`, but nothing equivalent makes a missing Godot fail. Today the Godot and xvfb install steps fail hard, so nothing falsely carries. If a later workflow edit makes either step soft, CI would still list the `.gd` tests as covered. A local run without Godot skips them the same way, so the carry is never weaker than the lane it replaces. FIXABLE: add `REQUIRE_GODOT=1`, or fail the record step when the run.sh output contains `test: SKIP`. Severity: Low. Blocks: no", '', 'Recommendation: Ship as-is.'].join('\n'); + expect(labels(out)).toEqual(['Low', 'Low']); + expect(labels('1. **Wording (a.md:3).** FIXABLE: say \'the base\'. Severity: Low. Blocks: no')).toEqual(['Low']); + expect(labels('1. **Wording (a.md:3).** FIXABLE: say \u201cthe base.\u201d Severity: Low. Blocks: no')).toEqual(['Low']); + expect(labels('1. **Data loss (a.ts:3).** FIXABLE: guard it." Severity: Low. Blocks: yes')).toEqual(['CRITICAL']); + }); + test('a stated Blocks field on the line wins: Blocks: yes stays CRITICAL', () => { expect(labels('All FIXABLE, none blocks. Blocks: yes')).toEqual(['CRITICAL']); }); diff --git a/test/review-packets-removed.test.ts b/test/review-packets-removed.test.ts index 89eaf5aa5..9b54cc7fb 100644 --- a/test/review-packets-removed.test.ts +++ b/test/review-packets-removed.test.ts @@ -73,10 +73,11 @@ const NO_LOSS_REPLAYS = { 'GST-967': "GST-967 dispatch-superset no-loss check (test/review-depth.test.ts, 'GST-967 dispatch-superset no-loss check'): over 1,440 planReview cases the branch plans exactly what main planned at 1022f4f except the 71 gate-class stops before Step 11, which add only Step 11's adversarial, outside and targeted voices, so no reviewer main dispatches is dropped or re-efforted; the brief bodies lib/review-brief.ts cuts from the ship sections hash to main's; only the lead's repeat-decision and Step 11 instructions changed; reviewer text is unchanged", 'GST-508+GST-464+GST-967': "wave review-53cfb5bfe011 merge of main 68410828f into #3852: ship/sections/review-army.md and adversarial.md are main's GST-967 bytes plus exactly the GST-508+GST-464 lines (the merged file minus main equals the wave minus fc292e22f, line for line), so GST-967's dispatch-superset no-loss check and GST-508's no-loss replay both hold; merged-head checks in GStack Minder drive handoffs/GSTA-202/merge-6841-checks.md", 'GST-508+R51+GST-911+GST-967': "wave review-53cfb5bfe011 merge of main 68410828f into #3852: lib/review-depth.ts is main's GST-967 bytes plus exactly the GST-508+R51+GST-911 lines (the merged file minus main equals the wave minus fc292e22f, line for line); GST-967's dispatch-superset no-loss check (test/review-depth.test.ts) passes on the merged file, and the GST-508+R51+GST-911 checks are rerun at the merged head (GStack Minder drive handoffs/GSTA-202/merge-6841-checks.md)", + 'GST-508+GST-464+GST-967+GSTA-258-L4': "GSTA-258 L4 (GST-1168) brief-identity no-loss check (the brief-body test below): ship/sections/review-army.md is the GST-508+GST-464+GST-967 bytes plus Step 9.4 lead instructions outside the cut spans (a finding's ledger_id, the Review-Finding fix-commit trailer, logging a late defer when it is decided); every reviewer brief lib/review-brief.ts cuts from it still hashes to BRIEF_BODIES, so no reviewer sees a changed byte", } as const; type Replay = keyof typeof NO_LOSS_REPLAYS; const REVIEWER_BRIEFS: Record = { - 'ship/sections/review-army.md': { sha: 'de3e0d35f2af56492c42fd287b6868fbe19bce96b86321e74a4947ba909b74eb', replay: 'GST-508+GST-464+GST-967' }, + 'ship/sections/review-army.md': { sha: '22c37a64fd937f94ce231f8d815c401f232e0b06da8a1e57d3d15a18d3c9f2c0', replay: 'GST-508+GST-464+GST-967+GSTA-258-L4' }, 'ship/sections/adversarial.md': { sha: '0142d6bc3f2a5bceb9f8b2adef1d3a206e5836c99a0667fe995568da2e80a5a6', replay: 'GST-508+GST-464+GST-967' }, 'review/sections/review-army.md': { sha: '78329b64640260cd8831ca81a9189f3f8dcade9c406431631d3f2cd798cd3967', replay: '#3638' }, 'review/sections/adversarial.md': { sha: 'e6962fa25b78f362654f2954f79683e524d2ebd452d1494f8a2b099888e8e255', replay: 'G1' }, From 8788d2402719c68d3ba2f5727cfdc79a5ec78b62 Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 06:32:59 +0000 Subject: [PATCH 3/7] fix: pre-landing review fixes (J1 never re-checks a legacy receipt, a cut path walk or moved base context; legacy READY asks for the cross-family read) Review-Finding: p1/specialist-batched#1 reader=security Review-Finding: p1/specialist-batched#2 reader=testing Review-Finding: p1/red-team#1 reader=red-team Review-Finding: p1/red-team#2 reader=red-team Review-Finding: p1/red-team#3 reader=red-team Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- bin/gstack-ship-receipt | 5 +- lib/ship-receipt.ts | 4 +- lib/ship-recheck.ts | 18 ++++- test/ship-receipt-runs.test.ts | 1 + test/ship-recheck.test.ts | 144 ++++++++++++++++++++++++--------- 5 files changed, 130 insertions(+), 42 deletions(-) diff --git a/bin/gstack-ship-receipt b/bin/gstack-ship-receipt index 527160383..7a4f1dbb8 100755 --- a/bin/gstack-ship-receipt +++ b/bin/gstack-ship-receipt @@ -696,6 +696,7 @@ function cmdCarry(rest: string[], json: boolean): number { } const current = retarget ? headPatch : productHead; const changedProduct: string[] = []; + let walkCut = false; const contextPaths: string[] = []; const stampOnly: string[] = []; if (retarget && reviewed !== current) changedProduct.push(`HEAD's product diff against ${base} is not the one reviewed against ${reviewedBase} (the parent has not fully landed on ${base}, or the branch changed since)`); @@ -712,7 +713,7 @@ function cmdCarry(rest: string[], json: boolean): number { // A stamp file the branch changed as product (its own row) whose reviewed-to-current change is a stamp on its own lines // and that the base did not touch holds no unreviewed change, so the comparison leaves its bytes out (GST-980). for (const p of [...touched].filter(p => movedHead.has(p) || movedBase.has(p)).sort()) { - if (changedProduct.length >= CARRY_NAMED_PATHS) break; + if (changedProduct.length >= CARRY_NAMED_PATHS) { walkCut = true; break; } const cls = changed.find(c => c.path === p)?.class ?? classifyCarryPath(p, rules.extra); if (cls === 'metadata' && spec.ownRows.includes(p) && !movedBase.has(p)) stampOnly.push(p); if (cls !== 'product') continue; @@ -764,7 +765,7 @@ function cmdCarry(rest: string[], json: boolean): number { const named = changedProduct.filter(p => !p.startsWith('(') && !p.includes(' (')); const inter = named.length ? spawnSync('git', ['diff', '--no-color', '--no-ext-diff', from, head, '--', ...named.map(p => `:(literal)${p}`)], { cwd: repoRoot, encoding: 'utf8', timeout: 30_000, maxBuffer: 64 * 1024 * 1024 }) : null; const fullRange = rangeRaw.status === 0 && inter?.status === 0 ? `${rangeRaw.stdout.trim()}\n${inter.stdout.trim()}` : null; - const r = planRecheck({ code: 'RECEIPT_CARRY_PRODUCT_CHANGED', ancestor, retarget, rangeDiff: fullRange, unnamedPaths: named.length !== changedProduct.length }); + const r = planRecheck({ code: 'RECEIPT_CARRY_PRODUCT_CHANGED', ancestor, retarget, rangeDiff: fullRange, unnamedPaths: named.length !== changedProduct.length, legacy: !receipt.review_depth, walkCut, contextPaths: contextPaths.length }); const record = flag(rest, '--recheck'); if (!r.ok) process.stderr.write(`RECHECK: not eligible (${r.reason}); run /ship on ${head.slice(0, 12)}\n`); else if (record) { diff --git a/lib/ship-receipt.ts b/lib/ship-receipt.ts index bc5e37cce..1149bc647 100644 --- a/lib/ship-receipt.ts +++ b/lib/ship-receipt.ts @@ -169,11 +169,13 @@ export function readyTailLines(r: ShipReceipt | undefined): string[] { const loop = parseReviewLoop(r?.review_loop); const opt = (k: keyof ReviewLoopReceipt) => (loop?.[k] ? ` ${k}=${loop[k]}` : ''); const risk = riskClassReceipt(r); + const legacy = !!r && !r.review_depth; return [ loop ? `REVIEW_LOOP: passes=${loop.passes} blocking_found=${loop.blocking_found} deferred=${loop.deferred} noted=${loop.noted ?? 0} fixed=${loop.fixed ?? 0}${opt('unrecorded')}${opt('unbound')}${opt('fix_checks')}` : 'REVIEW_LOOP: none in the receipt', 'TRIAL_MERGES: ', 'Attention:', - risk ? `reviewed by (risk class: ${oneLine([r?.review_depth?.class === 'gate' ? 'gate' : '', ...(r?.review_depth?.risk ?? [])].filter(Boolean).join(', '))}; rule 27 cross-family trail read)` : 'self-audited (not a risk class: no cross-family trail read)', + legacy ? 'reviewed by (risk class unknown: the receipt records no review_depth; rule 27 cross-family trail read)' + : risk ? `reviewed by (risk class: ${oneLine([r?.review_depth?.class === 'gate' ? 'gate' : '', ...(r?.review_depth?.risk ?? [])].filter(Boolean).join(', '))}; rule 27 cross-family trail read)` : 'self-audited (not a risk class: no cross-family trail read)', '', ]; } diff --git a/lib/ship-recheck.ts b/lib/ship-recheck.ts index aa62ff37d..2253f295c 100644 --- a/lib/ship-recheck.ts +++ b/lib/ship-recheck.ts @@ -7,8 +7,9 @@ * * Qualifies: RECEIPT_CARRY_PRODUCT_CHANGED on linear history (the receipt's head is an ancestor of HEAD) against the same base. * Does not qualify: RECEIPT_CARRY_TREE_MISMATCH (rewritten history: the reviewed commits are gone, so there is no range to - * diff), a retargeted stack (the reviewed range was against another base), an unreadable or empty range-diff, a range-diff - * over RECHECK_RANGE_DIFF_CAP lines, and every RECEIPT_CARRY_REFUSED / RECEIPT_CARRY_PREGATE_STALE (those need their inputs + * diff), a retargeted stack (the reviewed range was against another base), a legacy receipt with no review_depth, more + * changed product paths than the carry walk names, a path it cannot name, base context moved next to reviewed hunks, + * an unreadable or empty range-diff, a range-diff over RECHECK_RANGE_DIFF_CAP lines, and every RECEIPT_CARRY_REFUSED / RECEIPT_CARRY_PREGATE_STALE (those need their inputs * repaired, not a review). */ import { createHash } from 'node:crypto'; @@ -16,7 +17,15 @@ import * as fs from 'node:fs'; export const RECHECK_RANGE_DIFF_CAP = 400; /** rangeDiff: \`git range-diff\` of the two ranges followed by the diff of the changed product paths between the heads (a range-diff names added commits without their content). */ -export type RecheckInput = { code: string; ancestor: boolean; retarget: boolean; rangeDiff: string | null; unnamedPaths?: boolean }; +export type RecheckInput = { + code: string; ancestor: boolean; retarget: boolean; rangeDiff: string | null; unnamedPaths?: boolean; + /** The receipt records no review_depth (a legacy receipt), so carry cannot compare HEAD's risk class with the reviewed one. */ + legacy?: boolean; + /** The changed-path walk stopped at CARRY_NAMED_PATHS, so later changed product paths would be missing from the packet. */ + walkCut?: boolean; + /** Paths whose only difference is base context next to reviewed hunks: they need RECEIPT_CARRY_SCOPED_REVIEW's packet, not this one. */ + contextPaths?: number; +}; export type RecheckPlan = { ok: true; lines: number; sha256: string } | { ok: false; reason: string }; const sha = (s: string) => createHash('sha256').update(s).digest('hex'); @@ -26,7 +35,10 @@ export function planRecheck(i: RecheckInput): RecheckPlan { if (i.code !== 'RECEIPT_CARRY_PRODUCT_CHANGED') return { ok: false, reason: `${i.code} is not a product change a review can clear` }; if (!i.ancestor) return { ok: false, reason: 'rewritten history: the reviewed head is not an ancestor of HEAD, so there is no range to diff' }; if (i.retarget) return { ok: false, reason: 'the receipt was reviewed against another base (a retargeted stack)' }; + if (i.legacy) return { ok: false, reason: 'the receipt records no review_depth, so a changed risk class could not be caught' }; + if (i.walkCut) return { ok: false, reason: 'more changed product paths than the walk names, so the packet would miss some' }; if (i.unnamedPaths) return { ok: false, reason: 'a changed product path could not be named, so the packet would miss it' }; + if (i.contextPaths) return { ok: false, reason: 'base context also moved next to reviewed hunks, which needs the scoped review of those hunks' }; if (i.rangeDiff === null) return { ok: false, reason: 'git range-diff failed or timed out' }; const text = i.rangeDiff.trim(); if (!text) return { ok: false, reason: 'git range-diff is empty, so it shows nothing for a reader to check' }; diff --git a/test/ship-receipt-runs.test.ts b/test/ship-receipt-runs.test.ts index 52a33c941..f9957f900 100644 --- a/test/ship-receipt-runs.test.ts +++ b/test/ship-receipt-runs.test.ts @@ -44,6 +44,7 @@ describe('READY tail (E1: the ready output is the whole READY)', () => { expect(lines[0]).toBe('REVIEW_LOOP: passes=2 blocking_found=3 deferred=0 noted=0 fixed=2 unrecorded=1 unbound=1'); expect(lines.slice(1, 3)).toEqual([expect.stringMatching(/^TRIAL_MERGES: /), 'Attention:']); expect(readyTailLines(undefined)[0]).toBe('REVIEW_LOOP: none in the receipt'); + expect(readyTailLines(receipt('f'.repeat(40)))[3]).toStartWith('reviewed by (risk class unknown: the receipt records no review_depth'); }); }); diff --git a/test/ship-recheck.test.ts b/test/ship-recheck.test.ts index 4722eb300..3d57f3279 100644 --- a/test/ship-recheck.test.ts +++ b/test/ship-recheck.test.ts @@ -1,10 +1,13 @@ /** * J1 (GSTA-258 L4): a scoped re-check after a refused receipt carry. Each refusal reason that does or does not qualify, * the record binding, and the carry end to end: a bounded product change prints RECHECK: eligible and writes the packet, - * and a clean record bound to that range-diff carries the receipt with the reader named. + * a clean record bound to that packet carries the receipt with the reader named, and a changed risk class, a legacy + * receipt, more paths than the walk names and moved base context never take the re-check (the /ship p1 red-team and + * security findings). */ import { afterEach, describe, expect, test } from 'bun:test'; import * as fs from 'node:fs'; +import * as os from 'node:os'; import * as path from 'node:path'; import { EXIT } from '../lib/headless-artifacts'; import { extractReceipt, placeReceiptFirst } from '../lib/ship-receipt'; @@ -14,22 +17,25 @@ import { cleanup, makeFixtureRepo, runBin, type FixtureRepo } from './helpers/re const repos: FixtureRepo[] = []; afterEach(() => { while (repos.length) cleanup(repos.pop()!); }); const RANGE = '1: aaaaaaa ! 1: bbbbbbb feat: a=2\n @@ src/app.ts\n -export const a = 2;\n +export const a = 3;'; +const OK = { code: 'RECEIPT_CARRY_PRODUCT_CHANGED', ancestor: true, retarget: false, rangeDiff: RANGE }; describe('planRecheck: which refusals qualify', () => { test('a bounded product change on linear history against the same base qualifies', () => { - const p = planRecheck({ code: 'RECEIPT_CARRY_PRODUCT_CHANGED', ancestor: true, retarget: false, rangeDiff: RANGE }); - expect(p).toMatchObject({ ok: true, lines: 4 }); + expect(planRecheck(OK)).toMatchObject({ ok: true, lines: 4 }); }); test.each([ - ['tree mismatch', { code: 'RECEIPT_CARRY_TREE_MISMATCH', ancestor: false, retarget: false, rangeDiff: RANGE }, 'is not a product change'], - ['refused inputs', { code: 'RECEIPT_CARRY_REFUSED', ancestor: true, retarget: false, rangeDiff: RANGE }, 'is not a product change'], - ['stale pregate', { code: 'RECEIPT_CARRY_PREGATE_STALE', ancestor: true, retarget: false, rangeDiff: RANGE }, 'is not a product change'], - ['rewritten history', { code: 'RECEIPT_CARRY_PRODUCT_CHANGED', ancestor: false, retarget: false, rangeDiff: RANGE }, 'rewritten history'], - ['retargeted stack', { code: 'RECEIPT_CARRY_PRODUCT_CHANGED', ancestor: true, retarget: true, rangeDiff: RANGE }, 'another base'], - ['unnamed product path', { code: 'RECEIPT_CARRY_PRODUCT_CHANGED', ancestor: true, retarget: false, rangeDiff: RANGE, unnamedPaths: true }, 'could not be named'], - ['range-diff failed', { code: 'RECEIPT_CARRY_PRODUCT_CHANGED', ancestor: true, retarget: false, rangeDiff: null }, 'failed or timed out'], - ['empty range-diff', { code: 'RECEIPT_CARRY_PRODUCT_CHANGED', ancestor: true, retarget: false, rangeDiff: '\n' }, 'empty'], - ['over the cap', { code: 'RECEIPT_CARRY_PRODUCT_CHANGED', ancestor: true, retarget: false, rangeDiff: Array.from({ length: RECHECK_RANGE_DIFF_CAP + 1 }, (_, i) => `l${i}`).join('\n') }, 'over the scoped re-check cap'], + ['tree mismatch', { ...OK, code: 'RECEIPT_CARRY_TREE_MISMATCH', ancestor: false }, 'is not a product change'], + ['refused inputs', { ...OK, code: 'RECEIPT_CARRY_REFUSED' }, 'is not a product change'], + ['stale pregate', { ...OK, code: 'RECEIPT_CARRY_PREGATE_STALE' }, 'is not a product change'], + ['rewritten history', { ...OK, ancestor: false }, 'rewritten history'], + ['retargeted stack', { ...OK, retarget: true }, 'another base'], + ['legacy receipt', { ...OK, legacy: true }, 'no review_depth'], + ['walk cut at the named-path limit', { ...OK, walkCut: true }, 'more changed product paths'], + ['unnamed product path', { ...OK, unnamedPaths: true }, 'could not be named'], + ['base context moved', { ...OK, contextPaths: 1 }, 'base context'], + ['range-diff failed', { ...OK, rangeDiff: null }, 'failed or timed out'], + ['empty range-diff', { ...OK, rangeDiff: '\n' }, 'empty'], + ['over the cap', { ...OK, rangeDiff: Array.from({ length: RECHECK_RANGE_DIFF_CAP + 1 }, (_, i) => `l${i}`).join('\n') }, 'over the scoped re-check cap'], ])('%s does not qualify', (_name, input, why) => { const p = planRecheck(input); expect(p.ok).toBe(false); @@ -39,7 +45,7 @@ describe('planRecheck: which refusals qualify', () => { describe('readRecheck binds to from, head and the range-diff', () => { test('only a clean record for this exact range-diff stands', () => { - const dir = fs.mkdtempSync(path.join(require('node:os').tmpdir(), 'recheck-')); + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'recheck-')); const bind = { from: 'a'.repeat(40), head: 'b'.repeat(40), sha256: 'c'.repeat(64) }; const write = (o: object) => { const f = path.join(dir, `${Math.random()}.json`); fs.writeFileSync(f, JSON.stringify(o)); return f; }; const good = { from: bind.from, head: bind.head, range_diff_sha256: bind.sha256, reader: 'anthropic/claude-opus-5-5', verdict: 'clean' }; @@ -51,38 +57,104 @@ describe('readRecheck binds to from, head and the range-diff', () => { }); }); +/** A branch reviewed at one commit, with a receipt written by gstack-ship-receipt write (so it records review_depth). */ +function reviewed(files: Record = { 'src/app.ts': 'export const a = 2;\n' }) { + const repo = makeFixtureRepo({ prefix: 'ship-recheck-' }); repos.push(repo); + fs.mkdirSync(path.join(repo.work, 'src'), { recursive: true }); + fs.writeFileSync(path.join(repo.work, 'src', 'app.ts'), 'export const a = 1;\n'); + repo.git('add', '.'); repo.git('commit', '-qm', 'seed app'); repo.git('push', '-q', 'origin', 'main'); + repo.git('checkout', '-qb', 'feat'); + for (const [f, text] of Object.entries(files)) { fs.mkdirSync(path.dirname(path.join(repo.work, f)), { recursive: true }); fs.writeFileSync(path.join(repo.work, f), text); } + repo.git('add', '.'); repo.git('commit', '-qm', 'feat'); + const from = repo.git('rev-parse', 'HEAD'); + const body = path.join(repo.root, 'body.md'); + fs.writeFileSync(body, '## Summary\n'); + const env = { GSTACK_STATE_ROOT: path.join(repo.root, 'state'), GSTACK_SESSION_KIND: 'unattended' }; + const w = runBin(repo, 'gstack-ship-receipt', ['write', '--pr', '5', '--base', 'main', '--body', body], env); + expect([w.status, w.err.slice(0, 300)]).toEqual([EXIT.ok, expect.any(String)]); + expect(extractReceipt(fs.readFileSync(body, 'utf8')).receipt!.review_depth).toBeDefined(); + const commit = (changes: Record, msg: string) => { + for (const [f, text] of Object.entries(changes)) { fs.mkdirSync(path.dirname(path.join(repo.work, f)), { recursive: true }); fs.writeFileSync(path.join(repo.work, f), text); } + repo.git('add', '.'); repo.git('commit', '-qm', msg); + return repo.git('rev-parse', 'HEAD'); + }; + return { repo, from, body, env, commit }; +} +const eligible = (err: string) => /^RECHECK: eligible range_diff_lines=(\d+) range_diff_sha256=([0-9a-f]{64}) packet=(\S+);/m.exec(err); + describe('carry with a scoped re-check (CLI)', () => { test('a bounded product change prints RECHECK: eligible, keeps the refusal, and a clean record carries with the reader named', () => { - const repo = makeFixtureRepo({ prefix: 'ship-recheck-' }); repos.push(repo); - fs.mkdirSync(path.join(repo.work, 'src'), { recursive: true }); - fs.writeFileSync(path.join(repo.work, 'src', 'app.ts'), 'export const a = 1;\n'); - repo.git('add', '.'); repo.git('commit', '-qm', 'seed app'); repo.git('push', '-q', 'origin', 'main'); - repo.git('checkout', '-qb', 'feat'); - fs.writeFileSync(path.join(repo.work, 'src', 'app.ts'), 'export const a = 2;\n'); - repo.git('commit', '-qam', 'feat: a=2'); - const reviewed = repo.git('rev-parse', 'HEAD'); - fs.writeFileSync(path.join(repo.work, 'src', 'app.ts'), 'export const a = 3;\n'); - repo.git('commit', '-qam', 'fix: a=3'); - const head = repo.git('rev-parse', 'HEAD'); - const body = path.join(repo.root, 'body.md'); - fs.writeFileSync(body, placeReceiptFirst('## Summary\n', { schema_version: 1, pr: 5, head: reviewed, base: 'main', review_loop: { passes: 1, blocking_found: 0, deferred: 0 } } as any)); - const env = { GSTACK_STATE_ROOT: path.join(repo.root, 'state'), GSTACK_SESSION_KIND: 'unattended' }; + const { repo, from, body, env, commit } = reviewed(); + const head = commit({ 'src/app.ts': 'export const a = 3;\n' }, 'fix: a=3'); const refused = runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body], env); expect(refused.status).toBe(EXIT.refused); expect(refused.out.trim().split('\n')[0]).toBe('CARRY: refused (RECEIPT_CARRY_PRODUCT_CHANGED)'); - const m = /^RECHECK: eligible range_diff_lines=(\d+) range_diff_sha256=([0-9a-f]{64}) packet=(\S+);/m.exec(refused.err); + const m = eligible(refused.err); expect(m).not.toBeNull(); expect(fs.readFileSync(m![3]!, 'utf8')).toContain('export const a = 3;'); const record = path.join(repo.root, 'recheck.json'); - fs.writeFileSync(record, JSON.stringify({ from: reviewed, head, range_diff_sha256: m![2], reader: 'anthropic/claude-opus-5-5', verdict: 'clean' })); + fs.writeFileSync(record, JSON.stringify({ from, head, range_diff_sha256: m![2], reader: 'anthropic/claude-opus-5-5', verdict: 'clean' })); const carried = runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--recheck', record, '--out', path.join(repo.root, 'run')], env); - expect(carried.status).toBe(EXIT.ok); - expect(carried.out.trim().split('\n')[0]).toBe(`CARRY: carried from ${reviewed.slice(0, 12)} to ${head.slice(0, 12)} (scoped re-check by anthropic/claude-opus-5-5 (range-diff ${m![1]} lines))`); - expect(extractReceipt(fs.readFileSync(body, 'utf8')).receipt).toMatchObject({ head, carry: { from: reviewed } }); + expect([carried.status, carried.err.slice(0, 400)]).toEqual([EXIT.ok, expect.any(String)]); + expect(carried.out.trim().split('\n')[0]).toBe(`CARRY: carried from ${from.slice(0, 12)} to ${head.slice(0, 12)} (scoped re-check by anthropic/claude-opus-5-5 (range-diff ${m![1]} lines))`); + expect(extractReceipt(fs.readFileSync(body, 'utf8')).receipt).toMatchObject({ head, carry: { from } }); const stale = path.join(repo.root, 'stale.json'); - fs.writeFileSync(stale, JSON.stringify({ from: reviewed, head, range_diff_sha256: 'f'.repeat(64), reader: 'x', verdict: 'clean' })); - fs.writeFileSync(path.join(repo.work, 'src', 'app.ts'), 'export const a = 4;\n'); - repo.git('commit', '-qam', 'fix: a=4'); - expect(runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--recheck', stale], env).status).toBe(EXIT.refused); + fs.writeFileSync(stale, JSON.stringify({ from: head, head, range_diff_sha256: 'f'.repeat(64), reader: 'x', verdict: 'clean' })); + commit({ 'src/app.ts': 'export const a = 4;\n' }, 'fix: a=4'); + const bad = runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--recheck', stale], env); + expect([bad.status, bad.out.trim().split('\n')[0]]).toEqual([EXIT.refused, 'CARRY: refused (RECEIPT_CARRY_REFUSED)']); + }); + + test('a clean re-check cannot carry across a changed risk class', () => { + const { repo, from, body, env, commit } = reviewed(); + const before = extractReceipt(fs.readFileSync(body, 'utf8')).receipt!.review_depth!; + const head = commit({ 'src/app.ts': 'export const a = 3;\n', 'src/billing.ts': 'export function charge(cents: number) { return stripe.charges.create({ amount: cents }); }\n' }, 'feat: billing'); + const refused = runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body], env); + const m = eligible(refused.err); + expect(m).not.toBeNull(); + const record = path.join(repo.root, 'recheck.json'); + fs.writeFileSync(record, JSON.stringify({ from, head, range_diff_sha256: m![2], reader: 'x', verdict: 'clean' })); + const r = runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--recheck', record], env); + expect(r.status).toBe(EXIT.refused); + expect(r.err).toMatch(/risk class|review depth/); + expect(before.risk ?? []).not.toContain('money'); + }); + + test('a legacy receipt, more changed paths than the walk names, and moved base context print RECHECK: not eligible', () => { + const legacy = reviewed(); + fs.writeFileSync(legacy.body, placeReceiptFirst('## Summary\n', { schema_version: 1, pr: 5, head: legacy.from, base: 'main' } as any)); + legacy.commit({ 'src/app.ts': 'export const a = 3;\n' }, 'fix'); + expect(runBin(legacy.repo, 'gstack-ship-receipt', ['carry', '--body', legacy.body], legacy.env).err).toContain('RECHECK: not eligible (the receipt records no review_depth'); + + const many = Object.fromEntries(Array.from({ length: 22 }, (_, i) => [`src/f${i}.ts`, `export const f${i} = 1;\n`])); + const wide = reviewed(many); + wide.commit(Object.fromEntries(Object.keys(many).map(f => [f, 'export const changed = 2;\n'])), 'fix: all'); + expect(runBin(wide.repo, 'gstack-ship-receipt', ['carry', '--body', wide.body], wide.env).err).toContain('RECHECK: not eligible (more changed product paths'); + }); + + test('base context moved next to a reviewed hunk plus a product edit elsewhere is not eligible (red-team p1 repro)', () => { + const lines = (n: number, edits: Record = {}) => Array.from({ length: 10 }, (_, i) => edits[i + 1] ?? `line ${i + 1}`).join('\n') + '\n'; + const repo = makeFixtureRepo({ prefix: 'ship-recheck-ctx-' }); repos.push(repo); + fs.mkdirSync(path.join(repo.work, 'src'), { recursive: true }); + fs.writeFileSync(path.join(repo.work, 'src', 'c.ts'), lines(10)); + fs.writeFileSync(path.join(repo.work, 'src', 'app.ts'), 'export const a = 1;\n'); + repo.git('add', '.'); repo.git('commit', '-qm', 'seed'); repo.git('push', '-q', 'origin', 'main'); + repo.git('checkout', '-qb', 'feat'); + fs.writeFileSync(path.join(repo.work, 'src', 'c.ts'), lines(10, { 6: 'branch line 6' })); + repo.git('commit', '-qam', 'feat: c6'); + const body = path.join(repo.root, 'body.md'); + fs.writeFileSync(body, '## Summary\n'); + const env = { GSTACK_STATE_ROOT: path.join(repo.root, 'state'), GSTACK_SESSION_KIND: 'unattended' }; + expect(runBin(repo, 'gstack-ship-receipt', ['write', '--pr', '5', '--base', 'main', '--body', body], env).status).toBe(EXIT.ok); + repo.git('checkout', '-q', 'main'); + fs.writeFileSync(path.join(repo.work, 'src', 'c.ts'), lines(10, { 4: 'main line 4' })); + repo.git('commit', '-qam', 'main: c4'); repo.git('push', '-q', 'origin', 'main'); + repo.git('checkout', '-q', 'feat'); + repo.git('merge', '-q', '--no-edit', 'origin/main'); + fs.writeFileSync(path.join(repo.work, 'src', 'app.ts'), 'export const a = 2;\n'); + repo.git('commit', '-qam', 'fix: app'); + const r = runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body], env); + expect(r.status).toBe(EXIT.refused); + expect(r.err).toContain('RECHECK: not eligible (base context also moved'); }); }); From 64cab9f7112244b14e22f4b986f0ca58d4e36c29 Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 07:01:25 +0000 Subject: [PATCH 4/7] fix: pre-landing review fixes (Step 11: re-check refuses binary paths and packets over 64 KB; fingerprint trailers bind findings with a ledger_id; the runs fence closes only on its own line) Review-Finding: v/outside-voice#1 reader=outside-voice Review-Finding: v/outside-voice#2 reader=outside-voice Review-Finding: v/adversarial-native#1 reader=adversarial-native Review-Finding: v/targeted#1 reader=targeted Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- bin/gstack-ship-receipt | 4 +++- lib/review-scope.ts | 5 +++-- lib/ship-receipt.ts | 2 +- lib/ship-recheck.ts | 11 +++++++++-- test/review-loop-binding.test.ts | 6 ++++++ test/ship-receipt-runs.test.ts | 9 +++++++++ test/ship-recheck.test.ts | 8 +++++++- 7 files changed, 38 insertions(+), 7 deletions(-) diff --git a/bin/gstack-ship-receipt b/bin/gstack-ship-receipt index 7a4f1dbb8..233c43ff5 100755 --- a/bin/gstack-ship-receipt +++ b/bin/gstack-ship-receipt @@ -765,7 +765,9 @@ function cmdCarry(rest: string[], json: boolean): number { const named = changedProduct.filter(p => !p.startsWith('(') && !p.includes(' (')); const inter = named.length ? spawnSync('git', ['diff', '--no-color', '--no-ext-diff', from, head, '--', ...named.map(p => `:(literal)${p}`)], { cwd: repoRoot, encoding: 'utf8', timeout: 30_000, maxBuffer: 64 * 1024 * 1024 }) : null; const fullRange = rangeRaw.status === 0 && inter?.status === 0 ? `${rangeRaw.stdout.trim()}\n${inter.stdout.trim()}` : null; - const r = planRecheck({ code: 'RECEIPT_CARRY_PRODUCT_CHANGED', ancestor, retarget, rangeDiff: fullRange, unnamedPaths: named.length !== changedProduct.length, legacy: !receipt.review_depth, walkCut, contextPaths: contextPaths.length }); + const numstat = named.length ? spawnSync('git', ['diff', '--numstat', from, head, '--', ...named.map(p => `:(literal)${p}`)], { cwd: repoRoot, encoding: 'utf8', timeout: 30_000, maxBuffer: 16 * 1024 * 1024 }) : null; + const binary = numstat?.status !== 0 || (numstat?.stdout ?? '').split('\n').some(l => l.startsWith('-\t-\t')); + const r = planRecheck({ code: 'RECEIPT_CARRY_PRODUCT_CHANGED', ancestor, retarget, rangeDiff: fullRange, unnamedPaths: named.length !== changedProduct.length, legacy: !receipt.review_depth, walkCut, contextPaths: contextPaths.length, binary }); const record = flag(rest, '--recheck'); if (!r.ok) process.stderr.write(`RECHECK: not eligible (${r.reason}); run /ship on ${head.slice(0, 12)}\n`); else if (record) { diff --git a/lib/review-scope.ts b/lib/review-scope.ts index 7822fd958..a2bb74aa9 100644 --- a/lib/review-scope.ts +++ b/lib/review-scope.ts @@ -423,7 +423,7 @@ export function summarizeReviewLoop(rows: readonly Record[], since: const rowKey = (r: Record) => JSON.stringify([r.skill, r.pass ?? null, r.commit_full ?? r.commit ?? '', (Array.isArray(r.findings) ? r.findings : []).map((f: any) => (f && typeof f === 'object' ? (typeof f.ledger_id === 'string' ? f.ledger_id : findingIdentity(f)) : '')).sort()]); const loopRows = [...new Map(rows.filter(r => inRun(r) && loopSkill(r)).map((r, i) => [r.commit_full || r.commit ? rowKey(r) : `#${i}`, r] as const)).values()]; const passes = loopRows.filter(r => r.skill === 'review'); - type Seen = { locus: string | null; raiser: string | null; classified: boolean; blocks: boolean; candidate: boolean; disposed: boolean; ledgerId?: string }; + type Seen = { locus: string | null; raiser: string | null; classified: boolean; blocks: boolean; candidate: boolean; disposed: boolean; ledgerId?: string; fingerprint?: string }; const seen = new Map(); const deferred = new Set(); const noted = new Set(); @@ -447,6 +447,7 @@ export function summarizeReviewLoop(rows: readonly Record[], since: candidate: !!prior?.candidate || f.severity === undefined || rank(f.severity) === 4 || rank(f.raw_severity) === 4 || f.blocking === true, disposed: !!prior?.disposed || DISPOSITIONS.includes(f.action), ...(ledgerId ?? prior?.ledgerId ? { ledgerId: ledgerId ?? prior?.ledgerId } : {}), + ...(prior?.fingerprint ?? findingIdentity(f) ? { fingerprint: prior?.fingerprint ?? findingIdentity(f) } : {}), }); if (f.action === 'deferred') deferred.add(id); if (f.action === 'noted') noted.add(id); @@ -479,7 +480,7 @@ export function summarizeReviewLoop(rows: readonly Record[], since: } for (const [id, n] of lastPaired) if (followed(n)) followedIds.add(id); for (const t of trailers) { - const hit = [...seen].find(([k, s]) => k === t.finding || s.ledgerId === t.finding); + const hit = [...seen].find(([k, s]) => k === t.finding || s.ledgerId === t.finding || s.fingerprint === t.finding); if (hit) { hit[1].disposed = true; fixed.add(hit[0]); continue; } const only = ledgerOnlyKeys.find(l => l.key === t.finding || l.id === t.finding); if (only) { only.fixed = true; continue; } diff --git a/lib/ship-receipt.ts b/lib/ship-receipt.ts index 1149bc647..83e72a7d7 100644 --- a/lib/ship-receipt.ts +++ b/lib/ship-receipt.ts @@ -99,7 +99,7 @@ export function bindReceipt(body: string, pr: string | undefined, accepted: read */ export const RUNS_FENCE = 'gstack-receipt-runs'; export const RECEIPT_RUNS_KEPT = 30; -const RUNS_RE = /```gstack-receipt-runs[ \t]*\r?\n([\s\S]*?)\r?\n?```\n?/; +const RUNS_RE = /```gstack-receipt-runs[ \t]*\r?\n([\s\S]*?)\r?\n```[ \t]*(?:\r?\n|$)/; export type ReceiptRun = Pick & { depth?: string; carried_from?: string }; export function receiptRuns(body: string): ReceiptRun[] { const m = RUNS_RE.exec(body); diff --git a/lib/ship-recheck.ts b/lib/ship-recheck.ts index 2253f295c..db1a85c43 100644 --- a/lib/ship-recheck.ts +++ b/lib/ship-recheck.ts @@ -8,19 +8,23 @@ * Qualifies: RECEIPT_CARRY_PRODUCT_CHANGED on linear history (the receipt's head is an ancestor of HEAD) against the same base. * Does not qualify: RECEIPT_CARRY_TREE_MISMATCH (rewritten history: the reviewed commits are gone, so there is no range to * diff), a retargeted stack (the reviewed range was against another base), a legacy receipt with no review_depth, more - * changed product paths than the carry walk names, a path it cannot name, base context moved next to reviewed hunks, - * an unreadable or empty range-diff, a range-diff over RECHECK_RANGE_DIFF_CAP lines, and every RECEIPT_CARRY_REFUSED / RECEIPT_CARRY_PREGATE_STALE (those need their inputs + * changed product paths than the carry walk names, a path it cannot name, a binary path, base context moved next to reviewed hunks, + * an unreadable or empty range-diff, a packet over RECHECK_RANGE_DIFF_CAP lines or RECHECK_PACKET_BYTES bytes, and every RECEIPT_CARRY_REFUSED / RECEIPT_CARRY_PREGATE_STALE (those need their inputs * repaired, not a review). */ import { createHash } from 'node:crypto'; import * as fs from 'node:fs'; export const RECHECK_RANGE_DIFF_CAP = 400; +/** The packet's byte cap: 400 lines of unbounded width (a minified bundle) would not bound the one reader's input. */ +export const RECHECK_PACKET_BYTES = 64 * 1024; /** rangeDiff: \`git range-diff\` of the two ranges followed by the diff of the changed product paths between the heads (a range-diff names added commits without their content). */ export type RecheckInput = { code: string; ancestor: boolean; retarget: boolean; rangeDiff: string | null; unnamedPaths?: boolean; /** The receipt records no review_depth (a legacy receipt), so carry cannot compare HEAD's risk class with the reviewed one. */ legacy?: boolean; + /** A changed product path is binary, so the packet would show only "Binary files … differ" for it. */ + binary?: boolean; /** The changed-path walk stopped at CARRY_NAMED_PATHS, so later changed product paths would be missing from the packet. */ walkCut?: boolean; /** Paths whose only difference is base context next to reviewed hunks: they need RECEIPT_CARRY_SCOPED_REVIEW's packet, not this one. */ @@ -38,12 +42,15 @@ export function planRecheck(i: RecheckInput): RecheckPlan { if (i.legacy) return { ok: false, reason: 'the receipt records no review_depth, so a changed risk class could not be caught' }; if (i.walkCut) return { ok: false, reason: 'more changed product paths than the walk names, so the packet would miss some' }; if (i.unnamedPaths) return { ok: false, reason: 'a changed product path could not be named, so the packet would miss it' }; + if (i.binary) return { ok: false, reason: 'a changed product path is binary, so the packet cannot show its contents' }; if (i.contextPaths) return { ok: false, reason: 'base context also moved next to reviewed hunks, which needs the scoped review of those hunks' }; if (i.rangeDiff === null) return { ok: false, reason: 'git range-diff failed or timed out' }; const text = i.rangeDiff.trim(); if (!text) return { ok: false, reason: 'git range-diff is empty, so it shows nothing for a reader to check' }; const lines = text.split('\n').length; if (lines > RECHECK_RANGE_DIFF_CAP) return { ok: false, reason: `git range-diff is ${lines} lines, over the scoped re-check cap of ${RECHECK_RANGE_DIFF_CAP}` }; + const bytes = Buffer.byteLength(text); + if (bytes > RECHECK_PACKET_BYTES) return { ok: false, reason: `the packet is ${bytes} bytes, over the scoped re-check cap of ${RECHECK_PACKET_BYTES}` }; return { ok: true, lines, sha256: sha(text) }; } diff --git a/test/review-loop-binding.test.ts b/test/review-loop-binding.test.ts index 9e1a26eb8..a8c40e84b 100644 --- a/test/review-loop-binding.test.ts +++ b/test/review-loop-binding.test.ts @@ -68,6 +68,12 @@ describe('review loop counts bind by finding id (GST-1168)', () => { expect(s.unbound).toEqual(['pass 1 (ledger p1): no review-log row, so its fixes and dispositions cannot be counted']); }); + test('a fingerprint trailer binds a finding that also carries a ledger_id (outside voice #2)', () => { + const rows = [row('review', 1, [{ fingerprint: 'src/auth.ts:10:correctness', severity: 'CRITICAL', blocking: true, raised_by: 'red-team', ledger_id: 'p1/red-team#1' }])]; + const s = summarizeReviewLoop(rows, SINCE, [[blocker('p1', 'red-team#1', 'src/auth.ts:10')]], [pass(0, ['p1/red-team#1'])], parseFixTrailers('abc', ['src/auth.ts:10:correctness reader=red-team'])); + expect({ fixed: s.fixed, unrecorded: s.unrecorded, unbound: s.unbound }).toEqual({ fixed: 1, unrecorded: undefined, unbound: undefined }); + }); + test('rows without ledger_id keep pairing by path:line, as before', () => { const rows = [row('review', 1, [{ fingerprint: 'src/a.ts:10:correctness', severity: 'CRITICAL', action: 'fixed', blocking: true, raised_by: 'specialist' }])]; const s = summarizeReviewLoop(rows, SINCE, [[blocker('p1', 'specialist/security#1', 'src/a.ts:10')]]); diff --git a/test/ship-receipt-runs.test.ts b/test/ship-receipt-runs.test.ts index f9957f900..9752b23c8 100644 --- a/test/ship-receipt-runs.test.ts +++ b/test/ship-receipt-runs.test.ts @@ -20,6 +20,15 @@ describe('receipt runs block', () => { expect(body.match(/## Summary/g)!.length).toBe(1); }); + test('a fence inside a recorded reason never closes the runs block (adversarial p-voices #1)', () => { + const withReason = (head: string) => orderReceipt({ ...receipt(head, { fix_checks: 1 }), review_loop_exception: { accepted_by: 'Garry', date: '2026-11-03', via: 'x', reason: 'the \x60\x60\x60retry\x60\x60\x60 block is fine', fix_check: { head, findings: 0 } } }); + let body = placeReceiptFirst('## Summary\nprose\n', withReason('b'.repeat(40))); + body = placeReceiptFirst(body, withReason('c'.repeat(40))); + body = placeReceiptFirst(body, withReason('d'.repeat(40))); + expect(receiptRuns(body).map(r => r.head[0])).toEqual(['b', 'c']); + expect(body.endsWith('## Summary\nprose\n')).toBe(true); + }); + test('rewriting the same receipt adds no run, and the block keeps the newest RECEIPT_RUNS_KEPT', () => { const same = receipt('d'.repeat(40)); expect(receiptRuns(placeReceiptFirst(placeReceiptFirst('', same), same))).toEqual([]); diff --git a/test/ship-recheck.test.ts b/test/ship-recheck.test.ts index 3d57f3279..1fcaab7eb 100644 --- a/test/ship-recheck.test.ts +++ b/test/ship-recheck.test.ts @@ -11,7 +11,7 @@ import * as os from 'node:os'; import * as path from 'node:path'; import { EXIT } from '../lib/headless-artifacts'; import { extractReceipt, placeReceiptFirst } from '../lib/ship-receipt'; -import { planRecheck, readRecheck, RECHECK_RANGE_DIFF_CAP } from '../lib/ship-recheck'; +import { planRecheck, readRecheck, RECHECK_PACKET_BYTES, RECHECK_RANGE_DIFF_CAP } from '../lib/ship-recheck'; import { cleanup, makeFixtureRepo, runBin, type FixtureRepo } from './helpers/restamp-fixture'; const repos: FixtureRepo[] = []; @@ -33,6 +33,8 @@ describe('planRecheck: which refusals qualify', () => { ['walk cut at the named-path limit', { ...OK, walkCut: true }, 'more changed product paths'], ['unnamed product path', { ...OK, unnamedPaths: true }, 'could not be named'], ['base context moved', { ...OK, contextPaths: 1 }, 'base context'], + ['binary product path', { ...OK, binary: true }, 'binary'], + ['wide packet over the byte cap', { ...OK, rangeDiff: 'x'.repeat(RECHECK_PACKET_BYTES + 1) }, 'bytes'], ['range-diff failed', { ...OK, rangeDiff: null }, 'failed or timed out'], ['empty range-diff', { ...OK, rangeDiff: '\n' }, 'empty'], ['over the cap', { ...OK, rangeDiff: Array.from({ length: RECHECK_RANGE_DIFF_CAP + 1 }, (_, i) => `l${i}`).join('\n') }, 'over the scoped re-check cap'], @@ -130,6 +132,10 @@ describe('carry with a scoped re-check (CLI)', () => { const wide = reviewed(many); wide.commit(Object.fromEntries(Object.keys(many).map(f => [f, 'export const changed = 2;\n'])), 'fix: all'); expect(runBin(wide.repo, 'gstack-ship-receipt', ['carry', '--body', wide.body], wide.env).err).toContain('RECHECK: not eligible (more changed product paths'); + + const bin = reviewed({ 'src/app.wasm': '\0asm\x01\0\0\0' }); + bin.commit({ 'src/app.wasm': '\0asm\x01\0\0\0' + 'x'.repeat(4096) + '\0' }, 'fix: new wasm'); + expect(runBin(bin.repo, 'gstack-ship-receipt', ['carry', '--body', bin.body], bin.env).err).toContain('RECHECK: not eligible (a changed product path is binary'); }); test('base context moved next to a reviewed hunk plus a product edit elsewhere is not eligible (red-team p1 repro)', () => { From 955eb9ce96ecbc8165c024f7a79bec1081799e61 Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 07:02:10 +0000 Subject: [PATCH 5/7] docs: J1 re-check eligibility names every refusal (review fixes) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- changelog.d/gsta258-l4-ready-receipts.md | 2 +- docs/ship-policy.md | 3 ++- docs/troubleshooting.md | 7 ++++--- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/changelog.d/gsta258-l4-ready-receipts.md b/changelog.d/gsta258-l4-ready-receipts.md index 2b8b4845d..49f3320a6 100644 --- a/changelog.d/gsta258-l4-ready-receipts.md +++ b/changelog.d/gsta258-l4-ready-receipts.md @@ -4,7 +4,7 @@ - **`gstack-review-scope summary` binds by finding id.** A row finding carries `ledger_id` (`/`), each fix commit names what it fixed in a `Review-Finding: reader=` trailer, rows that record their head dedupe by kind, pass, head and finding ids, and a late defer counts once. Rows and findings without an id keep the path:line pairing. - **What can't bind is named, never a silent zero.** A row with no start token or readable time, a `pN` ledger with no review row, a `ledger_id` no ledger holds and a trailer naming nothing each print `REVIEW_SCOPE_UNBOUND: : `, and the line and receipt carry `unbound=N`. - **The native reader's own verdict line no longer collects as a blocker.** "There are three low-severity findings, all FIXABLE, and none blocks." (#3901) collected as CRITICAL beside the findings it summarized; it now collects at FIXABLE. A line that says something blocks, names a file:line or leads with FIXABLE still collects as CRITICAL. A finding's `Severity: Low. Blocks: no` after a closing quote (`… its base." Severity: Low`) now counts as its own field: on ggstack#71 that one quote made one of four identical findings CRITICAL. -- **`gstack-ship-receipt carry` offers a scoped re-check (J1).** On `RECEIPT_CARRY_PRODUCT_CHANGED` with linear history against the reviewed base and a range-diff (plus the diff of the changed product paths) of at most 400 lines, carry prints `RECHECK: eligible … packet=`. One reader on the packet and `carry --recheck ` rebind the receipt, unless HEAD needs a deeper review depth or another risk class. Rebased branches, retargeted stacks and anything over the cap still need /ship. +- **`gstack-ship-receipt carry` offers a scoped re-check (J1).** On `RECEIPT_CARRY_PRODUCT_CHANGED` with linear history against the reviewed base and a range-diff (plus the diff of the changed product paths) of at most 400 lines, carry prints `RECHECK: eligible … packet=`. One reader on the packet and `carry --recheck ` rebind the receipt, unless HEAD needs a deeper review depth or another risk class. Rebased branches, retargeted stacks, receipts with no recorded review depth, more than 20 changed product paths, binary paths, moved base context and packets over 400 lines or 64 KB still need /ship. - **Every /ship run's receipt is kept.** When `write` or `carry` replaces a different receipt, the old one's identifiers and counters move to an append-only `gstack-receipt-runs` block in the PR body (newest 30). - **`gstack-ship-queue ready` prints the whole READY.** It adds the receipt's `REVIEW_LOOP:` line, a `TRIAL_MERGES:` line for the lane to fill and an `Attention:` stub that opens with `reviewed by ` on risk-class PRs and `self-audited` on every other PR. docs/unattended.md now asks for the cross-family trail read only on risk-class PRs. - **The `GSTACK_RESULT:` line names the full head** (`status=complete head= run=`), so a leftover line from an earlier ship can't pass for a new head (GST-1179). `parseResult` reads the optional token. diff --git a/docs/ship-policy.md b/docs/ship-policy.md index ec6a2b47c..7a2277b79 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -1593,7 +1593,8 @@ until one fix-check reviewer clears the packet; a working tree with uncommitted changes is [`RECEIPT_CARRY_REFUSED`](troubleshooting.md#receipt-carry-refused). A `RECEIPT_CARRY_PRODUCT_CHANGED` on linear history against the reviewed base whose range-diff, plus the diff of the changed product paths, is at most 400 -lines also prints `RECHECK: eligible … packet=`: one reader on that +lines and 64 KB, with no binary path, no moved base context and a receipt that +records its review depth, also prints `RECHECK: eligible … packet=`: one reader on that packet and `carry --recheck ` rebind the receipt instead of a full second /ship, unless HEAD needs a deeper review depth or another risk class ([scoped re-check](troubleshooting.md#receipt-carry-recheck), J1). diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 799443897..418c5e456 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -6035,9 +6035,10 @@ against the current one is at most 400 lines (`RECHECK_RANGE_DIFF_CAP` in `lib/ship-recheck.ts`), carry also prints `RECHECK: eligible range_diff_lines=N range_diff_sha256= packet=`: one reader on that range-diff can clear it instead of a full second /ship. A rebased branch -(`RECEIPT_CARRY_TREE_MISMATCH`), a retargeted stack, an empty or unreadable -range-diff and anything over the cap print `RECHECK: not eligible ()` -instead. +(`RECEIPT_CARRY_TREE_MISMATCH`), a retargeted stack, a receipt with no +`review_depth`, more than 20 changed product paths, a binary path, base +context moved next to reviewed hunks, an empty or unreadable range-diff and a +packet over 400 lines or 64 KB print `RECHECK: not eligible ()` instead. **Fix.** Dispatch one reader on the packet (`.git/gstack/recheck-.md`; `--dry-run` writes none). When it comes back clean, write From 68f34a6def32292e7d3e77db75588b89ec8eaf64 Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 07:12:47 +0000 Subject: [PATCH 6/7] test: explicit timeouts on the re-check and result-head CLI tests (each spawns several git and bun runs; the carry tests ran past bun's 5 s default locally) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- test/ship-receipt-result-head.test.ts | 2 +- test/ship-recheck.test.ts | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/ship-receipt-result-head.test.ts b/test/ship-receipt-result-head.test.ts index 81adfc001..fbe211058 100644 --- a/test/ship-receipt-result-head.test.ts +++ b/test/ship-receipt-result-head.test.ts @@ -25,7 +25,7 @@ describe('GSTACK_RESULT head= token (GST-1179)', () => { expect(line).toBe(`GSTACK_RESULT: skill=ship status=complete head=${head} run=${out}`); expect(head).toMatch(/^[0-9a-f]{40}$/); expect(parseResult(r.out)).toEqual({ skill: 'ship', status: 'complete', head, run: out }); - }); + }, 30_000); test('a line without head= still parses, with no head', () => { expect(parseResult('GSTACK_RESULT: skill=pregate status=complete run=.gstack/tmp/pregate.json')).toEqual({ skill: 'pregate', status: 'complete', run: '.gstack/tmp/pregate.json' }); diff --git a/test/ship-recheck.test.ts b/test/ship-recheck.test.ts index 1fcaab7eb..487a61486 100644 --- a/test/ship-recheck.test.ts +++ b/test/ship-recheck.test.ts @@ -105,7 +105,7 @@ describe('carry with a scoped re-check (CLI)', () => { commit({ 'src/app.ts': 'export const a = 4;\n' }, 'fix: a=4'); const bad = runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--recheck', stale], env); expect([bad.status, bad.out.trim().split('\n')[0]]).toEqual([EXIT.refused, 'CARRY: refused (RECEIPT_CARRY_REFUSED)']); - }); + }, 60_000); test('a clean re-check cannot carry across a changed risk class', () => { const { repo, from, body, env, commit } = reviewed(); @@ -120,7 +120,7 @@ describe('carry with a scoped re-check (CLI)', () => { expect(r.status).toBe(EXIT.refused); expect(r.err).toMatch(/risk class|review depth/); expect(before.risk ?? []).not.toContain('money'); - }); + }, 60_000); test('a legacy receipt, more changed paths than the walk names, and moved base context print RECHECK: not eligible', () => { const legacy = reviewed(); @@ -136,7 +136,7 @@ describe('carry with a scoped re-check (CLI)', () => { const bin = reviewed({ 'src/app.wasm': '\0asm\x01\0\0\0' }); bin.commit({ 'src/app.wasm': '\0asm\x01\0\0\0' + 'x'.repeat(4096) + '\0' }, 'fix: new wasm'); expect(runBin(bin.repo, 'gstack-ship-receipt', ['carry', '--body', bin.body], bin.env).err).toContain('RECHECK: not eligible (a changed product path is binary'); - }); + }, 90_000); test('base context moved next to a reviewed hunk plus a product edit elsewhere is not eligible (red-team p1 repro)', () => { const lines = (n: number, edits: Record = {}) => Array.from({ length: 10 }, (_, i) => edits[i + 1] ?? `line ${i + 1}`).join('\n') + '\n'; @@ -162,5 +162,5 @@ describe('carry with a scoped re-check (CLI)', () => { const r = runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body], env); expect(r.status).toBe(EXIT.refused); expect(r.err).toContain('RECHECK: not eligible (base context also moved'); - }); + }, 60_000); }); From e15ba87d0914b8bc46e1f2a84d6e4928451c4e29 Mon Sep 17 00:00:00 2001 From: garrytan Date: Tue, 3 Nov 2026 15:56:25 +0000 Subject: [PATCH 7/7] ship: trim 11 skeleton bytes so the merged tree fits the 61,420 cap (GSTA-258 L4) The rule 19 merge of main 1ccab0d8 put #3929's push-guards wording (61,418 on main) together with J1's head= in the GSTACK_RESULT line, measuring 61,429. Three rewordings beside that line bring the skeleton back to 61,418; the branch ledger entry drops by 11 and the ship goldens are regenerated. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- ship/SKILL.md | 6 +++--- ship/SKILL.md.tmpl | 6 +++--- test/fixtures/golden/agy-ship-SKILL.md | 6 +++--- test/fixtures/golden/claude-ship-SKILL.md | 6 +++--- test/fixtures/golden/codex-ship-SKILL.md | 6 +++--- test/fixtures/golden/factory-ship-SKILL.md | 6 +++--- test/size-ledger/ship/gsta258-l4-ready-receipts.json | 4 ++-- 7 files changed, 20 insertions(+), 20 deletions(-) diff --git a/ship/SKILL.md b/ship/SKILL.md index 09fe304dd..689c4ae68 100644 --- a/ship/SKILL.md +++ b/ship/SKILL.md @@ -955,9 +955,9 @@ The last line printed is the result line a parent greps (docs/unattended.md): ~/.claude/skills/gstack/bin/gstack-ship-receipt result --pr # handoff: --body .gstack/tmp/ship-receipt.md ``` -It binds the receipt to HEAD, writes the run record and prints `GSTACK_RESULT: skill=ship -status=complete head= run=`; `QUEUE_RECEIPT_STALE` means HEAD moved after the receipt (rewrite, -repost, rerun). A run that STOPs earlier ends with `GSTACK_RESULT: skill=ship status=refused run=-`. +It binds the receipt to HEAD, writes the run record, prints `GSTACK_RESULT: skill=ship +status=complete head= run=`; `QUEUE_RECEIPT_STALE`: HEAD moved past the receipt (rewrite, +repost, rerun). A run that STOPs early ends with `GSTACK_RESULT: skill=ship status=refused run=-`. --- diff --git a/ship/SKILL.md.tmpl b/ship/SKILL.md.tmpl index 99f9c66c0..87342a349 100644 --- a/ship/SKILL.md.tmpl +++ b/ship/SKILL.md.tmpl @@ -674,9 +674,9 @@ The last line printed is the result line a parent greps (docs/unattended.md): ~/.claude/skills/gstack/bin/gstack-ship-receipt result --pr # handoff: --body .gstack/tmp/ship-receipt.md ``` -It binds the receipt to HEAD, writes the run record and prints `GSTACK_RESULT: skill=ship -status=complete head= run=`; `QUEUE_RECEIPT_STALE` means HEAD moved after the receipt (rewrite, -repost, rerun). A run that STOPs earlier ends with `GSTACK_RESULT: skill=ship status=refused run=-`. +It binds the receipt to HEAD, writes the run record, prints `GSTACK_RESULT: skill=ship +status=complete head= run=`; `QUEUE_RECEIPT_STALE`: HEAD moved past the receipt (rewrite, +repost, rerun). A run that STOPs early ends with `GSTACK_RESULT: skill=ship status=refused run=-`. --- diff --git a/test/fixtures/golden/agy-ship-SKILL.md b/test/fixtures/golden/agy-ship-SKILL.md index 80994cff9..8ec412507 100644 --- a/test/fixtures/golden/agy-ship-SKILL.md +++ b/test/fixtures/golden/agy-ship-SKILL.md @@ -956,9 +956,9 @@ The last line printed is the result line a parent greps (docs/unattended.md): ~/.gemini/antigravity-cli/skills/gstack/bin/gstack-ship-receipt result --pr # handoff: --body .gstack/tmp/ship-receipt.md ``` -It binds the receipt to HEAD, writes the run record and prints `GSTACK_RESULT: skill=ship -status=complete head= run=`; `QUEUE_RECEIPT_STALE` means HEAD moved after the receipt (rewrite, -repost, rerun). A run that STOPs earlier ends with `GSTACK_RESULT: skill=ship status=refused run=-`. +It binds the receipt to HEAD, writes the run record, prints `GSTACK_RESULT: skill=ship +status=complete head= run=`; `QUEUE_RECEIPT_STALE`: HEAD moved past the receipt (rewrite, +repost, rerun). A run that STOPs early ends with `GSTACK_RESULT: skill=ship status=refused run=-`. --- diff --git a/test/fixtures/golden/claude-ship-SKILL.md b/test/fixtures/golden/claude-ship-SKILL.md index 09fe304dd..689c4ae68 100644 --- a/test/fixtures/golden/claude-ship-SKILL.md +++ b/test/fixtures/golden/claude-ship-SKILL.md @@ -955,9 +955,9 @@ The last line printed is the result line a parent greps (docs/unattended.md): ~/.claude/skills/gstack/bin/gstack-ship-receipt result --pr # handoff: --body .gstack/tmp/ship-receipt.md ``` -It binds the receipt to HEAD, writes the run record and prints `GSTACK_RESULT: skill=ship -status=complete head= run=`; `QUEUE_RECEIPT_STALE` means HEAD moved after the receipt (rewrite, -repost, rerun). A run that STOPs earlier ends with `GSTACK_RESULT: skill=ship status=refused run=-`. +It binds the receipt to HEAD, writes the run record, prints `GSTACK_RESULT: skill=ship +status=complete head= run=`; `QUEUE_RECEIPT_STALE`: HEAD moved past the receipt (rewrite, +repost, rerun). A run that STOPs early ends with `GSTACK_RESULT: skill=ship status=refused run=-`. --- diff --git a/test/fixtures/golden/codex-ship-SKILL.md b/test/fixtures/golden/codex-ship-SKILL.md index c6ac42488..5e49d6e73 100644 --- a/test/fixtures/golden/codex-ship-SKILL.md +++ b/test/fixtures/golden/codex-ship-SKILL.md @@ -965,9 +965,9 @@ The last line printed is the result line a parent greps (docs/unattended.md): $GSTACK_ROOT/bin/gstack-ship-receipt result --pr # handoff: --body .gstack/tmp/ship-receipt.md ``` -It binds the receipt to HEAD, writes the run record and prints `GSTACK_RESULT: skill=ship -status=complete head= run=`; `QUEUE_RECEIPT_STALE` means HEAD moved after the receipt (rewrite, -repost, rerun). A run that STOPs earlier ends with `GSTACK_RESULT: skill=ship status=refused run=-`. +It binds the receipt to HEAD, writes the run record, prints `GSTACK_RESULT: skill=ship +status=complete head= run=`; `QUEUE_RECEIPT_STALE`: HEAD moved past the receipt (rewrite, +repost, rerun). A run that STOPs early ends with `GSTACK_RESULT: skill=ship status=refused run=-`. --- diff --git a/test/fixtures/golden/factory-ship-SKILL.md b/test/fixtures/golden/factory-ship-SKILL.md index db99f0c4b..39451441b 100644 --- a/test/fixtures/golden/factory-ship-SKILL.md +++ b/test/fixtures/golden/factory-ship-SKILL.md @@ -967,9 +967,9 @@ The last line printed is the result line a parent greps (docs/unattended.md): $GSTACK_ROOT/bin/gstack-ship-receipt result --pr # handoff: --body .gstack/tmp/ship-receipt.md ``` -It binds the receipt to HEAD, writes the run record and prints `GSTACK_RESULT: skill=ship -status=complete head= run=`; `QUEUE_RECEIPT_STALE` means HEAD moved after the receipt (rewrite, -repost, rerun). A run that STOPs earlier ends with `GSTACK_RESULT: skill=ship status=refused run=-`. +It binds the receipt to HEAD, writes the run record, prints `GSTACK_RESULT: skill=ship +status=complete head= run=`; `QUEUE_RECEIPT_STALE`: HEAD moved past the receipt (rewrite, +repost, rerun). A run that STOPs early ends with `GSTACK_RESULT: skill=ship status=refused run=-`. --- diff --git a/test/size-ledger/ship/gsta258-l4-ready-receipts.json b/test/size-ledger/ship/gsta258-l4-ready-receipts.json index 4a2a9e77c..eba7c110f 100644 --- a/test/size-ledger/ship/gsta258-l4-ready-receipts.json +++ b/test/size-ledger/ship/gsta258-l4-ready-receipts.json @@ -1,4 +1,4 @@ { - "bytes": 412, - "why": "GSTA-258 L4: review fixes carry ledger_id and a Review-Finding trailer, late defers log at Step 11 (GST-1168), the result line names head= (GST-1179)" + "bytes": 401, + "why": "GSTA-258 L4: review fixes carry ledger_id and a Review-Finding trailer, late defers log at Step 11 (GST-1168), the result line names head= (GST-1179); then -11 bytes (#3926): trim the ship skeleton's GSTACK_RESULT paragraph by 11 bytes after the rule 19 merge of main 1ccab0d8 summed #3929's push-guards line and J1's head=; skeleton back to 61,418 under the fixed 61,420 cap" }