From 5225e35cc60312ae1b675704e7f2ffdac1ccf40e Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Sun, 1 Nov 2026 13:26:59 -0800 Subject: [PATCH] v1.92.42.0 fix(ship): notes-only review passes bind; carry handles large diffs; scoped carry route (GST-430, GST-448, GST-442, GST-444) (#3694) * fix(ship): base merges, adjacent base edits, test-only and audit-requested fixes no longer force a second review loop (GST-407, GST-384, GST-391) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * ship driver: trim two phrases for headroom with #3662 Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * ship driver: keep the Next steps heading phrase the handoff check reads; trim elsewhere Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test: regenerate ship goldens Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: pre-landing review fixes comparePath returns same only when every hunk sits where the base change puts it (patch-ids ignore positions) and treats a file-mode change as changed; the scoped-review packet says when a fence is truncated; --docs-result must be the posted audit's result; the --context-reviewed record goes beside the packet, outside the worktree. Negative controls for each. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: pass-2 review fixes A test edit routes as tests= only when it adds lines with no gate tag; a removed or changed line or an unreadable test diff goes through the full loop. Diffs skip textconv. A hunkless patch (binary, empty, mode-only) is same when its patch-id matches. Without --reviewed-base test files route as before. finish's packet path and attempt's state directory have fallbacks. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: Step 11 review fixes; a version-only stamp_files edit after review is release metadata (GBRA-251) gateEditedPaths reads removed lines only inside hunks; carry refuses when a path listing fails; docs freshness stops on a failed listing; attempt counts an unreadable state file. releasePaths includes stamp_files paths whose edit is a version swap under stamp_patterns. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: no test edit skips review; guard packet writes; exclusive attempt state A test-only edit with no gate tag gets the scoped fix-check of its test hunks instead of a lanes-only rerun. Packet write failures print a STOP or RECEIPT_CARRY_REFUSED. attempt's allowance uses an exclusive create. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: attempt prints gstack's install command; carry checks the product diff outside context paths The attempt STOP no longer echoes the audit child's fix text. A scoped-review carry now requires the product patch-id without the context paths to match at both heads. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: oversized packets take the full loop; exact --docs-requested match; name-only test globs only in test dirs A scoped-review packet section over 200 lines sends that path to the full loop instead of a truncated packet. The docs-result path match is exact. test_*.py, *Test.java and similar count as test code only under a test directory. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs(unattended): carry's scoped-review refusal and what it keeps Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs: carry's scoped-review route in ship-policy, review-policy and troubleshooting (docs-a-1) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test(windows): curate ship-rereview-triggers out of the Windows lane like ship-release (run 41193336209) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(review-log): record a notes-only /ship pass clean; name the cause and repair when a pass's tree moved (GST-430) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(receipt): carry and write handle PRs with thousands of files; tree receipt fails closed on a failed diff (GST-448) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(ship): preflight routes RECEIPT_CARRY_SCOPED_REVIEW to the scoped review; carry --dry-run writes no packet (GST-442) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs(ship): Step 1.4 and Step 16 route the scoped-review refusal and the --docs-requested fix; trims keep the cap (GST-442, GST-444) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: /ship review pass 1: notes-only floor, scoped carry order, named temp and diff refusals - A notes-only pass is recorded clean only when every finding is fixed, or noted or deferred below the blocking floor (never CRITICAL or blocking), by its logger-stamped action, and the findings account for the claimed issue and critical counts (GST-430). - The preflight scoped carry route commits and pushes the base-merge fix before writing the packet (GST-442). - patchId refuses by name on an unwritable TMPDIR; the tree receipt names why its diff failed (GST-448). - Step 16 stage 3 spells out --docs-requested with its --docs-result (GST-444). Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test: pin the Step 16 --docs-requested route with its --docs-result Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: a claimed clean review row with a logger-stamped open gate finding reads UNVERIFIED (red-team, pass 1) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: Step 11 findings: gate rows from --findings are stamped, one open rule, stale patch-id dirs swept - deriveFromFindings stamps gate rows with the session kind before deriving status, so a findings file cannot dispose of a gate finding the unattended logger forces open (outside-voice#1). - reviewFreshness reads a claimed row's findings with isOpenByAction, the rule the notes-only rewrite uses; noted and deferred clear a finding only when it is marked blocking: false and is not CRITICAL. - patchId sweeps gstack-patch-id-* directories over an hour old that a killed carry left in TMPDIR. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: Step 11 round 2: stamped-open gates win over status and disposition; resolved gate, skipped and below-floor actions stay CURRENT - isOpenFinding treats action 'open' (the logger's forced-open stamp) as open before a row's status or disposition. - reviewFreshness flags a claimed clean row only for an 'open' action or a CRITICAL or blocking deferral or note, so interactive kept and restored gates, skipped advisories and INFORMATIONAL deferrals read CURRENT as before. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: carry dry run offers the scoped route last; patch-id diff under the git dir; changelog matches the review-log floor Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: a claimed clean review row with any unresolved non-gate CRITICAL finding reads UNVERIFIED (pass 2 red-team) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs: carry dry-run scoped route, patch-id directory, tree-receipt refusal (docs-d-1 audit) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(ship): Step 3 names merge conflicts in its STOP sentence again (ship-workflow-clarity pin) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(tree-receipt): keep the gated-to-current diff at the file helper timeout (60 s); pregate literals check Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * test(carry): large-diff fixture sets core.longpaths so Git for Windows can add its long receipt paths (windows-free-shard 1, run 41210080055) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: restore bin/gstack-ship-receipt after the main merge The merge commit b10e2db wrote a truncated copy of the file. This is the three-way merge of both sides with the refused() union: main's codes plus RECEIPT_CARRY_SCOPED_REVIEW. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * ship: trim a Step 3 sentence so the skeleton fits maxSkeletonBytes with train 77 Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * ship: keep the no-edit rule in the trimmed Step 3 sentence; regenerate the ship goldens Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- CHANGELOG.md | 22 +++ VERSION | 2 +- agents-digest/gstack-AGENTS.md | 2 +- bin/gstack-ship-receipt | 63 ++++++-- docs/ship-policy.md | 24 ++- docs/troubleshooting.md | 34 ++++- docs/unattended.md | 5 +- lib/headless-artifacts.ts | 9 +- lib/result-codes-release.ts | 2 +- lib/review-evidence.ts | 26 +++- lib/ship-preflight.ts | 4 + lib/tree-receipt.ts | 7 +- package.json | 2 +- ship/SKILL.md | 26 ++-- ship/SKILL.md.tmpl | 26 ++-- test/fixtures/golden/agy-ship-SKILL.md | 26 ++-- test/fixtures/golden/claude-ship-SKILL.md | 26 ++-- test/fixtures/golden/codex-ship-SKILL.md | 26 ++-- test/fixtures/golden/factory-ship-SKILL.md | 26 ++-- test/review-notes-only-pass.test.ts | 144 ++++++++++++++++++ test/ship-control-flow.test.ts | 12 +- test/ship-receipt-carry-large-diff.test.ts | 90 +++++++++++ test/ship-rereview-triggers.test.ts | 43 +++++- .../ship/gst-430-notes-only-pass.json | 4 + test/train-gate.test.ts | 17 +++ 25 files changed, 556 insertions(+), 112 deletions(-) create mode 100644 test/review-notes-only-pass.test.ts create mode 100644 test/ship-receipt-carry-large-diff.test.ts create mode 100644 test/size-ledger/ship/gst-430-notes-only-pass.json diff --git a/CHANGELOG.md b/CHANGELOG.md index abd27e4eb..c44245e20 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,27 @@ # Changelog +## [1.92.42.0] - 2026-11-01 + +**A /ship review pass that only left notes now binds at Step 11.5.** A pass whose findings were all fixed, deferred or noted has no open defect, but a driver could still log it as `issues_found`. Step 11.5 then refused it, and the only way out was an extra rebind pass (CAPY-2, qm#2837). When the tree changes during a pass, the refusal now names the start and end trees and the cheapest repair (GST-430). + +**Receipt carry and write handle results PRs with thousands of files (GST-448).** A PR that adds about 8,300 run receipts (harness-evals#147) refused every carry with `git could not compute HEAD's product patch-id: git diff … failed`, and every base move cost it about 3 hours of /ship. The name-only listing ran past a 1 MB output buffer. + +### Fixed +- **`gstack-ship-receipt carry` and `write`** read git's path listings with a 256 MB buffer instead of the 1 MB default, so a large PR's `product_patch_id` is recorded and carried. The product patch-id streams `git diff` through a file under the git directory (`.git/gstack-patch-id`) into `git patch-id --verbatim` and no longer holds the diff in memory; a directory a killed run left is removed after an hour. The per-path walk compares only touched paths that changed at either head or either base, so a 6,000-path PR refuses or carries in seconds. +- **/ship Step 1's carry check takes the scoped-review route (GST-442).** When the base edited context next to a reviewed hunk, `gstack-ship-preflight` now prints `NEXT: carry route`: fix what the base merge broke, commit and push, write the packet, one fix-check, then `carry --scoped-review`, instead of running the full loop. `carry --dry-run` writes no packet and offers this route only when none of carry's other checks would refuse. +- **Step 16 names finish's scoped-review STOP and the `--docs-requested` fix (GST-444).** Stage 2 sends a STOP that names a scoped-review packet to its fix-check and `--context-reviewed` rerun, and stage 3 offers a fix the audit's result named before a re-audit or a named risk. The RECEIPT_CARRY_PRODUCT_CHANGED fix text points to the scoped-review refusal. Trims in Steps 3, 12 and 16 keep ship/SKILL.md under its cap. +- **`gstack-tree-receipt`** fails closed (`EVIDENCE_IDENTITY_UNKNOWN`) when git cannot produce the diff between the gated and current heads, and names git's reason. Before, it read a failed diff as no changes. +- **`gstack-review-log`** records a completed `review` or `review-fix-check` row claimed as `issues_found` as `clean` with zero counts when every finding's logger-stamped action is `fixed` or `auto-fixed`, or `deferred` or `noted` on a finding marked `blocking: false` that is not CRITICAL, and the findings account for the claimed issue and critical counts. A skipped finding, a gate finding left open, a CRITICAL or blocking note, a finding with no action, or an incomplete pass keeps the claimed status. +- **Findings files** (`--findings`) count `auto-fixed` as resolved, the same as `fixed`, and `deferred` or `noted` only below the floor. Their gate rows are stamped like inline ones, so an unattended run's gate stays open whatever its status or disposition says. +- **`review_freshness`** reads a claimed clean row as UNVERIFIED when a finding is a gate left `open`, a CRITICAL or blocking deferral or note, or any other unresolved CRITICAL finding. +- **`review_freshness`** for a pass whose tree moved now reads `content changed during review: the tree moved from to while ran …`, followed by the repair: keep the checkout frozen until every reviewer returns, then rerun on the frozen tree (docs/troubleshooting.md#review-content-changed). + +### Tests +- `test/ship-rereview-triggers.test.ts`: preflight on a founderstar-style adjacent edit prints the scoped-review `NEXT: carry route`, and its dry run writes no packet. `test/ship-control-flow.test.ts`: Step 1.4 and Step 16 stages 2 and 3 name the routes. +- `test/ship-receipt-carry-large-diff.test.ts`: a PR that adds 6,000 receipts under a long directory name writes a receipt with its product patch-id and carries across a base merge. Negative control: one rescored receipt still refuses with `RECEIPT_CARRY_PRODUCT_CHANGED`. +- `test/review-notes-only-pass.test.ts`: a pass with three notes is recorded clean and reads CURRENT. Negative controls: a skipped finding, an open gate finding (also under a caller's resolved status), a CRITICAL or blocking note, a note with no `blocking` field, counts the findings do not cover, a finding with no action, and an incomplete pass keep `issues_found`. A claimed clean row with an open gate, a CRITICAL deferral, or a skipped or action-less CRITICAL finding is not CURRENT, while kept and restored gates, skipped advisories and an INFORMATIONAL deferral stay CURRENT. A `--findings` gate row stays open in an unattended run. Also checks the reason a pass gets when its tree moved. +- `test/ship-receipt-carry-large-diff.test.ts` also: the patch-id file lives under the git directory (an unwritable TMPDIR still carries; an unwritable spot refuses by name), and a stale directory is swept. `test/train-gate.test.ts`: a diff git cannot produce refuses the tree receipt with git's reason. `test/ship-rereview-triggers.test.ts`: a dry run the receipt would refuse anyway names that refusal. + ## [1.92.41.0] - 2026-11-01 **The Ubicloud runner works on macOS and on a project with no VMs (#3680, #3679, #3678).** On a Mac, `gstack-ubi pack` failed, so every `sync`, `run` and `verify` broke after the VM was already created, and `gstack-ubi usage` was a syntax error that also switched off the vCPU quota check. On any machine, an empty Ubicloud project made `reap` and every `up` die. All three were reported by Jared. diff --git a/VERSION b/VERSION index f0625b8df..bb114e4c2 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.92.41.0 +1.92.42.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index 72f251a5b..0f21b65e0 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.92.41.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.92.42.0 — regenerate/re-copy after upgrading gstack Behavioral rules from gstack (https://github.com/garrytan/gstack), compressed for agent hosts without a full skill install. The full skills add workflows, diff --git a/bin/gstack-ship-receipt b/bin/gstack-ship-receipt index 1079fd9df..b2d902b65 100755 --- a/bin/gstack-ship-receipt +++ b/bin/gstack-ship-receipt @@ -32,6 +32,7 @@ */ import { spawnSync } from 'node:child_process'; import * as fs from 'node:fs'; +import * as os from 'node:os'; import * as path from 'node:path'; import { EXIT, exitTableHelp, type ShipReceipt } from '../lib/headless-artifacts'; import { resultLine, resultLineWithDocs } from '../lib/result-codes'; @@ -119,7 +120,7 @@ function jsonArg(raw: string | undefined): Record | undefined { return parsed; } function git(args: string[], cwd?: string): string | undefined { - const r = spawnSync('git', args, { cwd, encoding: 'utf8', timeout: 30_000 }); + const r = spawnSync('git', args, { cwd, encoding: 'utf8', timeout: 30_000, maxBuffer: 256 * 1024 * 1024 }); return r.status === 0 ? r.stdout.trim() : undefined; } function gh(args: string[]): { ok: boolean; out: string; err: string } { @@ -399,12 +400,37 @@ function writeRunRecord(a: { receipt: ShipReceipt; head: string; pr: string | un */ function patchId(repoRoot: string, from: string, to: string, pathspec: string[]): string | { error: string } { const firstLine = (s: string | null | undefined, fallback: string) => (s ?? '').trim().split('\n')[0]?.slice(0, 200) || fallback; - const diff = spawnSync('git', ['diff', '--no-color', '--no-ext-diff', '--full-index', from, to, '--', ...pathspec], { cwd: repoRoot, encoding: 'utf8', timeout: 120_000, maxBuffer: 256 * 1024 * 1024 }); - if (diff.status !== 0) return { error: `git diff ${from.slice(0, 12)} ${to.slice(0, 12)}: ${firstLine(diff.stderr, diff.signal ? `killed by ${diff.signal}` : `exit ${diff.status}`)}` }; - if (!diff.stdout.trim()) return ''; - const id = spawnSync('git', ['patch-id', '--verbatim'], { cwd: repoRoot, input: diff.stdout, encoding: 'utf8', timeout: 120_000, maxBuffer: 64 * 1024 * 1024 }); - if (id.status !== 0) return { error: `git patch-id --verbatim: ${firstLine(id.stderr, id.signal ? `killed by ${id.signal}` : `exit ${id.status}`)}` }; - return id.stdout.trim().split(/\s+/)[0] ?? ''; + const root = path.resolve(repoRoot, git(['rev-parse', '--git-path', 'gstack-patch-id'], repoRoot)?.trim() || '.git/gstack-patch-id'); + sweepStalePatchDirs(root); + let dir: string; + try { fs.mkdirSync(root, { recursive: true }); dir = fs.mkdtempSync(path.join(root, 'run-')); } catch (e) { return { error: `cannot create a temp file under ${root} (${firstLine((e as Error).message, 'mkdtemp failed')}); make that directory writable` }; } + const file = path.join(dir, 'diff.patch'); + let fd: number | undefined; + try { + const diff = spawnSync('git', ['diff', '--no-color', '--no-ext-diff', '--full-index', `--output=${file}`, from, to, '--', ...pathspec], { cwd: repoRoot, encoding: 'utf8', timeout: 120_000 }); + if (diff.status !== 0) return { error: `git diff ${from.slice(0, 12)} ${to.slice(0, 12)}: ${firstLine(diff.stderr, diff.signal ? `killed by ${diff.signal}` : `exit ${diff.status}`)}` }; + fd = fs.openSync(file, 'r'); + const id = spawnSync('git', ['patch-id', '--verbatim'], { cwd: repoRoot, stdio: [fd, 'pipe', 'pipe'], encoding: 'utf8', timeout: 120_000 }); + if (id.status !== 0) return { error: `git patch-id --verbatim: ${firstLine(id.stderr, id.signal ? `killed by ${id.signal}` : `exit ${id.status}`)}` }; + return id.stdout.trim().split(/\s+/)[0] ?? ''; + } finally { + if (fd !== undefined) fs.closeSync(fd); + fs.rmSync(dir, { recursive: true, force: true }); + } +} + +/** A carry killed mid patch-id (preflight's timeout) skips patchId's `finally`; its product diff is removed by the next run once an hour old. */ +let swept = false; +function sweepStalePatchDirs(root: string): void { + if (swept) return; + swept = true; + const cutoff = Date.now() - 60 * 60 * 1000; + let names: string[] = []; + try { names = fs.readdirSync(root).filter(n => n.startsWith('run-')); } catch { return; } + for (const n of names) { + const p = path.join(root, n); + try { if (fs.statSync(p).mtimeMs < cutoff) fs.rmSync(p, { recursive: true, force: true }); } catch { /* another run removed it */ } + } } /** `git patch-id --verbatim` exists from git 2.39 (docs/ship-policy.md#receipt-carry); an older git is a named refusal, never a silent fallback to the whitespace-stripping default. */ @@ -581,6 +607,7 @@ function cmdCarry(rest: string[], json: boolean): number { const reviewedBase = receipt.base.replace(/^origin\//, ''); const retarget = reviewedBase !== base.replace(/^origin\//, ''); let plan: ReturnType; + let scopedDryRun: 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' }; @@ -620,10 +647,14 @@ function cmdCarry(rest: string[], json: boolean): number { else if (productFrom !== productHead && !retarget) { // planCarry names at most CARRY_NAMED_PATHS differing paths, so the walk stops there, and it reads only the paths the // branch itself touched against the base at either head: a base merge brings in every path the base changed, none of - // which can differ between the two base diffs. A path whose only difference is base context next to unchanged hunks + // which can differ between the two base diffs. A touched path with the same blob at both heads and at both bases has + // the same patch, so only paths that moved on one side are compared (GST-448: 6000 touched paths). A path whose only difference is base context next to unchanged hunks // (lib/ship-patch.ts) needs a scoped review instead of the loop. const sides = { reviewedBase: fromBase, reviewed: from, currentBase: headBase, current: head }; - for (const p of [...touched].sort()) { + const moved = [touchedPaths(repoRoot, from, head), touchedPaths(repoRoot, fromBase, headBase)]; + if (moved.some(l => !l)) return refused('RECEIPT_CARRY_REFUSED', `git could not list the paths the branch touched against ${base}; run /ship on ${head.slice(0, 12)}`); + const [movedHead, movedBase] = moved as Set[]; + for (const p of [...touched].filter(p => movedHead.has(p) || movedBase.has(p)).sort()) { if (changedProduct.length >= CARRY_NAMED_PATHS) break; const cls = changed.find(c => c.path === p)?.class ?? classifyCarryPath(p, rules.extra); if (cls !== 'product') continue; @@ -644,7 +675,10 @@ function cmdCarry(rest: string[], json: boolean): number { if (packetBody?.over.length) changedProduct.push(...packetBody.over.map(p => `${p} (too large for the scoped review: over ${PACKET_SECTION_CAP} lines in a section)`)); if (packetBody && !changedProduct.length) { const record = flag(rest, '--scoped-review'); - if (!record) { + if (!record && rest.includes('--dry-run')) { + scopedDryRun = `base context moved next to the reviewed hunks in ${contextPaths.join(', ')}: carry without --dry-run writes the scoped-review packet for one fix-check reviewer, then carry --scoped-review ; nothing written`; + scopedReview = `base context moved (scoped review pending: ${contextPaths.join(', ')})`; + } else if (!record) { const gitPath = git(['rev-parse', '--git-path', `gstack/scoped-review-${head.slice(0, 12)}.md`], repoRoot); const packet = path.resolve(repoRoot, gitPath ?? `.git/gstack/scoped-review-${head.slice(0, 12)}.md`); try { @@ -655,9 +689,11 @@ function cmdCarry(rest: string[], json: boolean): number { } return refused('RECEIPT_CARRY_SCOPED_REVIEW', `base context moved next to the reviewed hunks in ${contextPaths.join(', ')}: one fix-check reviewer reads ${packet}, then rerun carry with --scoped-review ({"head":"${head}","paths":[…],"reviewer":"","verdict":"clean"}); a blocking finding means /ship on ${head.slice(0, 12)}`); } - const review = readScopedReview(record, { head }, contextPaths); - if (!review.ok) return refused('RECEIPT_CARRY_REFUSED', `--scoped-review: ${review.message}`); - scopedReview = `base context moved (scoped review by ${review.review.reviewer}: ${contextPaths.join(', ')})`; + if (record) { + const review = readScopedReview(record, { head }, contextPaths); + if (!review.ok) return refused('RECEIPT_CARRY_REFUSED', `--scoped-review: ${review.message}`); + scopedReview = `base context moved (scoped review by ${review.review.reviewer}: ${contextPaths.join(', ')})`; + } } const mergeCommits = (git(['rev-list', '--merges', '--count', `${from}..${head}`], repoRoot) ?? '0').trim(); // Evidence for the record, never the decision: a slow range-diff is reported unavailable rather than waited for. @@ -698,6 +734,7 @@ function cmdCarry(rest: string[], json: boolean): number { if (errors.length) return refused('RECEIPT_CARRY_REFUSED', `the carried receipt does not validate: ${errors[0]!.message} at ${errors[0]!.path}`); const newBody = placeReceiptFirst(body, carried); const line = receiptSummaryLine(carried); + if (scopedDryRun) return refused('RECEIPT_CARRY_SCOPED_REVIEW', scopedDryRun); if (rest.includes('--dry-run')) { process.stderr.write(line + '\n'); process.stdout.write(json ? JSON.stringify({ skill: 'ship', status: 'dry-run', head, carry: plan.carry }) + '\n' : `CARRY: would carry from ${from.slice(0, 12)} to ${head.slice(0, 12)} (${plan.carry.reason}); nothing written\n`); diff --git a/docs/ship-policy.md b/docs/ship-policy.md index e7d658833..7cd24136a 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -1345,7 +1345,12 @@ falling back to the whitespace-stripping default, and a `git diff` or patch-ids differ, the refusal names up to `CARRY_NAMED_PATHS` (20) product paths whose patch against the base changed, walking only the paths the branch itself touched at either head (a base merge brings in every path the base -changed, none of which can differ between the two base diffs). Product means every changed path that is not test code (`test/**`, +changed, none of which can differ between the two base diffs), and of those +only the paths that changed between the two heads or the two bases (a path with +the same blob on both sides has the same patch). The product diff streams +through a file under the git directory (`.git/gstack-patch-id`) into +`git patch-id --verbatim`, so a PR with thousands of paths is not held in +memory; a directory a killed run left there is removed after an hour. Product means every changed path that is not test code (`test/**`, `spec/**`, `e2e/**`, `**/*.test.`, `**/*.spec.` and the other `CARRY_TEST_GLOBS` in `lib/ship-receipt.ts`; an `openapi.spec.yaml` is product) or release metadata (`CHANGELOG.md`, `agents-digest/**`, each @@ -1383,7 +1388,11 @@ along the receipt's own history, `rewritten` once any hop was a rebase; `CARRY: carried from to ()` and the `GSTACK_RESULT: skill=ship status=complete` line, and its summary line ends with `carried_from=`. `CARRY: current` means the receipt already names HEAD; -`--dry-run` prints `CARRY: would carry …` and writes nothing. +`--dry-run` prints `CARRY: would carry …` and writes nothing. When the only +difference is base context next to the branch's unchanged hunks and none of +carry's other checks would refuse, `--dry-run` refuses +`RECEIPT_CARRY_SCOPED_REVIEW` without writing the packet; carry without +`--dry-run` writes it. A changed product path, or a product patch-id that differs, is [`RECEIPT_CARRY_PRODUCT_CHANGED`](troubleshooting.md#receipt-carry-product-changed) @@ -1416,7 +1425,12 @@ the carry when the branch has an open PR, in this order, and never reruns the review loop for a carried receipt: 1. `gstack-ship-receipt carry --pr --base --dry-run`. `CARRY: would - carry` is eligibility only; a refusal continues the full ship. + carry` is eligibility only; a refusal continues the full ship, except + `RECEIPT_CARRY_SCOPED_REVIEW`, for which `gstack-ship-preflight` prints + `NEXT: carry route`: fix what the base merge broke, commit and push, run carry + without `--dry-run` to write the packet, have one fix-check reviewer read it, + then carry with `--scoped-review ` and `--pregate`. A blocking + finding continues the full ship. 2. At that pinned HEAD: the CHANGELOG check, the size-cap check the policy declares, and `gstack-pregate` (the remote tests stage when `requires_remote` asks for it). A failing check, or HEAD moving, continues @@ -1706,7 +1720,9 @@ hand edit), and the replacement the current one, so a dependency pin that happens to equal the old version is `changed`; `stamp_patterns` templates count as stamp forms too). With `stamp_files` set, a file outside it is `changed (stamp_files: not in the repo's stamp_files allowlist)` whatever -its diff. Any other byte is a change. +its diff. Any other byte is a change. A diff git cannot produce between the two +heads fails closed with `EVIDENCE_IDENTITY_UNKNOWN` and git's reason; it never +reads as no change. Equivalence is decided per gate. A lane in `stamp_sensitive_gates` always `rerun`s; a verified lane is `reusable` only when the tree is same-modulo-stamps diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 80496ac85..b63323749 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -550,6 +550,25 @@ local `origin/` at ``. **Fix.** Run `git fetch origin ` outside the sandbox and rerun /review if the base has moved. + +### `review_freshness: STALE content changed during review: the tree moved from to while ran ...` / **Review records missing or mismatched** + +**Meaning.** The working tree changed between the pass's `gstack-review-log --start` and +its logged row: an edit, commit or merge landed while reviewers were still reading. The +reviewers may not have seen that change, so the row's `review_binding.state` is `changed`. +Step 11.5 cannot bind it and inserts `9 → 10 → 11 → 11.5`. Logging the same results again +cannot repair it. + +**Fix.** Keep the checkout frozen (no edits, commits, merges or pushes) from a pass's start +until every reviewer in it has returned and its row is logged. If the mid-pass change was +your own fix to that pass's findings, log the pass with `"converged":false` and the fix +as `"action":"fixed"`. `gstack-review-plan` then picks the repeat: one fix-check reading +the touched hunks when every fix is below the blocking floor, and 11.5 accepts that +fix-check record as Step 9's. Rerun Step 11's native pass on the same frozen tree. + +**Expected result.** `gstack-review-read` shows both records at one tree with +`review_freshness` `CURRENT`, and Step 11.5 continues to Step 12. + ### `gstack-wtree: cannot create a temp index under ; set TMPDIR to a writable directory` / `gstack-review-log: cannot create a private object directory under ; no working-tree fingerprint. Set TMPDIR to a writable directory.` @@ -3140,9 +3159,14 @@ or pass `--force-gate ` in `/ship` to run the full gate again. **Meaning.** The bundle (or ledger record) lacks a committed tree id, runtime versions, lockfile hashes or the command hash, so no reuse decision can rest on it. Imported assertions without verification count as unknown too. +`gstack-tree-receipt` also fails with this code when git cannot produce the +diff between the gated and current heads (a missing object, a timeout): the +error names git's reason, and a failed diff is never read as no change. **Fix.** Rebuild the bundle on the gate machine after the lanes ran on a -committed head: `gstack-evidence bundle --out --label ...`. +committed head: `gstack-evidence bundle --out --label ...`. For +the tree receipt, fix what git's reason names (fetch a missing head with +`git fetch origin `), then rerun. ### `EVIDENCE_BUNDLE_INVALID` @@ -5530,7 +5554,9 @@ compare (a receipt written before `head_tree`, whose head is not in this clone), the PR was retargeted to the base after its stack parent landed but the receipt records no `product_patch_id` and `origin/` no longer resolves, so the reviewed range cannot be recomputed, or git could not -compare the two product diffs or read the working tree's status. It also refuses when an owner exception the receipt records +compare the two product diffs (including when carry cannot create its temp +directory under `.git/gstack-patch-id`; the message names it) or read the +working tree's status. It also refuses when an owner exception the receipt records no longer stands, and, for a receipt from a gstack that records review depth, when HEAD needs a deeper review than the receipt records, `--head` is not the checked-out HEAD, or the receipt records `gstack_version` without @@ -5623,6 +5649,10 @@ the tree it is bound to. Finish also stops this way for test code changed since review with no gate edit (`tests=N`): the packet holds those test hunks, and the reviewer checks that each test still tests what it did. A packet that cannot be written (an unwritable `.git`) is a STOP or `RECEIPT_CARRY_REFUSED` naming the path. +`carry --dry-run` reports this code without writing the packet, and only when +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)`. ### `RECEIPT_PROSE_MISMATCH` diff --git a/docs/unattended.md b/docs/unattended.md index be924d334..398b9fd59 100644 --- a/docs/unattended.md +++ b/docs/unattended.md @@ -240,8 +240,9 @@ now names HEAD with a `carry` object holding the `git range-diff` evidence, and counters are kept, because the reviewed product diff is unchanged or its only difference passed a scoped review. `CARRY: current` means nothing moved. `CARRY: refused (RECEIPT_CARRY_SCOPED_REVIEW)` means the base edited context -next to reviewed hunks: one fix-check reviewer reads the packet the refusal -names, and a clean record reruns carry with `--scoped-review ` +next to reviewed hunks. Carry without `--dry-run` writes the packet the +refusal names (a dry run writes none), one fix-check reviewer reads it, and a +clean record reruns carry with `--scoped-review ` ([troubleshooting](troubleshooting.md#receipt-carry-scoped-review)). `CARRY: refused (RECEIPT_CARRY_PRODUCT_CHANGED)`, `(RECEIPT_CARRY_TREE_MISMATCH)` or `(RECEIPT_CARRY_REFUSED)` with the refused result line means the product diff changed, or history was rewritten onto a diff --git a/lib/headless-artifacts.ts b/lib/headless-artifacts.ts index be4708a97..2bd2a2dc1 100644 --- a/lib/headless-artifacts.ts +++ b/lib/headless-artifacts.ts @@ -551,9 +551,16 @@ export interface DerivedReviewStatus { findings_total: number; findings_open: number; findings_resolved: number; } export function isOpenFinding(row: Record): boolean { + if (row.action === 'open') return true; if (typeof row.disposition === 'string') return !RESOLVED.includes(row.disposition); if (typeof row.status === 'string') return !['resolved', 'fixed', 'closed'].includes(row.status); - return row.action !== 'fixed'; + return isOpenByAction(row); +} +/** A finding's `action` alone: fixed and auto-fixed resolve it; deferred and noted resolve only one marked `blocking: false` that is not CRITICAL (review policy's floor). */ +export function isOpenByAction(row: Record): boolean { + if (row.action === 'fixed' || row.action === 'auto-fixed') return false; + if (row.action === 'deferred' || row.action === 'noted') return row.blocking !== false || isCriticalFinding(row); + return true; } export function isCriticalFinding(row: Record): boolean { return typeof row.severity === 'string' && row.severity.toLowerCase() === 'critical'; diff --git a/lib/result-codes-release.ts b/lib/result-codes-release.ts index 9dc34904f..9c6496756 100644 --- a/lib/result-codes-release.ts +++ b/lib/result-codes-release.ts @@ -383,7 +383,7 @@ export const RELEASE_RESULT_CODES = { RECEIPT_CARRY_PRODUCT_CHANGED: { anchor: 'receipt-carry-product-changed', summary: 'the product diff against the base changed between the receipt\'s head and HEAD (a product file differs beyond a base merge, a test-only fix, release metadata or a regenerated output), so the reviewed verdict does not cover HEAD', - fix: 'run /ship on the new head; the receipt carries only when every changed path is a test, release metadata, a registered output the pregate regen check proved, or the base merge, and the product patch-id is unchanged', + fix: 'run /ship on the new head; the receipt carries only when every changed path is a test, release metadata, a registered output the pregate regen check proved, or the base merge, and the product patch-id is unchanged; base context next to unchanged reviewed hunks is RECEIPT_CARRY_SCOPED_REVIEW instead (one scoped fix-check, no /ship)', }, RECEIPT_CARRY_SCOPED_REVIEW: { anchor: 'receipt-carry-scoped-review', diff --git a/lib/review-evidence.ts b/lib/review-evidence.ts index 777971f50..60dc4e669 100644 --- a/lib/review-evidence.ts +++ b/lib/review-evidence.ts @@ -2,7 +2,7 @@ import { createHash } from 'node:crypto'; import { spawnSync } from 'node:child_process'; import { closeSync, constants, fstatSync, lstatSync, mkdirSync, openSync, readFileSync, unlinkSync, writeFileSync } from 'node:fs'; import { join } from 'node:path'; -import { deriveReviewStatus, readJsonl } from './headless-artifacts'; +import { deriveReviewStatus, isCriticalFinding, isOpenByAction, readJsonl } from './headless-artifacts'; import { resultLine, type ResultCodeName } from './result-codes'; const DIFF_REVIEWS = new Set(['review', 'review-fix-check', 'adversarial-review', 'codex-review', 'design-review-lite', 'ship']); @@ -248,11 +248,12 @@ const DERIVED_KEYS = ['status', 'unresolved', 'issues_found', 'critical_gaps'] a * severity + disposition/action). A claimed value that disagrees rejects the * row with REVIEW_STATUS_MISMATCH; the file is the record, never the claim. */ -export function deriveFromFindings(rec: Record, findingsPath: string): Record { +export function deriveFromFindings(rec: Record, findingsPath: string, sessionKind?: string): Record { let read; try { read = readJsonl(findingsPath); } catch (e: any) { throw new ReviewLogError('ARTIFACT_MISSING', resultLine('review-log', 'ARTIFACT_MISSING', `${findingsPath}: ${e.message}`)); } if (read.errors.length) throw new ReviewLogError('ARTIFACT_MALFORMED_JSONL', resultLine('review-log', 'ARTIFACT_MALFORMED_JSONL', `${read.errors[0]!.path}: ${read.errors[0]!.message}`)); + stampGateFindings(read.rows, sessionKind); const derived = deriveReviewStatus(read.rows); const mismatches = DERIVED_KEYS.filter(key => rec[key] !== undefined && rec[key] !== derived[key]) .map(key => `claimed ${key} ${JSON.stringify(rec[key])}, findings file has ${JSON.stringify(derived[key])}`); @@ -266,11 +267,20 @@ export function bindReview(rec: Record, token: string, env = proces for (const key of ['commit_full', 'tree', 'wtree', 'dirty', 'review_binding', 'review_freshness', 'shared_libs_coverage_version', 'status_source', 'findings_total', 'findings_open', 'findings_resolved', 'findings_file', 'scope', 'fix_base']) delete rec[key]; // The findings file is validated BEFORE the start token is consumed below: // a rejected row must leave the token for the retry. - rec = findingsPath ? deriveFromFindings(rec, findingsPath) : { ...rec, status_source: 'claimed' }; + rec = findingsPath ? deriveFromFindings(rec, findingsPath, env.GSTACK_STAMP_SESSION_KIND) : { ...rec, status_source: 'claimed' }; if (env.GSTACK_STAMP_COMMIT_FULL) rec.commit_full = env.GSTACK_STAMP_COMMIT_FULL; if (env.GSTACK_STAMP_TREE) rec.tree = env.GSTACK_STAMP_TREE; if (env.GSTACK_STAMP_DIRTY) rec.dirty = env.GSTACK_STAMP_DIRTY === 'true'; stampGateFindings(rec.findings, env.GSTACK_STAMP_SESSION_KIND); + // A /ship pass whose findings were all fixed, or deferred or noted below the floor, left no open defect (review-army + // item 6), so a claimed issues_found is recorded clean with zero counts and Step 11.5 can bind it (GST-430). Only the + // logger-stamped action counts (never a caller's status), and the findings must account for every claimed issue. + if (!findingsPath && (rec.skill === 'review' || rec.skill === 'review-fix-check') && rec.status === 'issues_found' && rec.completed === true && + Array.isArray(rec.findings) && rec.findings.length > 0 && rec.findings.length >= Number(rec.issues_found ?? 0) && + rec.findings.filter(f => record(f) && isCriticalFinding(f)).length >= Number(rec.critical ?? 0) && + rec.findings.every(f => record(f) && typeof f.action === 'string' && !isOpenByAction(f))) { + Object.assign(rec, { status: 'clean', issues_found: 0, critical: 0, informational: 0 }); + } if (!DIFF_REVIEWS.has(rec.skill)) { if (env.GSTACK_STAMP_WTREE) rec.wtree = env.GSTACK_STAMP_WTREE; return rec; @@ -326,7 +336,10 @@ export function reviewFreshness(rec: Record, currentWtree: string): if (!DIFF_REVIEWS.has(rec.skill)) return; if (rec.skill === 'ship') return { status: 'UNVERIFIED', reason: 'ship telemetry is not a review pass' }; const binding = rec.review_binding; - if (binding?.state === 'changed') return { status: 'STALE', reason: 'content changed during review' }; + if (binding?.state === 'changed') { + const short = (w: unknown) => typeof w === 'string' ? w.slice(0, 12) : 'unknown'; + return { status: 'STALE', reason: `content changed during review: the tree moved from ${short(binding.start_wtree)} to ${short(binding.end_wtree)} while ${rec.skill} ran (an edit, commit or merge landed before its row was logged), so the record cannot bind. Repair: freeze the checkout until every reviewer returns, then rerun the steps 11.5 names on the frozen tree; when the edit was your own fix, log it as a fix so gstack-review-plan picks the repeat depth (docs/troubleshooting.md#review-content-changed)` }; + } if (rec.scope === 'fix-range') return { status: 'UNVERIFIED', reason: 'fix-range pass; only a later full pass can clear review' }; if (binding?.state !== 'verified' || rec.completed !== true || rec.converged !== true || !rec.wtree || binding.start_wtree !== rec.wtree || binding.end_wtree !== rec.wtree) { @@ -340,7 +353,10 @@ export function reviewFreshness(rec: Record, currentWtree: string): // issues_found, and the legacy `critical` and Codex counters stay as they were. const open = typeof rec.findings_open === 'number' ? rec.findings_open : rec.issues_found; if (rec.status !== 'clean' || open > 0 || rec.critical > 0 || - (rec.skill === 'codex-review' && rec.findings > (rec.findings_fixed ?? 0))) { + (rec.skill === 'codex-review' && rec.findings > (rec.findings_fixed ?? 0)) || + (Array.isArray(rec.findings) && rec.findings.some(f => record(f) && (f.action === 'open' || + ((f.action === 'deferred' || f.action === 'noted') && (f.blocking === true || isCriticalFinding(f))) || + (!f.gate && isCriticalFinding(f) && isOpenByAction(f)))))) { return { status: 'UNVERIFIED', reason: 'review has unresolved findings or did not finish clean' }; } return { status: 'CURRENT', reason: 'completed clean pass on unchanged content' }; diff --git a/lib/ship-preflight.ts b/lib/ship-preflight.ts index dd1f51d5e..ed1f48f50 100644 --- a/lib/ship-preflight.ts +++ b/lib/ship-preflight.ts @@ -53,6 +53,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_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; + } } else r.add('PR: none open'); const apple = git(cwd, ['ls-files', '--', '*.xcodeproj/*', '*.xcworkspace/*', 'Package.swift']).stdout.split('\n').filter(Boolean).map(p => p.replace(/(\.xcodeproj|\.xcworkspace)\/.*$/, '$1')); if (apple.length) r.add(`APPLE: ${short([...new Set(apple)], 3)}`).next('Step 0.9: for an App Store/TestFlight ask, confirm the app target and follow apple-release.md; otherwise ignore'); diff --git a/lib/tree-receipt.ts b/lib/tree-receipt.ts index de94d0004..c946ffa71 100644 --- a/lib/tree-receipt.ts +++ b/lib/tree-receipt.ts @@ -170,7 +170,12 @@ export function buildReceipt(o: ReceiptOptions): TreeReceipt | { error: string; const versionRel = o.versionRel ?? ('rel' in cv ? cv.rel : 'rel' in gv ? gv.rel : 'VERSION'); const oldV = 'version' in gv ? gv.version : null; const newV = 'version' in cv ? cv.version : null; - const diffText = git(o.repoRoot, ['diff', '--unified=0', '--no-color', '--no-ext-diff', gatedHead, currentHead]) ?? ''; + const d = spawnSync('git', ['diff', '--unified=0', '--no-color', '--no-ext-diff', gatedHead, currentHead], { cwd: o.repoRoot, encoding: 'utf8', timeout: 60_000, maxBuffer: 256 * 1024 * 1024 }); + if (d.status !== 0) { + const why = (d.error as NodeJS.ErrnoException | undefined)?.code ?? (d.signal ? `killed by ${d.signal}` : (d.stderr ?? '').trim().split('\n')[0]?.slice(0, 200) || `exit ${d.status}`); + return { error: `git diff ${gatedHead.slice(0, 12)} ${currentHead.slice(0, 12)} failed (${why}), so the changed files cannot be classified (GST-448)`, code: 'EVIDENCE_IDENTITY_UNKNOWN' }; + } + const diffText = d.stdout.replace(/\n$/, ''); const files = classifyFiles(o.repoRoot, parseUnifiedDiff(diffText), gatedHead, currentHead, o.policy, versionRel, oldV, newV); const residual = files.filter(f => f.verdict === 'changed').map(f => f.path); const stampFiles = files.filter(f => f.verdict === 'stamp').map(f => f.path); diff --git a/package.json b/package.json index 7b2485382..8acc970b4 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.92.41", + "version": "1.92.42", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module", diff --git a/ship/SKILL.md b/ship/SKILL.md index 32de83e7d..c53c541b5 100644 --- a/ship/SKILL.md +++ b/ship/SKILL.md @@ -336,7 +336,8 @@ act on every `STOP:` and `NEXT:` line. `CARRY: would carry` (base merge committed, same product patch-id): skip review, fix what it broke (CHANGELOG, size caps), run `gstack-pregate --allow-repo-commands`, then carry without `--dry-run` plus `--pregate .gstack/tmp/pregate.json`. `CARRY: carried` binds HEAD and - ends the run. A failed check, HEAD moving after pregate, `refused` or `current` re-runs Steps 2–16. + ends the run. A failed check, HEAD moving after pregate, `current`, or `refused` with no + `NEXT: carry route` re-runs Steps 2–16. 5. Display historical readiness using the dashboard below, then finish Step 1. Prior CLEAR reviews or dashboard skips never replace Step 9's gates. @@ -457,7 +458,7 @@ not web services already deployed. The preflight merges the fetched base (`git merge`) so tests and reviews cover the integrated code (`MERGE:`; `up to date` if nothing new; `skipped` under stamp-at-merge). -**Merge conflicts:** `MERGE:` names them. Auto-resolve simple ones (VERSION, schema.rb, CHANGELOG ordering). For complex or ambiguous conflicts, **STOP**, show the conflicting choices, use AskUserQuestion for the resolution, and wait for the answer before editing or continuing; then commit and rerun the preflight. +**Merge conflicts:** `MERGE:` names them. Auto-resolve simple ones (VERSION, schema.rb, CHANGELOG ordering). For complex or ambiguous conflicts, **STOP**: show the choices, ask via AskUserQuestion, wait for the answer before any edit; then commit and rerun the preflight. **Ship policy:** `POLICY:` is `gstack-ship-policy show-effective --base ` (docs/ship-policy.md); its `ship:` lines bind Steps 12-16 and history. `RUNTIME_LINES:` goes @@ -529,8 +530,7 @@ runs items 1 and 3–5; each decision left to you is a `STOP:` naming its rerun - **FRESH** → use the recorded level or choose it in item 2, then check the queue and write. - **ALREADY_BUMPED** → keep `NEW_VERSION=currentVersion`. If `BUMP_LEVEL` is missing, use the first changed component from `baseVersion` to `currentVersion` - (major/minor/patch/micro; an absent fourth component is zero). Continue at item 3, - not another automatic bump. + (major/minor/patch/micro; an absent fourth component is zero). Continue at item 3; never bump again. - **DRIFT_STALE_PKG** → run `gstack-version-bump repair`, then reclassify. Success follows ALREADY_BUMPED, including its queue check; failure stops. Repair alone never re-bumps. @@ -549,8 +549,7 @@ runs items 1 and 3–5; each decision left to you is a `STOP:` naming its rerun **MAJOR**: ask for milestones or breaking changes (`ship: bump patch` never asks). AskUserQuestion: recommended level with rationale, smaller level, or cancel. Wait; cancel stops before release writes or push and preserves existing work. - Save lowercase `BUMP_LEVEL`. A claimed version may move the next available number - forward, but cannot change the chosen MICRO/PATCH/MINOR/MAJOR level. + Save lowercase `BUMP_LEVEL`. A claimed version may move the number forward, never the level. 3. **Queue-aware pick:** ```bash @@ -696,7 +695,8 @@ Classify the comparison in this order: Insert `5–11.5 → 12–14 → 16` before the pending Step 17, then stop this step. This repair excludes Step 14.5 because the rebuild can change generated docs. Step 16 restarts at stage 1: rebuild and compare again before stage 3 decides - documentation freshness. Further repairs use the same work list. + documentation freshness. Further repairs use the same work list. A STOP naming a scoped-review packet + instead takes its fix-check and `--context-reviewed` rerun; a blocking finding is this item. 2. **Only authored docs or release metadata changed:** Keep Step 8's original child report and counts. Recheck affected plan items using their recorded verification and append current evidence to the invocation record. If a classification is no @@ -719,6 +719,7 @@ audit or risk decision never qualifies. Report changed inputs, blockers and attempts used: +- **A fix the audit's result named:** rerun finish with `--docs-requested --docs-result `. - **An attempt remains, with changed inputs or an available repair:** insert `14.5 → 15 → 16` before Step 17. Use Blocked recovery with the existing count. Validate the outcome before Step 15, then restart Step 16 stage 1. @@ -756,9 +757,9 @@ every allowed edit is release metadata: |---|---| | FRESH (exit 0) | Cite the label, exit, timestamp, log and `tests_ran`. | | STALE/MISSING: changed content, command or age, or no proven run | Run `~/.claude/skills/gstack/bin/gstack-evidence run --label -- ''`, read the result and recheck once. Handle failures as described below. | -| ZERO-RUN: the receipt's `tests_ran` is 0 | STALE-equivalent: the lane proved nothing. Use Step 5's ZERO-RUN triage (a failed lane), never FRESH, even with exit 0. | +| ZERO-RUN: the receipt's `tests_ran` is 0 | STALE-equivalent: use Step 5's ZERO-RUN triage (a failed lane), never FRESH, even with exit 0. | | `tests_ran: unknown` | Non-blocking: keep the exit-0 verdict, cite the lane as `count unavailable`. Never record it as ZERO-RUN or as a counted pass. | -| Only receipt storage/readback failed | Independently prove unchanged final content, the same command and valid age from the successful run's evidence. Cite its exact command, exit, timestamp and log as **ledger unavailable**, never FRESH. Without that proof, use STALE/MISSING. | +| Only receipt storage/readback failed | Prove unchanged final content, the same command and valid age from the successful run's evidence. Cite its exact command, exit, timestamp and log as **ledger unavailable**, never FRESH. Without that proof, use STALE/MISSING. | No test lanes: require Step 5's explicit untested-scope approval for final content, or run Steps 5–15, including the no-tests decision, then return to Step 16 stage 1. @@ -766,8 +767,8 @@ Report the gap, never FRESH; builds must pass. **New, changed or unwaived test failure:** STOP publication. Run Steps 5–15, starting with Step 5's triage, then return to Step 16 stage 1. This recovery also -applies if a failure appears while reporting in stage 5. Reentry to Step 14.5 -keeps its existing audit count; it does not authorize a third attempt. +covers a failure seen in stage 5. Reentry to Step 14.5 keeps its audit count and +does not authorize a third attempt. ### 5. Report, then push @@ -777,8 +778,7 @@ Preserve unrelated user files. Paste build/docs/test results. Reuse waivers only for the same verified pre-existing failures and approved scope; cite the actual approval and failing -counts, never FRESH or all-green. A new, changed or unwaived test failure uses -stage 4's recovery before publication. +counts, never FRESH or all-green. Then the honesty inventory, two lines, from Step 9's latest `Gate edits:` line and its records: diff --git a/ship/SKILL.md.tmpl b/ship/SKILL.md.tmpl index 4b9491940..45a5c5232 100644 --- a/ship/SKILL.md.tmpl +++ b/ship/SKILL.md.tmpl @@ -142,7 +142,8 @@ act on every `STOP:` and `NEXT:` line. `CARRY: would carry` (base merge committed, same product patch-id): skip review, fix what it broke (CHANGELOG, size caps), run `gstack-pregate --allow-repo-commands`, then carry without `--dry-run` plus `--pregate .gstack/tmp/pregate.json`. `CARRY: carried` binds HEAD and - ends the run. A failed check, HEAD moving after pregate, `refused` or `current` re-runs Steps 2–16. + ends the run. A failed check, HEAD moving after pregate, `current`, or `refused` with no + `NEXT: carry route` re-runs Steps 2–16. 5. Display historical readiness using the dashboard below, then finish Step 1. Prior CLEAR reviews or dashboard skips never replace Step 9's gates. @@ -187,7 +188,7 @@ not web services already deployed. The preflight merges the fetched base (`git merge`) so tests and reviews cover the integrated code (`MERGE:`; `up to date` if nothing new; `skipped` under stamp-at-merge). -**Merge conflicts:** `MERGE:` names them. Auto-resolve simple ones (VERSION, schema.rb, CHANGELOG ordering). For complex or ambiguous conflicts, **STOP**, show the conflicting choices, use AskUserQuestion for the resolution, and wait for the answer before editing or continuing; then commit and rerun the preflight. +**Merge conflicts:** `MERGE:` names them. Auto-resolve simple ones (VERSION, schema.rb, CHANGELOG ordering). For complex or ambiguous conflicts, **STOP**: show the choices, ask via AskUserQuestion, wait for the answer before any edit; then commit and rerun the preflight. **Ship policy:** `POLICY:` is `gstack-ship-policy show-effective --base ` (docs/ship-policy.md); its `ship:` lines bind Steps 12-16 and history. `RUNTIME_LINES:` goes @@ -252,8 +253,7 @@ runs items 1 and 3–5; each decision left to you is a `STOP:` naming its rerun - **FRESH** → use the recorded level or choose it in item 2, then check the queue and write. - **ALREADY_BUMPED** → keep `NEW_VERSION=currentVersion`. If `BUMP_LEVEL` is missing, use the first changed component from `baseVersion` to `currentVersion` - (major/minor/patch/micro; an absent fourth component is zero). Continue at item 3, - not another automatic bump. + (major/minor/patch/micro; an absent fourth component is zero). Continue at item 3; never bump again. - **DRIFT_STALE_PKG** → run `gstack-version-bump repair`, then reclassify. Success follows ALREADY_BUMPED, including its queue check; failure stops. Repair alone never re-bumps. @@ -272,8 +272,7 @@ runs items 1 and 3–5; each decision left to you is a `STOP:` naming its rerun **MAJOR**: ask for milestones or breaking changes (`ship: bump patch` never asks). AskUserQuestion: recommended level with rationale, smaller level, or cancel. Wait; cancel stops before release writes or push and preserves existing work. - Save lowercase `BUMP_LEVEL`. A claimed version may move the next available number - forward, but cannot change the chosen MICRO/PATCH/MINOR/MAJOR level. + Save lowercase `BUMP_LEVEL`. A claimed version may move the number forward, never the level. 3. **Queue-aware pick:** ```bash @@ -417,7 +416,8 @@ Classify the comparison in this order: Insert `5–11.5 → 12–14 → 16` before the pending Step 17, then stop this step. This repair excludes Step 14.5 because the rebuild can change generated docs. Step 16 restarts at stage 1: rebuild and compare again before stage 3 decides - documentation freshness. Further repairs use the same work list. + documentation freshness. Further repairs use the same work list. A STOP naming a scoped-review packet + instead takes its fix-check and `--context-reviewed` rerun; a blocking finding is this item. 2. **Only authored docs or release metadata changed:** Keep Step 8's original child report and counts. Recheck affected plan items using their recorded verification and append current evidence to the invocation record. If a classification is no @@ -440,6 +440,7 @@ audit or risk decision never qualifies. Report changed inputs, blockers and attempts used: +- **A fix the audit's result named:** rerun finish with `--docs-requested --docs-result `. - **An attempt remains, with changed inputs or an available repair:** insert `14.5 → 15 → 16` before Step 17. Use Blocked recovery with the existing count. Validate the outcome before Step 15, then restart Step 16 stage 1. @@ -477,9 +478,9 @@ every allowed edit is release metadata: |---|---| | FRESH (exit 0) | Cite the label, exit, timestamp, log and `tests_ran`. | | STALE/MISSING: changed content, command or age, or no proven run | Run `~/.claude/skills/gstack/bin/gstack-evidence run --label -- ''`, read the result and recheck once. Handle failures as described below. | -| ZERO-RUN: the receipt's `tests_ran` is 0 | STALE-equivalent: the lane proved nothing. Use Step 5's ZERO-RUN triage (a failed lane), never FRESH, even with exit 0. | +| ZERO-RUN: the receipt's `tests_ran` is 0 | STALE-equivalent: use Step 5's ZERO-RUN triage (a failed lane), never FRESH, even with exit 0. | | `tests_ran: unknown` | Non-blocking: keep the exit-0 verdict, cite the lane as `count unavailable`. Never record it as ZERO-RUN or as a counted pass. | -| Only receipt storage/readback failed | Independently prove unchanged final content, the same command and valid age from the successful run's evidence. Cite its exact command, exit, timestamp and log as **ledger unavailable**, never FRESH. Without that proof, use STALE/MISSING. | +| Only receipt storage/readback failed | Prove unchanged final content, the same command and valid age from the successful run's evidence. Cite its exact command, exit, timestamp and log as **ledger unavailable**, never FRESH. Without that proof, use STALE/MISSING. | No test lanes: require Step 5's explicit untested-scope approval for final content, or run Steps 5–15, including the no-tests decision, then return to Step 16 stage 1. @@ -487,8 +488,8 @@ Report the gap, never FRESH; builds must pass. **New, changed or unwaived test failure:** STOP publication. Run Steps 5–15, starting with Step 5's triage, then return to Step 16 stage 1. This recovery also -applies if a failure appears while reporting in stage 5. Reentry to Step 14.5 -keeps its existing audit count; it does not authorize a third attempt. +covers a failure seen in stage 5. Reentry to Step 14.5 keeps its audit count and +does not authorize a third attempt. ### 5. Report, then push @@ -498,8 +499,7 @@ Preserve unrelated user files. Paste build/docs/test results. Reuse waivers only for the same verified pre-existing failures and approved scope; cite the actual approval and failing -counts, never FRESH or all-green. A new, changed or unwaived test failure uses -stage 4's recovery before publication. +counts, never FRESH or all-green. Then the honesty inventory, two lines, from Step 9's latest `Gate edits:` line and its records: diff --git a/test/fixtures/golden/agy-ship-SKILL.md b/test/fixtures/golden/agy-ship-SKILL.md index b3aa51ef4..431fd2735 100644 --- a/test/fixtures/golden/agy-ship-SKILL.md +++ b/test/fixtures/golden/agy-ship-SKILL.md @@ -336,7 +336,8 @@ act on every `STOP:` and `NEXT:` line. `CARRY: would carry` (base merge committed, same product patch-id): skip review, fix what it broke (CHANGELOG, size caps), run `gstack-pregate --allow-repo-commands`, then carry without `--dry-run` plus `--pregate .gstack/tmp/pregate.json`. `CARRY: carried` binds HEAD and - ends the run. A failed check, HEAD moving after pregate, `refused` or `current` re-runs Steps 2–16. + ends the run. A failed check, HEAD moving after pregate, `current`, or `refused` with no + `NEXT: carry route` re-runs Steps 2–16. 5. Display historical readiness using the dashboard below, then finish Step 1. Prior CLEAR reviews or dashboard skips never replace Step 9's gates. @@ -457,7 +458,7 @@ not web services already deployed. The preflight merges the fetched base (`git merge`) so tests and reviews cover the integrated code (`MERGE:`; `up to date` if nothing new; `skipped` under stamp-at-merge). -**Merge conflicts:** `MERGE:` names them. Auto-resolve simple ones (VERSION, schema.rb, CHANGELOG ordering). For complex or ambiguous conflicts, **STOP**, show the conflicting choices, use AskUserQuestion for the resolution, and wait for the answer before editing or continuing; then commit and rerun the preflight. +**Merge conflicts:** `MERGE:` names them. Auto-resolve simple ones (VERSION, schema.rb, CHANGELOG ordering). For complex or ambiguous conflicts, **STOP**: show the choices, ask via AskUserQuestion, wait for the answer before any edit; then commit and rerun the preflight. **Ship policy:** `POLICY:` is `gstack-ship-policy show-effective --base ` (docs/ship-policy.md); its `ship:` lines bind Steps 12-16 and history. `RUNTIME_LINES:` goes @@ -529,8 +530,7 @@ runs items 1 and 3–5; each decision left to you is a `STOP:` naming its rerun - **FRESH** → use the recorded level or choose it in item 2, then check the queue and write. - **ALREADY_BUMPED** → keep `NEW_VERSION=currentVersion`. If `BUMP_LEVEL` is missing, use the first changed component from `baseVersion` to `currentVersion` - (major/minor/patch/micro; an absent fourth component is zero). Continue at item 3, - not another automatic bump. + (major/minor/patch/micro; an absent fourth component is zero). Continue at item 3; never bump again. - **DRIFT_STALE_PKG** → run `gstack-version-bump repair`, then reclassify. Success follows ALREADY_BUMPED, including its queue check; failure stops. Repair alone never re-bumps. @@ -549,8 +549,7 @@ runs items 1 and 3–5; each decision left to you is a `STOP:` naming its rerun **MAJOR**: ask for milestones or breaking changes (`ship: bump patch` never asks). AskUserQuestion: recommended level with rationale, smaller level, or cancel. Wait; cancel stops before release writes or push and preserves existing work. - Save lowercase `BUMP_LEVEL`. A claimed version may move the next available number - forward, but cannot change the chosen MICRO/PATCH/MINOR/MAJOR level. + Save lowercase `BUMP_LEVEL`. A claimed version may move the number forward, never the level. 3. **Queue-aware pick:** ```bash @@ -696,7 +695,8 @@ Classify the comparison in this order: Insert `5–11.5 → 12–14 → 16` before the pending Step 17, then stop this step. This repair excludes Step 14.5 because the rebuild can change generated docs. Step 16 restarts at stage 1: rebuild and compare again before stage 3 decides - documentation freshness. Further repairs use the same work list. + documentation freshness. Further repairs use the same work list. A STOP naming a scoped-review packet + instead takes its fix-check and `--context-reviewed` rerun; a blocking finding is this item. 2. **Only authored docs or release metadata changed:** Keep Step 8's original child report and counts. Recheck affected plan items using their recorded verification and append current evidence to the invocation record. If a classification is no @@ -719,6 +719,7 @@ audit or risk decision never qualifies. Report changed inputs, blockers and attempts used: +- **A fix the audit's result named:** rerun finish with `--docs-requested --docs-result `. - **An attempt remains, with changed inputs or an available repair:** insert `14.5 → 15 → 16` before Step 17. Use Blocked recovery with the existing count. Validate the outcome before Step 15, then restart Step 16 stage 1. @@ -756,9 +757,9 @@ every allowed edit is release metadata: |---|---| | FRESH (exit 0) | Cite the label, exit, timestamp, log and `tests_ran`. | | STALE/MISSING: changed content, command or age, or no proven run | Run `~/.gemini/antigravity-cli/skills/gstack/bin/gstack-evidence run --label -- ''`, read the result and recheck once. Handle failures as described below. | -| ZERO-RUN: the receipt's `tests_ran` is 0 | STALE-equivalent: the lane proved nothing. Use Step 5's ZERO-RUN triage (a failed lane), never FRESH, even with exit 0. | +| ZERO-RUN: the receipt's `tests_ran` is 0 | STALE-equivalent: use Step 5's ZERO-RUN triage (a failed lane), never FRESH, even with exit 0. | | `tests_ran: unknown` | Non-blocking: keep the exit-0 verdict, cite the lane as `count unavailable`. Never record it as ZERO-RUN or as a counted pass. | -| Only receipt storage/readback failed | Independently prove unchanged final content, the same command and valid age from the successful run's evidence. Cite its exact command, exit, timestamp and log as **ledger unavailable**, never FRESH. Without that proof, use STALE/MISSING. | +| Only receipt storage/readback failed | Prove unchanged final content, the same command and valid age from the successful run's evidence. Cite its exact command, exit, timestamp and log as **ledger unavailable**, never FRESH. Without that proof, use STALE/MISSING. | No test lanes: require Step 5's explicit untested-scope approval for final content, or run Steps 5–15, including the no-tests decision, then return to Step 16 stage 1. @@ -766,8 +767,8 @@ Report the gap, never FRESH; builds must pass. **New, changed or unwaived test failure:** STOP publication. Run Steps 5–15, starting with Step 5's triage, then return to Step 16 stage 1. This recovery also -applies if a failure appears while reporting in stage 5. Reentry to Step 14.5 -keeps its existing audit count; it does not authorize a third attempt. +covers a failure seen in stage 5. Reentry to Step 14.5 keeps its audit count and +does not authorize a third attempt. ### 5. Report, then push @@ -777,8 +778,7 @@ Preserve unrelated user files. Paste build/docs/test results. Reuse waivers only for the same verified pre-existing failures and approved scope; cite the actual approval and failing -counts, never FRESH or all-green. A new, changed or unwaived test failure uses -stage 4's recovery before publication. +counts, never FRESH or all-green. Then the honesty inventory, two lines, from Step 9's latest `Gate edits:` line and its records: diff --git a/test/fixtures/golden/claude-ship-SKILL.md b/test/fixtures/golden/claude-ship-SKILL.md index 32de83e7d..c53c541b5 100644 --- a/test/fixtures/golden/claude-ship-SKILL.md +++ b/test/fixtures/golden/claude-ship-SKILL.md @@ -336,7 +336,8 @@ act on every `STOP:` and `NEXT:` line. `CARRY: would carry` (base merge committed, same product patch-id): skip review, fix what it broke (CHANGELOG, size caps), run `gstack-pregate --allow-repo-commands`, then carry without `--dry-run` plus `--pregate .gstack/tmp/pregate.json`. `CARRY: carried` binds HEAD and - ends the run. A failed check, HEAD moving after pregate, `refused` or `current` re-runs Steps 2–16. + ends the run. A failed check, HEAD moving after pregate, `current`, or `refused` with no + `NEXT: carry route` re-runs Steps 2–16. 5. Display historical readiness using the dashboard below, then finish Step 1. Prior CLEAR reviews or dashboard skips never replace Step 9's gates. @@ -457,7 +458,7 @@ not web services already deployed. The preflight merges the fetched base (`git merge`) so tests and reviews cover the integrated code (`MERGE:`; `up to date` if nothing new; `skipped` under stamp-at-merge). -**Merge conflicts:** `MERGE:` names them. Auto-resolve simple ones (VERSION, schema.rb, CHANGELOG ordering). For complex or ambiguous conflicts, **STOP**, show the conflicting choices, use AskUserQuestion for the resolution, and wait for the answer before editing or continuing; then commit and rerun the preflight. +**Merge conflicts:** `MERGE:` names them. Auto-resolve simple ones (VERSION, schema.rb, CHANGELOG ordering). For complex or ambiguous conflicts, **STOP**: show the choices, ask via AskUserQuestion, wait for the answer before any edit; then commit and rerun the preflight. **Ship policy:** `POLICY:` is `gstack-ship-policy show-effective --base ` (docs/ship-policy.md); its `ship:` lines bind Steps 12-16 and history. `RUNTIME_LINES:` goes @@ -529,8 +530,7 @@ runs items 1 and 3–5; each decision left to you is a `STOP:` naming its rerun - **FRESH** → use the recorded level or choose it in item 2, then check the queue and write. - **ALREADY_BUMPED** → keep `NEW_VERSION=currentVersion`. If `BUMP_LEVEL` is missing, use the first changed component from `baseVersion` to `currentVersion` - (major/minor/patch/micro; an absent fourth component is zero). Continue at item 3, - not another automatic bump. + (major/minor/patch/micro; an absent fourth component is zero). Continue at item 3; never bump again. - **DRIFT_STALE_PKG** → run `gstack-version-bump repair`, then reclassify. Success follows ALREADY_BUMPED, including its queue check; failure stops. Repair alone never re-bumps. @@ -549,8 +549,7 @@ runs items 1 and 3–5; each decision left to you is a `STOP:` naming its rerun **MAJOR**: ask for milestones or breaking changes (`ship: bump patch` never asks). AskUserQuestion: recommended level with rationale, smaller level, or cancel. Wait; cancel stops before release writes or push and preserves existing work. - Save lowercase `BUMP_LEVEL`. A claimed version may move the next available number - forward, but cannot change the chosen MICRO/PATCH/MINOR/MAJOR level. + Save lowercase `BUMP_LEVEL`. A claimed version may move the number forward, never the level. 3. **Queue-aware pick:** ```bash @@ -696,7 +695,8 @@ Classify the comparison in this order: Insert `5–11.5 → 12–14 → 16` before the pending Step 17, then stop this step. This repair excludes Step 14.5 because the rebuild can change generated docs. Step 16 restarts at stage 1: rebuild and compare again before stage 3 decides - documentation freshness. Further repairs use the same work list. + documentation freshness. Further repairs use the same work list. A STOP naming a scoped-review packet + instead takes its fix-check and `--context-reviewed` rerun; a blocking finding is this item. 2. **Only authored docs or release metadata changed:** Keep Step 8's original child report and counts. Recheck affected plan items using their recorded verification and append current evidence to the invocation record. If a classification is no @@ -719,6 +719,7 @@ audit or risk decision never qualifies. Report changed inputs, blockers and attempts used: +- **A fix the audit's result named:** rerun finish with `--docs-requested --docs-result `. - **An attempt remains, with changed inputs or an available repair:** insert `14.5 → 15 → 16` before Step 17. Use Blocked recovery with the existing count. Validate the outcome before Step 15, then restart Step 16 stage 1. @@ -756,9 +757,9 @@ every allowed edit is release metadata: |---|---| | FRESH (exit 0) | Cite the label, exit, timestamp, log and `tests_ran`. | | STALE/MISSING: changed content, command or age, or no proven run | Run `~/.claude/skills/gstack/bin/gstack-evidence run --label -- ''`, read the result and recheck once. Handle failures as described below. | -| ZERO-RUN: the receipt's `tests_ran` is 0 | STALE-equivalent: the lane proved nothing. Use Step 5's ZERO-RUN triage (a failed lane), never FRESH, even with exit 0. | +| ZERO-RUN: the receipt's `tests_ran` is 0 | STALE-equivalent: use Step 5's ZERO-RUN triage (a failed lane), never FRESH, even with exit 0. | | `tests_ran: unknown` | Non-blocking: keep the exit-0 verdict, cite the lane as `count unavailable`. Never record it as ZERO-RUN or as a counted pass. | -| Only receipt storage/readback failed | Independently prove unchanged final content, the same command and valid age from the successful run's evidence. Cite its exact command, exit, timestamp and log as **ledger unavailable**, never FRESH. Without that proof, use STALE/MISSING. | +| Only receipt storage/readback failed | Prove unchanged final content, the same command and valid age from the successful run's evidence. Cite its exact command, exit, timestamp and log as **ledger unavailable**, never FRESH. Without that proof, use STALE/MISSING. | No test lanes: require Step 5's explicit untested-scope approval for final content, or run Steps 5–15, including the no-tests decision, then return to Step 16 stage 1. @@ -766,8 +767,8 @@ Report the gap, never FRESH; builds must pass. **New, changed or unwaived test failure:** STOP publication. Run Steps 5–15, starting with Step 5's triage, then return to Step 16 stage 1. This recovery also -applies if a failure appears while reporting in stage 5. Reentry to Step 14.5 -keeps its existing audit count; it does not authorize a third attempt. +covers a failure seen in stage 5. Reentry to Step 14.5 keeps its audit count and +does not authorize a third attempt. ### 5. Report, then push @@ -777,8 +778,7 @@ Preserve unrelated user files. Paste build/docs/test results. Reuse waivers only for the same verified pre-existing failures and approved scope; cite the actual approval and failing -counts, never FRESH or all-green. A new, changed or unwaived test failure uses -stage 4's recovery before publication. +counts, never FRESH or all-green. Then the honesty inventory, two lines, from Step 9's latest `Gate edits:` line and its records: diff --git a/test/fixtures/golden/codex-ship-SKILL.md b/test/fixtures/golden/codex-ship-SKILL.md index e3b16338d..44868bb8a 100644 --- a/test/fixtures/golden/codex-ship-SKILL.md +++ b/test/fixtures/golden/codex-ship-SKILL.md @@ -334,7 +334,8 @@ act on every `STOP:` and `NEXT:` line. `CARRY: would carry` (base merge committed, same product patch-id): skip review, fix what it broke (CHANGELOG, size caps), run `gstack-pregate --allow-repo-commands`, then carry without `--dry-run` plus `--pregate .gstack/tmp/pregate.json`. `CARRY: carried` binds HEAD and - ends the run. A failed check, HEAD moving after pregate, `refused` or `current` re-runs Steps 2–16. + ends the run. A failed check, HEAD moving after pregate, `current`, or `refused` with no + `NEXT: carry route` re-runs Steps 2–16. 5. Display historical readiness using the dashboard below, then finish Step 1. Prior CLEAR reviews or dashboard skips never replace Step 9's gates. @@ -456,7 +457,7 @@ not web services already deployed. The preflight merges the fetched base (`git merge`) so tests and reviews cover the integrated code (`MERGE:`; `up to date` if nothing new; `skipped` under stamp-at-merge). -**Merge conflicts:** `MERGE:` names them. Auto-resolve simple ones (VERSION, schema.rb, CHANGELOG ordering). For complex or ambiguous conflicts, **STOP**, show the conflicting choices, use AskUserQuestion for the resolution, and wait for the answer before editing or continuing; then commit and rerun the preflight. +**Merge conflicts:** `MERGE:` names them. Auto-resolve simple ones (VERSION, schema.rb, CHANGELOG ordering). For complex or ambiguous conflicts, **STOP**: show the choices, ask via AskUserQuestion, wait for the answer before any edit; then commit and rerun the preflight. **Ship policy:** `POLICY:` is `gstack-ship-policy show-effective --base ` (docs/ship-policy.md); its `ship:` lines bind Steps 12-16 and history. `RUNTIME_LINES:` goes @@ -529,8 +530,7 @@ runs items 1 and 3–5; each decision left to you is a `STOP:` naming its rerun - **FRESH** → use the recorded level or choose it in item 2, then check the queue and write. - **ALREADY_BUMPED** → keep `NEW_VERSION=currentVersion`. If `BUMP_LEVEL` is missing, use the first changed component from `baseVersion` to `currentVersion` - (major/minor/patch/micro; an absent fourth component is zero). Continue at item 3, - not another automatic bump. + (major/minor/patch/micro; an absent fourth component is zero). Continue at item 3; never bump again. - **DRIFT_STALE_PKG** → run `gstack-version-bump repair`, then reclassify. Success follows ALREADY_BUMPED, including its queue check; failure stops. Repair alone never re-bumps. @@ -549,8 +549,7 @@ runs items 1 and 3–5; each decision left to you is a `STOP:` naming its rerun **MAJOR**: ask for milestones or breaking changes (`ship: bump patch` never asks). AskUserQuestion: recommended level with rationale, smaller level, or cancel. Wait; cancel stops before release writes or push and preserves existing work. - Save lowercase `BUMP_LEVEL`. A claimed version may move the next available number - forward, but cannot change the chosen MICRO/PATCH/MINOR/MAJOR level. + Save lowercase `BUMP_LEVEL`. A claimed version may move the number forward, never the level. 3. **Queue-aware pick:** ```bash @@ -698,7 +697,8 @@ Classify the comparison in this order: Insert `5–11.5 → 12–14 → 16` before the pending Step 17, then stop this step. This repair excludes Step 14.5 because the rebuild can change generated docs. Step 16 restarts at stage 1: rebuild and compare again before stage 3 decides - documentation freshness. Further repairs use the same work list. + documentation freshness. Further repairs use the same work list. A STOP naming a scoped-review packet + instead takes its fix-check and `--context-reviewed` rerun; a blocking finding is this item. 2. **Only authored docs or release metadata changed:** Keep Step 8's original child report and counts. Recheck affected plan items using their recorded verification and append current evidence to the invocation record. If a classification is no @@ -721,6 +721,7 @@ audit or risk decision never qualifies. Report changed inputs, blockers and attempts used: +- **A fix the audit's result named:** rerun finish with `--docs-requested --docs-result `. - **An attempt remains, with changed inputs or an available repair:** insert `14.5 → 15 → 16` before Step 17. Use Blocked recovery with the existing count. Validate the outcome before Step 15, then restart Step 16 stage 1. @@ -759,9 +760,9 @@ $GSTACK_ROOT/bin/gstack-evidence check --label tests --expect-cmd '' --la |---|---| | FRESH (exit 0) | Cite the label, exit, timestamp, log and `tests_ran`. | | STALE/MISSING: changed content, command or age, or no proven run | Run `$GSTACK_ROOT/bin/gstack-evidence run --label -- ''`, read the result and recheck once. Handle failures as described below. | -| ZERO-RUN: the receipt's `tests_ran` is 0 | STALE-equivalent: the lane proved nothing. Use Step 5's ZERO-RUN triage (a failed lane), never FRESH, even with exit 0. | +| ZERO-RUN: the receipt's `tests_ran` is 0 | STALE-equivalent: use Step 5's ZERO-RUN triage (a failed lane), never FRESH, even with exit 0. | | `tests_ran: unknown` | Non-blocking: keep the exit-0 verdict, cite the lane as `count unavailable`. Never record it as ZERO-RUN or as a counted pass. | -| Only receipt storage/readback failed | Independently prove unchanged final content, the same command and valid age from the successful run's evidence. Cite its exact command, exit, timestamp and log as **ledger unavailable**, never FRESH. Without that proof, use STALE/MISSING. | +| Only receipt storage/readback failed | Prove unchanged final content, the same command and valid age from the successful run's evidence. Cite its exact command, exit, timestamp and log as **ledger unavailable**, never FRESH. Without that proof, use STALE/MISSING. | No test lanes: require Step 5's explicit untested-scope approval for final content, or run Steps 5–15, including the no-tests decision, then return to Step 16 stage 1. @@ -769,8 +770,8 @@ Report the gap, never FRESH; builds must pass. **New, changed or unwaived test failure:** STOP publication. Run Steps 5–15, starting with Step 5's triage, then return to Step 16 stage 1. This recovery also -applies if a failure appears while reporting in stage 5. Reentry to Step 14.5 -keeps its existing audit count; it does not authorize a third attempt. +covers a failure seen in stage 5. Reentry to Step 14.5 keeps its audit count and +does not authorize a third attempt. ### 5. Report, then push @@ -780,8 +781,7 @@ Preserve unrelated user files. Paste build/docs/test results. Reuse waivers only for the same verified pre-existing failures and approved scope; cite the actual approval and failing -counts, never FRESH or all-green. A new, changed or unwaived test failure uses -stage 4's recovery before publication. +counts, never FRESH or all-green. Then the honesty inventory, two lines, from Step 9's latest `Gate edits:` line and its records: diff --git a/test/fixtures/golden/factory-ship-SKILL.md b/test/fixtures/golden/factory-ship-SKILL.md index 148a7b6fb..dc9aa3ebf 100644 --- a/test/fixtures/golden/factory-ship-SKILL.md +++ b/test/fixtures/golden/factory-ship-SKILL.md @@ -336,7 +336,8 @@ act on every `STOP:` and `NEXT:` line. `CARRY: would carry` (base merge committed, same product patch-id): skip review, fix what it broke (CHANGELOG, size caps), run `gstack-pregate --allow-repo-commands`, then carry without `--dry-run` plus `--pregate .gstack/tmp/pregate.json`. `CARRY: carried` binds HEAD and - ends the run. A failed check, HEAD moving after pregate, `refused` or `current` re-runs Steps 2–16. + ends the run. A failed check, HEAD moving after pregate, `current`, or `refused` with no + `NEXT: carry route` re-runs Steps 2–16. 5. Display historical readiness using the dashboard below, then finish Step 1. Prior CLEAR reviews or dashboard skips never replace Step 9's gates. @@ -458,7 +459,7 @@ not web services already deployed. The preflight merges the fetched base (`git merge`) so tests and reviews cover the integrated code (`MERGE:`; `up to date` if nothing new; `skipped` under stamp-at-merge). -**Merge conflicts:** `MERGE:` names them. Auto-resolve simple ones (VERSION, schema.rb, CHANGELOG ordering). For complex or ambiguous conflicts, **STOP**, show the conflicting choices, use AskUserQuestion for the resolution, and wait for the answer before editing or continuing; then commit and rerun the preflight. +**Merge conflicts:** `MERGE:` names them. Auto-resolve simple ones (VERSION, schema.rb, CHANGELOG ordering). For complex or ambiguous conflicts, **STOP**: show the choices, ask via AskUserQuestion, wait for the answer before any edit; then commit and rerun the preflight. **Ship policy:** `POLICY:` is `gstack-ship-policy show-effective --base ` (docs/ship-policy.md); its `ship:` lines bind Steps 12-16 and history. `RUNTIME_LINES:` goes @@ -531,8 +532,7 @@ runs items 1 and 3–5; each decision left to you is a `STOP:` naming its rerun - **FRESH** → use the recorded level or choose it in item 2, then check the queue and write. - **ALREADY_BUMPED** → keep `NEW_VERSION=currentVersion`. If `BUMP_LEVEL` is missing, use the first changed component from `baseVersion` to `currentVersion` - (major/minor/patch/micro; an absent fourth component is zero). Continue at item 3, - not another automatic bump. + (major/minor/patch/micro; an absent fourth component is zero). Continue at item 3; never bump again. - **DRIFT_STALE_PKG** → run `gstack-version-bump repair`, then reclassify. Success follows ALREADY_BUMPED, including its queue check; failure stops. Repair alone never re-bumps. @@ -551,8 +551,7 @@ runs items 1 and 3–5; each decision left to you is a `STOP:` naming its rerun **MAJOR**: ask for milestones or breaking changes (`ship: bump patch` never asks). AskUserQuestion: recommended level with rationale, smaller level, or cancel. Wait; cancel stops before release writes or push and preserves existing work. - Save lowercase `BUMP_LEVEL`. A claimed version may move the next available number - forward, but cannot change the chosen MICRO/PATCH/MINOR/MAJOR level. + Save lowercase `BUMP_LEVEL`. A claimed version may move the number forward, never the level. 3. **Queue-aware pick:** ```bash @@ -700,7 +699,8 @@ Classify the comparison in this order: Insert `5–11.5 → 12–14 → 16` before the pending Step 17, then stop this step. This repair excludes Step 14.5 because the rebuild can change generated docs. Step 16 restarts at stage 1: rebuild and compare again before stage 3 decides - documentation freshness. Further repairs use the same work list. + documentation freshness. Further repairs use the same work list. A STOP naming a scoped-review packet + instead takes its fix-check and `--context-reviewed` rerun; a blocking finding is this item. 2. **Only authored docs or release metadata changed:** Keep Step 8's original child report and counts. Recheck affected plan items using their recorded verification and append current evidence to the invocation record. If a classification is no @@ -723,6 +723,7 @@ audit or risk decision never qualifies. Report changed inputs, blockers and attempts used: +- **A fix the audit's result named:** rerun finish with `--docs-requested --docs-result `. - **An attempt remains, with changed inputs or an available repair:** insert `14.5 → 15 → 16` before Step 17. Use Blocked recovery with the existing count. Validate the outcome before Step 15, then restart Step 16 stage 1. @@ -761,9 +762,9 @@ $GSTACK_ROOT/bin/gstack-evidence check --label tests --expect-cmd '' --la |---|---| | FRESH (exit 0) | Cite the label, exit, timestamp, log and `tests_ran`. | | STALE/MISSING: changed content, command or age, or no proven run | Run `$GSTACK_ROOT/bin/gstack-evidence run --label -- ''`, read the result and recheck once. Handle failures as described below. | -| ZERO-RUN: the receipt's `tests_ran` is 0 | STALE-equivalent: the lane proved nothing. Use Step 5's ZERO-RUN triage (a failed lane), never FRESH, even with exit 0. | +| ZERO-RUN: the receipt's `tests_ran` is 0 | STALE-equivalent: use Step 5's ZERO-RUN triage (a failed lane), never FRESH, even with exit 0. | | `tests_ran: unknown` | Non-blocking: keep the exit-0 verdict, cite the lane as `count unavailable`. Never record it as ZERO-RUN or as a counted pass. | -| Only receipt storage/readback failed | Independently prove unchanged final content, the same command and valid age from the successful run's evidence. Cite its exact command, exit, timestamp and log as **ledger unavailable**, never FRESH. Without that proof, use STALE/MISSING. | +| Only receipt storage/readback failed | Prove unchanged final content, the same command and valid age from the successful run's evidence. Cite its exact command, exit, timestamp and log as **ledger unavailable**, never FRESH. Without that proof, use STALE/MISSING. | No test lanes: require Step 5's explicit untested-scope approval for final content, or run Steps 5–15, including the no-tests decision, then return to Step 16 stage 1. @@ -771,8 +772,8 @@ Report the gap, never FRESH; builds must pass. **New, changed or unwaived test failure:** STOP publication. Run Steps 5–15, starting with Step 5's triage, then return to Step 16 stage 1. This recovery also -applies if a failure appears while reporting in stage 5. Reentry to Step 14.5 -keeps its existing audit count; it does not authorize a third attempt. +covers a failure seen in stage 5. Reentry to Step 14.5 keeps its audit count and +does not authorize a third attempt. ### 5. Report, then push @@ -782,8 +783,7 @@ Preserve unrelated user files. Paste build/docs/test results. Reuse waivers only for the same verified pre-existing failures and approved scope; cite the actual approval and failing -counts, never FRESH or all-green. A new, changed or unwaived test failure uses -stage 4's recovery before publication. +counts, never FRESH or all-green. Then the honesty inventory, two lines, from Step 9's latest `Gate edits:` line and its records: diff --git a/test/review-notes-only-pass.test.ts b/test/review-notes-only-pass.test.ts new file mode 100644 index 000000000..fb58821e6 --- /dev/null +++ b/test/review-notes-only-pass.test.ts @@ -0,0 +1,144 @@ +/** + * GST-430 (CAPY-2, qm#2837): a /ship review pass whose only findings were notes or deferrals has no + * open defect, so the logger records it clean with issues_found 0 and Step 11.5 can bind it. A finding + * still open (skipped, gate open, no action) keeps the claimed status. A row whose tree moved during + * the pass names that cause and the cheapest repair. + */ +import { describe, test, expect, beforeEach, afterEach } from 'bun:test'; +import { spawnSync } from 'node:child_process'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { isOpenFinding } from '../lib/headless-artifacts'; +import { reviewFreshness } from '../lib/review-evidence'; + +const ROOT = path.resolve(import.meta.dir, '..'); +const BIN = path.join(ROOT, 'bin', 'gstack-review-log'); + +let home: string; +beforeEach(() => { home = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-notes-only-')); }); +afterEach(() => { fs.rmSync(home, { recursive: true, force: true }); }); + +function run(args: string[]) { + const r = spawnSync(BIN, args, { cwd: ROOT, encoding: 'utf8', timeout: 60_000, env: { ...process.env, GSTACK_HOME: home, GSTACK_SESSION_KIND: 'unattended' } }); + return { code: r.status, err: r.stderr, out: r.stdout }; +} +function lastRow(): any { + const projects = path.join(home, 'projects'); + const rows = fs.readdirSync(projects).flatMap(slug => fs.readdirSync(path.join(projects, slug)).filter(f => f.endsWith('-reviews.jsonl')) + .flatMap(f => fs.readFileSync(path.join(projects, slug, f), 'utf8').trim().split('\n').map(l => JSON.parse(l)))); + return rows.at(-1); +} +const note = (i: number, action: string, extra: Record = {}) => + ({ fingerprint: `lib/a.ts:${i}:testing`, severity: 'INFORMATIONAL', action, locus: 'diff', kind: 'test-gap', blocking: false, raised_by: 'specialist-batched', ...extra }); +const pass = (findings: unknown[], extra: Record = {}) => JSON.stringify({ + skill: 'review', via: 'ship', timestamp: '2026-10-31T00:00:00Z', status: 'issues_found', issues_found: findings.length, critical: 0, + informational: findings.length, quality_score: 9, specialists: {}, findings, completed: true, converged: true, cycles: 0, pass: 2, + review_scope: 'frozen', range_base: null, ...extra, +}); + +describe('a notes-only /ship review pass (GST-430)', () => { + // Value: protects=Step 11.5 binding a pass whose 3 notes are not defects; catches=a notes-only pass logged as + // issues_found and left UNVERIFIED, which cost CAPY-2 an extra rebind pass. + test('three noted findings are recorded clean with 0 issues and read CURRENT', () => { + const token = run(['--start', 'review']).out.trim(); + const r = run([pass([note(1, 'noted'), note(2, 'noted'), note(3, 'deferred', { issue: null })]), '--finish', token]); + expect(r.code).toBe(0); + const row = lastRow(); + expect([row.status, row.issues_found, row.critical, row.informational, row.review_binding.state]).toEqual(['clean', 0, 0, 0, 'verified']); + expect(reviewFreshness(row, row.wtree)).toEqual({ status: 'CURRENT', reason: 'completed clean pass on unchanged content' }); + }); + + // Value: protects=open defects still blocking 11.5; catches=a normalization that clears a skipped finding, a + // gate finding the unattended logger forced open, a finding with no action, or an incomplete pass. + test('negative controls keep issues_found', () => { + const cases: Array<[string, string]> = [ + ['skipped', pass([note(1, 'noted'), note(2, 'skipped')])], + ['gate open', pass([note(1, 'noted'), note(2, 'kept', { gate: 'RH1', gate_id: 'g', reason: 'r' })])], + ['no action', pass([note(1, 'noted'), { fingerprint: 'lib/a.ts:9:testing', severity: 'CRITICAL' }])], + ['incomplete', pass([note(1, 'noted')], { completed: false })], + ['caller status over a stamped-open gate', pass([note(1, 'noted'), note(2, 'kept', { gate: 'RH1', gate_id: 'g', reason: 'r', severity: 'CRITICAL', status: 'resolved' })])], + ['critical noted', pass([note(1, 'noted'), note(2, 'noted', { severity: 'CRITICAL' })])], + ['blocking deferred', pass([note(1, 'noted'), note(2, 'deferred', { blocking: true, issue: null })])], + ['counts beyond the findings', pass([note(1, 'noted')], { issues_found: 2, critical: 1 })], + ['noted with no blocking field', pass([note(1, 'noted'), note(2, 'noted', { blocking: undefined })])], + ]; + for (const [name, row] of cases) { + expect(run([row]).code).toBe(0); + expect([name, lastRow().status]).toEqual([name, 'issues_found']); + } + }); + + // Value: protects=a claimed clean row cannot carry a gate finding the unattended logger stamped open past Step 11.5; + // catches=a reader that trusts the claimed status and counts over the stamped finding (red-team#2, pass 1). + test('a claimed clean row with a stamped-open gate finding is not CURRENT', () => { + const token = run(['--start', 'review']).out.trim(); + const gate = note(1, 'kept', { gate: 'RH1', gate_id: 'g', reason: 'r', severity: 'CRITICAL' }); + expect(run([pass([gate], { status: 'clean', issues_found: 0, critical: 0, informational: 0 }), '--finish', token]).code).toBe(0); + const row = lastRow(); + expect([row.status, row.findings[0].action, row.review_binding.state]).toEqual(['clean', 'open', 'verified']); + expect(reviewFreshness(row, row.wtree)?.status).toBe('UNVERIFIED'); + }); + + // Value: protects=a claimed clean row reads open findings by the same rule the rewrite uses; catches=a CRITICAL + // deferral or a skipped finding under a claimed clean status reading CURRENT (adversarial-native#3, Step 11). + // Kept and restored interactive gates, skipped advisories and an INFORMATIONAL deferral stay CURRENT, as before this PR + // (fix-check and adversarial-native, Step 11 round 2). + test('a claimed clean row with a CRITICAL deferral is not CURRENT; resolved gate, skipped and below-floor actions are', () => { + const w = 'c'.repeat(40); + const row = (findings: unknown[]) => ({ skill: 'review', status: 'clean', issues_found: 0, critical: 0, completed: true, converged: true, wtree: w, + review_binding: { state: 'verified', start_wtree: w, end_wtree: w }, findings, status_source: 'claimed' }); + expect(reviewFreshness(row([note(1, 'deferred', { severity: 'CRITICAL', issue: null })]), w)?.status).toBe('UNVERIFIED'); + expect(reviewFreshness(row([note(1, 'deferred', { blocking: true })]), w)?.status).toBe('UNVERIFIED'); + expect(reviewFreshness(row([note(1, 'open', { gate: 'RH1' })]), w)?.status).toBe('UNVERIFIED'); + expect([note(1, 'skipped', { severity: 'CRITICAL' }), { fingerprint: 'lib/a.ts:2:x', severity: 'CRITICAL' }, note(1, 'mystery', { severity: 'CRITICAL' })] + .map(f => reviewFreshness(row([f]), w)?.status)).toEqual(['UNVERIFIED', 'UNVERIFIED', 'UNVERIFIED']); + expect(reviewFreshness(row([note(1, 'kept', { gate: 'RH1', reason: 'r', actor: 'interactive', severity: 'CRITICAL' })]), w)?.status).toBe('CURRENT'); + const gate = (action: string) => note(1, action, { gate: 'RH1', reason: 'r', actor: 'interactive' }); + expect([gate('kept'), gate('restored'), note(1, 'skipped'), note(1, 'deferred', { blocking: undefined }), note(1, 'noted')] + .map(f => reviewFreshness(row([f]), w)?.status)).toEqual(['CURRENT', 'CURRENT', 'CURRENT', 'CURRENT', 'CURRENT']); + }); + + // Value: protects=a findings file cannot dispose of a gate finding the unattended logger must force open; catches=gate rows + // from --findings derived as resolved without stamping (outside-voice#1, Step 11). + test('a --findings file gate row is stamped open before its status is derived', () => { + const file = path.join(home, 'findings.jsonl'); + fs.writeFileSync(file, JSON.stringify({ schema_version: 1, run: 'r', phase: 'review', voice: 'native', title: 't', fingerprint: 'lib/a.ts:1:x', severity: 'CRITICAL', action: 'auto-fixed', gate: 'RH1', gate_id: 'g' }) + '\n'); + const token = run(['--start', 'review']).out.trim(); + run([JSON.stringify({ skill: 'review', via: 'ship', timestamp: '2026-10-31T00:00:00Z', completed: true, converged: true }), '--findings', file, '--finish', token]); + const row = lastRow(); + expect([row.findings_open, row.status === 'clean']).toEqual([1, false]); + expect(reviewFreshness(row, row.wtree)?.status).not.toBe('CURRENT'); + }); + + // Value: protects=the forced-open stamp wins over a findings file's own status or disposition; catches=a gate row marked + // status resolved deriving clean after it was stamped open (outside-voice#1, Step 11 round 2). + test('a --findings gate row stamped open stays open whatever its status or disposition says', () => { + for (const extra of [{ status: 'resolved' }, { disposition: 'fixed' }]) { + const file = path.join(home, 'findings.jsonl'); + fs.writeFileSync(file, JSON.stringify({ schema_version: 1, run: 'r', phase: 'review', voice: 'native', title: 't', fingerprint: 'lib/a.ts:1:x', severity: 'CRITICAL', action: 'auto-fixed', gate: 'RH1', gate_id: 'g', ...extra }) + '\n'); + const token = run(['--start', 'review']).out.trim(); + run([JSON.stringify({ skill: 'review', via: 'ship', timestamp: '2026-10-31T00:00:00Z', completed: true, converged: true }), '--findings', file, '--finish', token]); + expect([JSON.stringify(extra), lastRow().findings_open]).toEqual([JSON.stringify(extra), 1]); + } + }); + + test('isOpenFinding: fixed and auto-fixed are not open; noted and deferred only below the floor; skipped and missing actions are', () => { + expect(['fixed', 'auto-fixed', 'deferred', 'noted', 'skipped', undefined].map(action => isOpenFinding({ action, blocking: false }))) + .toEqual([false, false, false, false, true, true]); + expect(['deferred', 'noted'].map(action => isOpenFinding({ action }))).toEqual([true, true]); + }); +}); + +describe('a tree that changed during the pass (GST-430)', () => { + // Value: protects=a refusal that says why the record cannot bind and what the cheapest repair is; catches=the + // bare 'content changed during review' that sent CAPY-2 through a ~$20 full pass without saying why. + test('the STALE reason names both trees, the cause and the repair', () => { + const s = 'a'.repeat(64), e = 'b'.repeat(64); + const fresh = reviewFreshness({ skill: 'adversarial-review', review_binding: { state: 'changed', start_wtree: s, end_wtree: e } }, e); + expect(fresh?.status).toBe('STALE'); + expect(fresh?.reason).toContain(`content changed during review: the tree moved from ${s.slice(0, 12)} to ${e.slice(0, 12)} while adversarial-review ran`); + expect(fresh?.reason).toContain('freeze the checkout until every reviewer returns'); + expect(fresh?.reason).toContain('docs/troubleshooting.md#review-content-changed'); + }); +}); diff --git a/test/ship-control-flow.test.ts b/test/ship-control-flow.test.ts index 9c0048eec..b6da27c93 100644 --- a/test/ship-control-flow.test.ts +++ b/test/ship-control-flow.test.ts @@ -55,7 +55,7 @@ describe('ship source controller', () => { test('Step 1 carries a receipt only after a dry run and a pregate at HEAD', () => { const carry = compact(entry.slice(entry.indexOf('4. **Carry check.**'), entry.indexOf('5. Display historical readiness'))); - const order = ['carry --pr --base --dry-run', '`CARRY: would carry`', 'skip review', '`gstack-pregate --allow-repo-commands`', 'carry without `--dry-run`', '`--pregate .gstack/tmp/pregate.json`', '`CARRY: carried` binds HEAD', 'A failed check, HEAD moving after pregate, `refused` or `current` re-runs Steps 2–16.']; + const order = ['carry --pr --base --dry-run', '`CARRY: would carry`', 'skip review', '`gstack-pregate --allow-repo-commands`', 'carry without `--dry-run`', '`--pregate .gstack/tmp/pregate.json`', '`CARRY: carried` binds HEAD', 'A failed check, HEAD moving after pregate, `current`, or `refused` with no `NEXT: carry route` re-runs Steps 2–16.']; const at = order.map(t => carry.indexOf(t)); expect(at.every(i => i >= 0)).toBe(true); expect(at).toEqual([...at].sort((a, b) => a - b)); @@ -338,6 +338,16 @@ describe('ship source controller', () => { expect(docs).toMatch(/never silently refresh hashes/i); }); + // Value: protects=finish's scoped-review STOP and --docs-requested carry are routes Step 16 names, so a driver that reads + // the template instead of the STOP text keeps the GST-384/GST-391 saving; fails_when=stage 2 sends every tests=/context= + // STOP to the full loop or stage 3 offers only a re-audit or a named risk (GST-444); seam=none; no_claim=the bins' own STOP text + test('Step 16 routes the scoped-review STOP and the --docs-requested fix (GST-444)', () => { + const behavior = gate.slice(gate.indexOf('1. **Behavior'), gate.indexOf('2. **Only authored')); + expect(behavior).toContain('A STOP naming a scoped-review packet instead takes its fix-check and `--context-reviewed` rerun; a blocking finding is this item.'); + const docs = gate.slice(gate.indexOf('### 3.'), gate.indexOf('### 4.')); + expect(docs).toContain('**A fix the audit\'s result named:** rerun finish with `--docs-requested --docs-result `.'); + }); + test('tests, absent test approval and new writes reopen final verification', () => { const tests = gate.slice(gate.indexOf('### 4.'), gate.indexOf('### 5.')); expect(tests).toMatch(/\*\*New, changed or unwaived test failure:\*\* stop publication\. Run Steps 5–15/i); diff --git a/test/ship-receipt-carry-large-diff.test.ts b/test/ship-receipt-carry-large-diff.test.ts new file mode 100644 index 000000000..d1976ee35 --- /dev/null +++ b/test/ship-receipt-carry-large-diff.test.ts @@ -0,0 +1,90 @@ +/** + * gstack-ship-receipt carry on a PR whose own diff is large (GST-448, harness-evals#147): a results PR that adds thousands + * of run-receipt files has a name-only listing and a product diff over the child-process output buffer, so carry refused + * with "git could not compute HEAD's product patch-id: git diff … failed" after every base move. + */ +import { afterEach, describe, expect, setDefaultTimeout, test } from 'bun:test'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { EXIT } from '../lib/headless-artifacts'; +import { extractReceipt } from '../lib/ship-receipt'; +import { cleanup, commit, makeFixtureRepo as makeRepo, runBin, write, type FixtureRepo } from './helpers/pregate-fixture'; +const makeFixtureRepo = (): FixtureRepo => { const r = makeRepo(); r.git('config', 'core.longpaths', 'true'); return r; }; + +setDefaultTimeout(120_000); +const repos: FixtureRepo[] = []; +afterEach(() => { while (repos.length) cleanup(repos.pop()!); }); +const DIR = `results/${'harness-eval-run-receipts-'.repeat(8)}`; +/** `count` run receipts under a long directory name: about 1.4 MB of paths and 2 MB of diff, past the 1 MB default buffer. */ +const receipts = (count: number, tag: string) => Object.fromEntries(Array.from({ length: count }, (_, i) => [`${DIR}/${tag}-run-${String(i).padStart(5, '0')}.json`, `{"run":${i},"tag":"${tag}","score":0.${i % 10}}\n`])); + +describe('carry with a large product diff (GST-448)', () => { + // Value: protects=a results PR keeps its receipt across a base move when its own diff is megabytes of added files + // (harness-evals#147 paid ~3h of /ship per base move); fails_when=carry buffers git's name-only listing or the diff in + // a bounded child-process buffer and refuses; seam=none; no_claim=gh pr edit (--body stands in) + test('a base merge carries the receipt of a PR that adds 6000 files', () => { + const repo = makeFixtureRepo(); repos.push(repo); + commit(repo, receipts(6000, 'base'), 'results: 6000 run receipts'); + const body = path.join(repo.root, 'body.md'); + const w = runBin(repo, 'gstack-ship-receipt', ['write', '--pr', '9', '--base', 'main', '--version', '1.0.0', '--session-kind', 'unattended', '--body', body]); + expect(w.status).toBe(EXIT.ok); + const reviewed = extractReceipt(fs.readFileSync(body, 'utf8')).receipt!.product_patch_id; + expect(reviewed).toMatch(/^[0-9a-f]{40}$/); + repo.git('checkout', '-q', 'main'); + write(repo.work, 'docs/README.md', '# docs moved on main\n'); + repo.git('add', '-A'); repo.git('commit', '-qm', 'main moves on'); repo.git('push', '-q', 'origin', 'main'); + repo.git('checkout', '-q', 'feature'); + repo.git('merge', '-q', '--no-edit', 'origin/main'); + const out = runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--base', 'main']); + expect([out.status, out.out.split('\n').find(l => l.startsWith('CARRY:')), out.err.split('\n').find(l => l.includes('RECEIPT_CARRY'))]).toEqual([EXIT.ok, expect.stringMatching(/^CARRY: carried/), undefined]); + expect(extractReceipt(fs.readFileSync(body, 'utf8')).receipt!.product_patch_id).toBe(reviewed); + }); + + // Value: protects=negative control: the large-diff path still refuses when the PR's own receipts change after review; + // fails_when=the streamed patch-id reads as unchanged for a changed diff; seam=none; no_claim=none + test('one changed receipt in the large diff still refuses the carry', () => { + const repo = makeFixtureRepo(); repos.push(repo); + commit(repo, receipts(6000, 'base'), 'results: 6000 run receipts'); + const body = path.join(repo.root, 'body.md'); + expect(runBin(repo, 'gstack-ship-receipt', ['write', '--pr', '9', '--base', 'main', '--version', '1.0.0', '--session-kind', 'unattended', '--body', body]).status).toBe(EXIT.ok); + commit(repo, { [`${DIR}/base-run-00042.json`]: '{"run":42,"tag":"base","score":0.9}\n' }, 'results: rescore run 42'); + const out = runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--base', 'main']); + expect([out.status, out.out.split('\n').find(l => l.startsWith('CARRY:'))]).toEqual([EXIT.refused, 'CARRY: refused (RECEIPT_CARRY_PRODUCT_CHANGED)']); + }); + + // Value: protects=the patch-id diff lives under the git dir (on disk, not a tmpfs TMPDIR), and a spot it cannot write is a + // named refusal, never a stack trace preflight reads as 'CARRY: check failed'; fails_when=patchId writes to TMPDIR or + // creates its directory outside its error handling; seam=TMPDIR, a file where the directory goes; no_claim=none + test('patch-id diffs go under the git dir: an unwritable TMPDIR still carries, an unwritable git-dir spot refuses by name', () => { + const repo = makeFixtureRepo(); repos.push(repo); + commit(repo, receipts(3, 'base'), 'results: 3 run receipts'); + const body = path.join(repo.root, 'body.md'); + expect(runBin(repo, 'gstack-ship-receipt', ['write', '--pr', '9', '--base', 'main', '--version', '1.0.0', '--session-kind', 'unattended', '--body', body]).status).toBe(EXIT.ok); + repo.git('checkout', '-q', 'main'); + write(repo.work, 'docs/README.md', '# docs moved on main\n'); + repo.git('add', '-A'); repo.git('commit', '-qm', 'main moves on'); repo.git('push', '-q', 'origin', 'main'); + repo.git('checkout', '-q', 'feature'); + repo.git('merge', '-q', '--no-edit', 'origin/main'); + const dry = runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--base', 'main', '--dry-run'], { TMPDIR: path.join(repo.root, 'no-such-dir') }); + expect([dry.status, dry.out.split('\n').find(l => l.startsWith('CARRY:'))]).toEqual([EXIT.ok, expect.stringMatching(/^CARRY: would carry/)]); + fs.rmSync(path.join(repo.work, '.git', 'gstack-patch-id'), { recursive: true, force: true }); + fs.writeFileSync(path.join(repo.work, '.git', 'gstack-patch-id'), 'not a directory\n'); + const out = runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--base', 'main', '--dry-run']); + expect([out.status, out.out.split('\n').find(l => l.startsWith('CARRY:'))]).toEqual([EXIT.refused, 'CARRY: refused (RECEIPT_CARRY_REFUSED)']); + expect(out.err).toContain('cannot create a temp file under'); + }); + + // Value: protects=a carry killed mid patch-id (preflight's 120 s timeout) does not leave product diffs in TMPDIR for good; + // fails_when=nothing removes a gstack-patch-id-* directory a killed run left; seam=TMPDIR, an aged directory; no_claim=none + test('a stale patch-id directory a killed run left is swept; a fresh one is kept', () => { + const repo = makeFixtureRepo(); repos.push(repo); + commit(repo, receipts(3, 'base'), 'results: 3 run receipts'); + const tmp = path.join(repo.work, '.git', 'gstack-patch-id'); fs.mkdirSync(tmp, { recursive: true }); + const stale = path.join(tmp, 'run-stale'); const fresh = path.join(tmp, 'run-fresh'); + for (const d of [stale, fresh]) { fs.mkdirSync(d); fs.writeFileSync(path.join(d, 'diff.patch'), 'diff\n'); } + const old = new Date(Date.now() - 2 * 60 * 60 * 1000); fs.utimesSync(stale, old, old); + const body = path.join(repo.root, 'body.md'); + expect(runBin(repo, 'gstack-ship-receipt', ['write', '--pr', '9', '--base', 'main', '--version', '1.0.0', '--session-kind', 'unattended', '--body', body]).status).toBe(EXIT.ok); + expect([fs.existsSync(stale), fs.existsSync(fresh)]).toEqual([false, true]); + }); +}); diff --git a/test/ship-rereview-triggers.test.ts b/test/ship-rereview-triggers.test.ts index 82cac144a..f06b65074 100644 --- a/test/ship-rereview-triggers.test.ts +++ b/test/ship-rereview-triggers.test.ts @@ -11,7 +11,7 @@ import { EXIT } from '../lib/headless-artifacts'; import { comparePath } from '../lib/ship-patch'; import { extractReceipt, placeReceiptFirst } from '../lib/ship-receipt'; import { cleanup, makeFixtureRepo, runBin, write, type FixtureRepo } from './helpers/pregate-fixture'; -import { lineOf, RELEASE, shipFixture, type ShipFx } from './helpers/ship-bin-fixture'; +import { lineOf, PREFLIGHT, RELEASE, shipFixture, type ShipFx } from './helpers/ship-bin-fixture'; setDefaultTimeout(90_000); const BIN = path.dirname(RELEASE); @@ -484,3 +484,44 @@ describe('test-only and audit-requested edits after the docs audit (GST-391, gbr expect([reset.status, reset.stdout.includes('ATTEMPT: counted (a second install refusal in this invocation)')]).toEqual([3, true]); }); }); + +describe('preflight carry route for an adjacent base edit (GST-442)', () => { + // Value: protects=/ship Step 1's carry check offers the scoped review when the base edited context next to a reviewed hunk, + // instead of treating the refusal as "run the full loop", and its dry run writes nothing; fails_when=preflight acts only on + // 'CARRY: would carry' or carry --dry-run writes the packet; seam=gstack-ship-receipt carry --dry-run; no_claim=the reviewer + test('preflight routes RECEIPT_CARRY_SCOPED_REVIEW to one scoped review, and the dry run writes no packet', () => { + const src = (edit: Record = {}) => Array.from({ length: 12 }, (_, i) => edit[i + 1] ?? `exports.v${i + 1} = ${i + 1};`).join('\n') + '\n'; + const f = fx({ 'lib/rows.js': src() }, { 'lib/rows.js': src({ 6: 'exports.v6 = 600;' }) }); + const reviewed = f.git('rev-parse', 'HEAD').stdout.trim(); + f.advanceBase({ 'lib/rows.js': src({ 5: 'exports.v5 = "five";' }) }); + mergeMain(f, { 'lib/rows.js': src({ 5: 'exports.v5 = "five";', 6: 'exports.v6 = 600;' }) }); + const head = f.git('rev-parse', 'HEAD').stdout.trim(); + const view = path.join(f.root, 'view.json'); + fs.writeFileSync(view, JSON.stringify({ body: placeReceiptFirst('## Summary\n\nProse.\n', { schema_version: 1, pr: 9, head: reviewed, base: 'main', queue_mode: 'stamp-at-merge' } as any), headRefOid: head })); + f.gh(`case "$1 $2" in "pr list") echo 9 ;; "pr view") cat '${view}' ;; *) exit 1 ;; esac`); + const r = f.run(PREFLIGHT, ['--base', 'main']); + expect(lineOf(r.stdout, 'CARRY')).toBe('CARRY: refused (RECEIPT_CARRY_SCOPED_REVIEW)'); + expect(r.stdout).toMatch(/^NEXT: carry route \(Step 1\.4\): .*fix what the base merge broke.*commit and push.*writes the packet.*one fix-check reviewer.*--scoped-review /m); + expect(fs.existsSync(path.join(f.work, '.git', 'gstack', `scoped-review-${head.slice(0, 12)}.md`))).toBe(false); + const wrote = f.run(path.join(BIN, 'gstack-ship-receipt'), ['carry', '--pr', '9', '--base', 'main']); + expect([wrote.status, fs.existsSync(path.join(f.work, '.git', 'gstack', `scoped-review-${head.slice(0, 12)}.md`))]).toEqual([EXIT.refused, true]); + }); + + // Value: protects=the dry run offers the scoped route only when nothing else would refuse the carry, so preflight never sends + // the agent through a commit, packet, reviewer and pregate that end in another refusal (adversarial-native#5, Step 11 + // round 2); fails_when=the dry run returns the scoped refusal before the receipt's other carry checks; seam=gh; no_claim=none + test('a dry run that the receipt would refuse anyway names that refusal, not the scoped route', () => { + const src = (edit: Record = {}) => Array.from({ length: 12 }, (_, i) => edit[i + 1] ?? `exports.v${i + 1} = ${i + 1};`).join('\n') + '\n'; + const f = fx({ 'lib/rows.js': src() }, { 'lib/rows.js': src({ 6: 'exports.v6 = 600;' }) }); + const reviewed = f.git('rev-parse', 'HEAD').stdout.trim(); + f.advanceBase({ 'lib/rows.js': src({ 5: 'exports.v5 = "five";' }) }); + mergeMain(f, { 'lib/rows.js': src({ 5: 'exports.v5 = "five";', 6: 'exports.v6 = 600;' }) }); + const head = f.git('rev-parse', 'HEAD').stdout.trim(); + const view = path.join(f.root, 'view.json'); + fs.writeFileSync(view, JSON.stringify({ body: placeReceiptFirst('## Summary\n\nProse.\n', { schema_version: 1, pr: 9, head: reviewed, base: 'main', queue_mode: 'stamp-at-merge', gstack_version: '1.0.0' } as any), headRefOid: head })); + f.gh(`case "$1 $2" in "pr list") echo 9 ;; "pr view") cat '${view}' ;; *) exit 1 ;; esac`); + const r = f.run(path.join(BIN, 'gstack-ship-receipt'), ['carry', '--pr', '9', '--base', 'main', '--dry-run']); + expect([r.status, lineOf(r.stdout, 'CARRY')]).toEqual([EXIT.refused, 'CARRY: refused (RECEIPT_CARRY_REFUSED)']); + }); +}); + diff --git a/test/size-ledger/ship/gst-430-notes-only-pass.json b/test/size-ledger/ship/gst-430-notes-only-pass.json new file mode 100644 index 000000000..ac715c577 --- /dev/null +++ b/test/size-ledger/ship/gst-430-notes-only-pass.json @@ -0,0 +1,4 @@ +{ + "bytes": -6, + "why": "GST-442/GST-444: Step 1.4 follows a scoped-review NEXT; Step 16 stage 2 and 3 route finish scoped-review STOP and --docs-requested with its --docs-result; trims in Steps 3, 12 and 16 keep the skill under its cap, and a Step 3 merge-conflict trim keeps the skeleton under maxSkeletonBytes with train 77 on main" +} diff --git a/test/train-gate.test.ts b/test/train-gate.test.ts index 32757a46d..17702d1f0 100644 --- a/test/train-gate.test.ts +++ b/test/train-gate.test.ts @@ -224,6 +224,23 @@ describe('prefix trees modulo stamps', () => { expect(stampFreeTree(git, 'f'.repeat(40), repo.work, policy)).toHaveProperty('error'); }); + // Value: protects=a gated-to-current diff git cannot produce refuses with its cause instead of reading as 'no changes' + // (GST-448); fails_when=buildReceipt treats a failed diff as an empty one or drops why it failed; seam=a missing blob; no_claim=none + test('a diff git cannot produce refuses the receipt with EVIDENCE_IDENTITY_UNKNOWN and git\'s reason', () => { + const policy = loadPolicyFromHead(repo.work).policy; + repo.git('checkout', '-q', '-B', 'lost-blob', sha.t1!); + fs.writeFileSync(path.join(repo.work, 'lost.txt'), 'a blob this test deletes from the object store\n'); + repo.git('add', '-A'); repo.git('commit', '-q', '-m', 'add a blob'); + const head = repo.git('rev-parse', 'HEAD'); + const blobId = repo.git('rev-parse', `${head}:lost.txt`); + const gitDir = repo.git('rev-parse', '--absolute-git-dir'); + fs.rmSync(path.join(gitDir, 'objects', blobId.slice(0, 2), blobId.slice(2))); + const r = buildReceipt({ repoRoot: repo.work, gated: sha.t1!, current: head, policy, policyLabel: 'fixture' }) as { error?: string; code?: string }; + expect(r.code).toBe('EVIDENCE_IDENTITY_UNKNOWN'); + expect(r.error).toMatch(/^git diff [0-9a-f]{12} [0-9a-f]{12} failed \(.+\), so the changed files cannot be classified/); + repo.git('checkout', '-q', '-f', 'squash'); + }); + // #3512: a stamp path carrying a tentative `*_KEY_VERSION = ''` (gstack's own convention since #3511) digests the // same before and after the slot stamp moves it, so train-backstop finds the record that covers main; a base-version mention // on a line the base already carried digests the same at the prefix (where it equals VERSION) and on the stamped main.