diff --git a/bin/gstack-diff-scope b/bin/gstack-diff-scope index 8671aa7fd..afdddbb06 100755 --- a/bin/gstack-diff-scope +++ b/bin/gstack-diff-scope @@ -287,6 +287,7 @@ for f in ${FILES_LIST[@]+"${FILES_LIST[@]}"}; do Gemfile|Gemfile.lock) m_config=true ;; *.yml|*.yaml) m_config=true ;; .github/*) m_config=true ;; + .gstack/*) m_config=true ;; requirements.txt|pyproject.toml|go.mod|Cargo.toml|composer.json) m_config=true ;; pubspec.lock|*/pubspec.lock) m_config=true ;; esac diff --git a/changelog.d/gst-1276-bounded-post-review-edits.md b/changelog.d/gst-1276-bounded-post-review-edits.md new file mode 100644 index 000000000..3a825b191 --- /dev/null +++ b/changelog.d/gst-1276-bounded-post-review-edits.md @@ -0,0 +1,13 @@ +### Fixed + +- **A required edit after review binds no longer forces a third review round (GST-1276, GST-1311).** Four instances in two days each cost a Captain grant (#3970, #3985, gstack-evals#203, gbrain#8523 and #8525). Now `gstack-ship-release finish` proves a post-review edit's kind from the diff and sends it to the one scoped reader that test-code edits already take, which isn't a review round (Garry, 2026-11-04). The kinds are: a docs-audit fix to `docs/*.md`, `AGENTS.md` or `CLAUDE.md` whose bytes are the ones the post-audit snapshot recorded; a `.gstack/pregate.json` change that only adds lanes (pregate's own `headPolicyDiff`, with every other top-level key unchanged); and a file matching the new `bookkeeping` ship-policy key. Each needs `--reviewed-base`, at most 20 changed lines, readable text and no gate-scan tag. `ROUTE:` prints `bounded=`. The packet carries the targeted brief for every class, and the reader's clean record must carry the packet's `packet_sha256`. A candidate that fails a proof stays behavior and prints `ROUTE_BOUNDED_REFUSED: (fix: …)`. Gate-path prose (`review/specialists/targeted.md`, skill templates) never qualifies. +- **A test literal changed after review takes the scoped reader, not behavior (GST-1276, #3984).** This covers a removed line swapped one for one with a line that differs only inside quoted strings and has no assertion or test call, such as a fixture value the secrets scan flagged. A changed expected value or a deleted assertion still stops. +- **`gstack-diff-scope` reads a `.gstack/`-only diff as config** instead of `SCOPE_ERROR=unmatched`, and `.gstack/always-run.json` is now a gate path, so its review depth stays full. + +### Added + +- **`bookkeeping` ship-policy key**: root-anchored globs of `.tsv`, `.csv` or `.txt` tables, never under `.gstack/` or `.github/`, read from the base only. It needs `min_gstack_version`. See docs/ship-policy.md#bookkeeping. + +### Changed + +- Finish's scoped-review STOP now says "one scoped reader (not a review round) reads", and its record template includes `packet_sha256`. See docs/review-policy.md#bounded-post-review-edits and docs/troubleshooting.md#route-bounded-refused. diff --git a/docs/review-policy.md b/docs/review-policy.md index 35b62cda2..7413792c4 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -648,6 +648,49 @@ refuses a `review-fix-check` row without boolean `completed` and `converged`, before it consumes the start token, because without them the row reads `UNVERIFIED` and finish cannot count it. + +### Bounded post-review edits (GST-1276, GST-1311) + +Some edits are required after the review binds: the Step 14.5 docs audit +corrects stale policy text, pregate asks for a lane mapping, and a verify +step renames rows in a size baseline. Finish proves each one's kind from +the diff and sends it to one scoped reader instead of Steps 5–11.5. That +read is not a review round under R51 (Garry, 2026-11-04: "Do it."), so it +needs no grant. It is the same stage 2.1 reader that test code changed +after review already takes. A path is bounded when finish runs with +`--reviewed-base`, the path changed at most 20 lines since review, its +diff is readable text, the gate scan tags none of its hunks, and it is one +of these kinds: + +- **audit prose**: `docs/*.md` or the root `AGENTS.md` or `CLAUDE.md`, + never a gate path, whose bytes now are the ones the post-audit snapshot + (`--docs-post`) recorded. An edit made after the snapshot isn't the + audit's. +- **lanes**: `.gstack/pregate.json` changes that pregate's own + `headPolicyDiff` reads as added tests only, over the reviewed copy. A + removed mapping, a new `[]` declaration or any other key stops. +- **bookkeeping**: a path matching a `bookkeeping` glob in the base + branch's ship policy ([ship-policy.md#bookkeeping](ship-policy.md#bookkeeping)), + first touched after review or not, a `.txt` table included, never a gate + path or a path under `.gstack/` or `.github/`. + +`ROUTE:` prints `bounded=`, and finish writes the packet with every +bounded hunk, a reading note per kind and the targeted brief for every +class. The reader's clean record must carry the packet's `packet_sha256`, +which the STOP line prints. A candidate that fails a proof stays behavior, +and finish prints `ROUTE_BOUNDED_REFUSED: (fix: …)` +([troubleshooting](troubleshooting.md#route-bounded-refused)). A lane +change whose other top-level keys differ from the reviewed copy stops too. A +targeted record read before the edits does not carry to the new head: +receipt write still asks for the targeted reader on HEAD. A test file whose only removed lines are swapped +one for one with lines that differ inside plain quoted strings, in +statements with no assertion or test call, takes the plain test route (the +scoped reader) instead of behavior. A changed expected value in the +assertion's own statement (a multi-line one too) or in a template literal, +code inside an interpolated or prefixed string, or a deleted assertion still +stops. A value in a case table that a later test reads takes the scoped +reader. + The receipt's summary line prints `passes= blocking_found= deferred=`, which is what a scorecard tracks (target: a median of 1 pass per PR). A receipt carried across a base merge or a test-only fix diff --git a/docs/ship-policy.md b/docs/ship-policy.md index 346b868dd..5a54d736b 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -67,6 +67,7 @@ refuses the file (`POLICY_GSTACK_TOO_OLD`). `validate` ends with | `train` | `{ gate_workflow, free_workflow, full_gate_check, record_artifact, max_size, fill_hours, screen_selection, trial_reservation_usd, land? }`, `{ gates, land, max_size, fill_hours }` or `null` | `null` | Turns on [merge trains](#run-a-merge-train), read from the base revision only. Every field is required and an unknown field is `POLICY_INVALID`: `max_size` 1 to 6, `fill_hours` above 0 and at most 24, `screen_selection` `gstack-paid` \| `none`, `trial_reservation_usd` above 0 and at most 100. A block with `gates` ([any repository](#train-gates)) takes `max_size` 1 to 5 and needs `min_gstack_version` at least `TRAIN_GATES_KEY_VERSION`. `null` keeps the per-PR queue exactly as it is. Needs `min_gstack_version`. | | `merge` | `train` \| `direct` | `train` | `direct` lands each READY PR at its own green head with [`land --pr --pin `](#land-direct): no train and no stamp commit, and `train build` refuses. A PR then leaves VERSION, the `package.json` version and the CHANGELOG at its merge base, and the release sets them. Needs `changelog_fragments`, and the stamping `land --pr ` refuses under it. `train` keeps merge trains. Read from the base revision only. Needs `min_gstack_version`. | | `land` | `{ merged_checks }` or `null` | `null` | Under `merge: direct`, the shell commands (1 to 16, one line of at most 512 characters each, 15 minutes each) [`land --pin`](#land-direct) runs on the pin merged onto the base tip when the base moved past the pin's merge base; it lands only when every one exits 0. They are repo commands, so land runs them only with `--allow-repo-commands`, never under `--dry-run`. `null` lands a stale base untested with a `STALE_BASE: warn` line. Read from the base revision only. Needs `min_gstack_version`. | +| `bookkeeping` | glob list | `[]` | Root-anchored globs of bookkeeping tables a verify or ratchet step rewrites (size baselines, serial lists, limits), each ending in `.tsv`, `.csv` or `.txt` and never under `.gstack/` or `.github/` (`["scripts/*-baseline.tsv"]`). A matching file changed at most 20 lines after review binds, first touched or not, takes finish's [one scoped reader](review-policy.md#bounded-post-review-edits) instead of Steps 5–11.5; the reader still reads every changed row. Read from the base revision only. Needs `min_gstack_version`. | | `base_red` | `{ setup, select, timeout_sec }` or `null` | `null` | How `/ship` Step 5 proves a failure is the base branch's own before pausing on it; see [base_red](#base-red). Read from the base revision only. Needs `min_gstack_version`. | | `changelog_fragments` | `{ dir, required_checks? }` or `null` | `null` | PRs carry unversioned [CHANGELOG fragments](#changelog-fragments) in `dir` (one flat, repo-relative directory, no `..`) and the merge step ([`land`](#land)) assigns the version (under `merge: direct`, the [release PR](#release-stamp) does). `required_checks` (up to 50 distinct names) are the checks [`train land`](#stamped-trains-under-changelog_fragments-train-land) requires present and green on a stamp commit. `null` keeps `[Unreleased]` sections and per-PR stamps. Needs `min_gstack_version`. | diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index b64bbd1e6..1e595db62 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -3143,6 +3143,29 @@ table above and end each error line with a code in parentheses. The policy file is described in [docs/ship-policy.md](ship-policy.md); the queue flow in [docs/ship-policy.md#stamp-at-merge](ship-policy.md#stamp-at-merge). + +### `ROUTE_BOUNDED_REFUSED: (fix: …)` + +**Meaning.** `gstack-ship-release finish` found a post-review edit of a +bounded kind (audit prose, a `.gstack/pregate.json` lane change, or a +declared bookkeeping file) that failed one of its proofs, so the path stays +behavior and finish stops for Steps 5–11.5 +([review-policy.md#bounded-post-review-edits](review-policy.md#bounded-post-review-edits)). +The reason names the failed proof: `needs --reviewed-base`, `changed +lines since review (at most 20)`, `is binary or unreadable`, `carries a +hunk the gate scan tagged`, `could not be gate-scanned`, `is not a doc the Step 14.5 audit snapshot +recorded`, `is not the bytes the post-audit snapshot recorded (edited after +the audit)`, `does not parse at both the reviewed and the current tree`, +`loosens the reviewed policy: `, `changes ; a lanes edit changes +only dependencies`, `adds no lane`, or `is a gate path or under .gstack/ or +.github/, never bookkeeping`. + +**Fix.** Do what the line's `fix:` says. Rerun finish with the merge base +saved with the reviewed tree. Restore the audited bytes, or the mapping a +lane change removed (a `[]` declaration lands on the base first). Or take +Steps 5–11.5 for the edit. Never shrink an edit to get under the cap by +moving its lines into another file. + ### `policy source unavailable — fix: git fetch origin --depth=1 (POLICY_SOURCE_UNAVAILABLE)` @@ -6412,7 +6435,10 @@ on the new head. `gstack-ship-release finish` stops the same way with write that record beside the packet, outside the worktree, so it does not change 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 +reviewer checks that each test still tests what it did. It stops this way for +[bounded post-review edits](review-policy.md#bounded-post-review-edits) too +(`bounded=N`): one scoped reader, not a review round, reads the packet, and +its clean record must also carry the `packet_sha256` the STOP line prints. 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. diff --git a/lib/review-depth.ts b/lib/review-depth.ts index a1379835c..8ed041ff9 100644 --- a/lib/review-depth.ts +++ b/lib/review-depth.ts @@ -57,7 +57,7 @@ const HARNESS_PINS = /^scripts\/harness-pins\//; /** Gate paths, the review-depth machinery included: an edit to how much review runs gets the most review. */ const GATE_PATH = [ /(^|\/)\.github\/workflows\//, SECRET_PATH, /state-root/, /(^|\/)(pregate|gate|ship-receipt|ship-policy)[^/]*\.(ts|sh|json)$/, /(^|\/)bin\/gstack-(pregate|gate|ship-receipt|verify-gate|ci-gate)$/, - /^lib\/review-(depth|collect|scope|targeted|risk|exclude)\.ts$/, /^lib\/redact-(engine|patterns)\.ts$/, /^lib\/default-base\.ts$/, /^review\/specialists\/targeted\.md$/, /^bin\/gstack-review-plan$/, /^lib\/ship-queue[^/]*\.ts$/, /^lib\/regen\.ts$/, /^\.gstack\/generated\.json$/, /^ship\/sections\/(review-army|adversarial)\.md\.tmpl$/, /^scripts\/resolvers\/review-army\.ts$/, + /^lib\/review-(depth|collect|scope|targeted|risk|exclude)\.ts$/, /^lib\/redact-(engine|patterns)\.ts$/, /^lib\/default-base\.ts$/, /^review\/specialists\/targeted\.md$/, /^bin\/gstack-review-plan$/, /^lib\/ship-queue[^/]*\.ts$/, /^lib\/regen\.ts$/, /^\.gstack\/(generated|always-run)\.json$/, /^ship\/sections\/(review-army|adversarial)\.md\.tmpl$/, /^scripts\/resolvers\/review-army\.ts$/, ]; /** The path rules lib/review-exclude.ts keeps a matched file for: a gate path, agent prose, or a data-shaped config file. */ diff --git a/lib/ship-patch.ts b/lib/ship-patch.ts index 447c1a548..d3e85840f 100644 --- a/lib/ship-patch.ts +++ b/lib/ship-patch.ts @@ -20,11 +20,18 @@ * characters cannot widen or empty a comparison. */ import { spawnSync } from 'node:child_process'; +import { createHash } from 'node:crypto'; import * as fs from 'node:fs'; +import * as path from 'node:path'; import { EMPTY_PROJECT_CONFIG, scanCandidate } from './gate-diff/index'; import { parseUnifiedDiff } from './gate-diff/git'; import { CARRY_TEST_CODE_GLOBS } from './ship-receipt'; import { matchesAny } from './ship-policy'; +import { isGatePath } from './review-depth'; +import { TARGETED_BRIEF_FILE, TARGETED_CLASSES, targetedBrief } from './review-targeted'; +import { parseConfig, PREGATE_CONFIG_PATH } from './pregate/config'; +import { headPolicyDiff } from './pregate/head-policy'; +import { CHECK_IDS } from './pregate/index'; export interface PatchSides { reviewedBase: string; reviewed: string; currentBase: string; current: string } export type PathVerdict = 'same' | 'context' | 'changed'; @@ -135,24 +142,36 @@ export const BASE_SECTION_CEILING = 2000; * cannot be shown all of it, so the caller sends it to the full loop. The base's +/- lines, already merged at base, are * shown but do not count toward the cap (GST-1226) until that section passes BASE_SECTION_CEILING lines. */ -export function contextPacket(repoRoot: string, s: PatchSides, paths: readonly string[], tests: readonly string[] = []): { text: string; over: string[] } { +export function contextPacket(repoRoot: string, s: PatchSides, paths: readonly string[], tests: readonly string[] = [], bounded: Readonly> = {}): { text: string; over: string[]; sha256: string } { const over = new Set(); let current = ''; const fence = (label: string, body: string, counts: (line: string) => boolean = () => true) => { if (body.split('\n').filter(counts).length > PACKET_SECTION_CAP) over.add(current); return `${label}\n\`\`\`diff\n${body.trimEnd()}\n\`\`\`\n`; }; - const parts = [`# Scoped review: base context moved next to the reviewed hunks\n\nreviewed ${s.reviewed.slice(0, 12)} against ${s.reviewedBase.slice(0, 12)}; current ${s.current.slice(0, 12)} against ${s.currentBase.slice(0, 12)}. The branch's +/- lines are unchanged; read whether the merge kept their meaning next to the base's edit. Report clean, or findings at the blocking floor.\n`]; + const late = Object.keys(bounded).sort(); + const sides = `reviewed ${s.reviewed.slice(0, 12)} against ${s.reviewedBase.slice(0, 12)}; current ${s.current.slice(0, 12)} against ${s.currentBase.slice(0, 12)}.`; + const parts = [paths.length || !late.length + ? `# Scoped review: base context moved next to the reviewed hunks\n\n${sides} The branch's +/- lines are unchanged; read whether the merge kept their meaning next to the base's edit. Report clean, or findings at the blocking floor.\n` + : `# Scoped review: edits that landed after the review bound\n\n${sides}\n`]; + if (late.length) parts.push(`Bounded edits since review in ${late.map(p => `${p} (${bounded[p]})`).join(', ')}: each landed after the review bound, and its kind was proven from the diff; this one read replaces another review round (docs/review-policy.md#bounded-post-review-edits). Read every line under "Changed since review" as gate code. Audit prose is policy text agents follow: flag any change to what a gate, cap, reviewer, approval or exception does. A lanes edit maps files to tests: flag a glob that marks files covered by tests that do not exercise them. A bookkeeping row is an exemption or a limit: flag a row that moves an exemption onto code it should not cover, or raises a limit without a reason in the diff. Read them through the targeted brief at the end for money, gate, auth and secret wording. Report clean, or findings at the blocking floor.\n`); if (tests.length) parts.push(`Test code changed since review in ${tests.join(', ')}: read whether each test still checks what it did (no weakened or bypassed assertion, early return, new exemption, skip or mock). Report clean, or findings at the blocking floor.\n`); - for (const rel of [...paths, ...tests]) { + for (const rel of [...paths, ...tests, ...late]) { current = rel; const r = hunkTexts(pathDiff(repoRoot, s.reviewedBase, s.reviewed, rel, 3) ?? ''); const c = hunkTexts(pathDiff(repoRoot, s.currentBase, s.current, rel, 3) ?? ''); const b = hunkTexts(pathDiff(repoRoot, s.reviewedBase, s.currentBase, rel, 3) ?? '').join(''); - parts.push(`## ${rel}\n`, fence('Reviewed hunks', r.filter(h => !c.includes(h)).join('')), fence('Current hunks', c.filter(h => !r.includes(h)).join('')), + parts.push(`## ${rel}\n`, ...(rel in bounded ? [fence('Changed since review', pathDiff(repoRoot, s.reviewed, s.current, rel, 3) ?? '')] : []), fence('Reviewed hunks', r.filter(h => !c.includes(h)).join('')), fence('Current hunks', c.filter(h => !r.includes(h)).join('')), fence("The base's change", b, b.split('\n').length > BASE_SECTION_CEILING ? undefined : l => !/^[+-]/.test(l))); } - return { text: parts.join('\n'), over: [...over] }; + if (late.length) parts.push(`## Targeted brief (${TARGETED_CLASSES.join(', ')})\n\n${packetBrief()}\n`); + const text = parts.join('\n'); + return { text, over: [...over], sha256: createHash('sha256').update(text).digest('hex') }; +} + +/** The targeted brief with every class's block, as a bounded packet carries it; receipt write checks a packet holds it. */ +export function packetBrief(): string { + return targetedBrief(fs.readFileSync(TARGETED_BRIEF_FILE, 'utf8'), TARGETED_CLASSES); } /** @@ -167,25 +186,120 @@ export function isTestCode(rel: string): boolean { /** * Test paths whose change from..to needs review: any removed or changed line (a weakened assertion or a changed - * expected value carries no gate tag), a gate edit lib/gate-diff tags (a skipped test, a snapshot, a relaxed gate - * value), or a diff the scan cannot read. The rest still get the scoped fix-check of their test hunks (lib/ship-release.ts route). + * expected value carries no gate tag) except a literal-only rewrite, a gate edit lib/gate-diff tags (a skipped test, a + * snapshot, a relaxed gate value), or a diff the scan cannot read. The rest still get the scoped reader of their test + * hunks (lib/ship-release.ts route). */ export function gateEditedPaths(repoRoot: string, from: string, to: string, paths: readonly string[]): Set | null { - if (!paths.length) return new Set(); - const diff = git(repoRoot, ['--literal-pathspecs', 'diff', '--no-color', '--no-ext-diff', '--no-textconv', '--no-renames', from, to, '--', ...paths]); - if (diff === null) return null; - const files = parseUnifiedDiff(diff); - const scan = scanCandidate({ files, commits: [], project: EMPTY_PROJECT_CONFIG, specIsTestPath: true, candidate: 'route', artifact: '' }); + const tagged = gateTaggedPaths(repoRoot, from, to, paths); + if (!tagged) return null; const removed = paths.filter(p => { const d = pathDiff(repoRoot, from, to, p, 0); if (d === null || /^(Binary files |GIT binary patch)/m.test(d)) return true; const at = d.search(/^@@ /m); - return at >= 0 && d.slice(at).split('\n').some(l => l.startsWith('-')); + return at >= 0 && d.slice(at).split('\n').some(l => l.startsWith('-')) && !literalOnly(d, git(repoRoot, ['show', `${to}:${p}`])); }); - return new Set([...scan.hunks.filter(h => h.tags.length > 0).map(h => h.path), ...scan.unsupportedPaths.map(u => u.path), ...removed]); + return new Set([...tagged, ...removed]); } -export interface ScopedReview { paths: string[]; reviewer: string; verdict: 'clean'; head?: string; tree?: string } +/** Paths with a hunk lib/gate-diff tags, or a diff its scan cannot read; null when git cannot diff them. */ +function gateTaggedPaths(repoRoot: string, from: string, to: string, paths: readonly string[]): Set | null { + if (!paths.length) return new Set(); + const diff = git(repoRoot, ['--literal-pathspecs', 'diff', '--no-color', '--no-ext-diff', '--no-textconv', '--no-renames', from, to, '--', ...paths]); + if (diff === null) return null; + const scan = scanCandidate({ files: parseUnifiedDiff(diff), commits: [], project: EMPTY_PROJECT_CONFIG, specIsTestPath: true, candidate: 'route', artifact: '' }); + return new Set([...scan.hunks.filter(h => h.tags.length > 0).map(h => h.path), ...scan.unsupportedPaths.map(u => u.path)]); +} + +const CHECK_CALL = /\b(expect|assert\w*|should|skip|only|todo|each|test|it|describe|bench)\b|\.to[A-Z]/; +const QUOTED = /(['"])(?:\\.|(?!\1)[^\\\n])*\1/g; +const CODE_IN_STRING = /#\{|\$\{|\b[fFrRbB]{1,2}['"]/; +const STATEMENT_END = /(;|\}|=>\s*\{|\)\s*\{)\s*$/; +/** + * A zero-context diff whose every hunk swaps lines one for one, each pair the same once quoted strings are emptied, in + * statements with no assertion or test call (the changed line and the lines above it back to the previous statement's end in + * `after`, blank lines included, outside any template literal): a test literal changed (a fixture value the secrets scan flagged, GST-1276 instance 5), never a check or an + * expected value. Template literals, interpolated strings and prefixed (f, r, b) strings do not count, since they can hold code. + */ +function literalOnly(diff: string, after: string | null): boolean { + const hunks = zeroHunks(diff); + if (!hunks?.length || after === null) return false; + const text = after.split('\n'); + const statement = (n: number) => { const out = [text[n - 1] ?? '']; for (let k = n - 2; k >= 0 && !STATEMENT_END.test(text[k]!); k--) out.push(text[k]!); return out; }; + const inTemplate = (n: number) => (text.slice(0, n - 1).join('\n').match(/`/g)?.length ?? 0) % 2 === 1; + return hunks.every(h => { + const lines = h.body.split('\n'); + const minus = lines.filter(l => l.startsWith('-')).map(l => l.slice(1)); + const plus = lines.filter(l => l.startsWith('+')).map(l => l.slice(1)); + const bare = (l: string) => l.replace(QUOTED, '$1$1'); + return minus.length === plus.length && minus.every((m, i) => m !== plus[i] && !CODE_IN_STRING.test(m + plus[i]) && bare(m) === bare(plus[i]!) + && !CHECK_CALL.test(bare(m)) && !inTemplate(h.newStart + i) && !statement(h.newStart + i).some(l => CHECK_CALL.test(l.replace(QUOTED, '$1$1')))); + }); +} + +export type BoundedKind = 'audit prose' | 'lanes' | 'bookkeeping'; +/** The most lines (added plus removed) one bounded path may change since review. */ +export const BOUNDED_LINES = 20; +export interface BoundedInput { + sides?: PatchSides; reviewed: string; now: string; base: string; candidates: readonly string[]; + /** The Step 14.5 post-audit snapshot's `hashes` (path -> blob), or null when finish could not read it. */ + audited: Readonly> | null; + /** origin/'s ship-policy `bookkeeping` globs. */ + bookkeeping: readonly string[]; +} +export interface BoundedRefusal { path: string; reason: string; fix: string } + +/** Policy prose an agent reads that a docs audit may correct: docs/*.md and the root AGENTS.md or CLAUDE.md, never a gate path. */ +export const auditProse = (p: string) => (/^docs\/[^/]+\.md$/.test(p) || /^(AGENTS|CLAUDE)\.md$/.test(p)) && !isGatePath(p); + +/** + * The post-review edits finish may send to one scoped reader instead of Steps 5–11.5 (GST-1276, GST-1311), each kind proven + * from the diff: audit prose whose bytes now are the ones the post-audit snapshot recorded; a .gstack/pregate.json change that + * only adds lanes over the reviewed copy (pregate's own headPolicyDiff); a file the base policy declares bookkeeping. Every + * kind needs both base sides, at most BOUNDED_LINES changed lines, readable text and no gate-scan tag. A candidate of a kind + * that fails a proof is refused with the reason and the fix; any other path is not a candidate. + */ +export function boundedPaths(repoRoot: string, i: BoundedInput): { kinds: Record; refused: BoundedRefusal[] } { + const kinds: Record = {}; + const refused: BoundedRefusal[] = []; + const full = 'take Steps 5–11.5'; + const kindOf = (p: string): BoundedKind | null => auditProse(p) ? 'audit prose' : p === PREGATE_CONFIG_PATH ? 'lanes' : matchesAny([...i.bookkeeping], p) ? 'bookkeeping' : null; + const candidates = i.candidates.filter(p => kindOf(p)); + const tagged = candidates.length ? gateTaggedPaths(repoRoot, i.reviewed, i.now, candidates) : new Set(); + const blob = (tree: string, p: string) => git(repoRoot, ['rev-parse', '--verify', '--quiet', `${tree}:${p}`])?.trim() ?? null; + for (const p of candidates) { + const kind = kindOf(p)!; + const refuse = (reason: string, fix: string) => refused.push({ path: p, reason, fix }); + if (!i.sides) { refuse('needs --reviewed-base', 'rerun finish with --reviewed-base '); continue; } + const stat = git(repoRoot, ['--literal-pathspecs', 'diff', '--numstat', '--no-renames', i.reviewed, i.now, '--', p])?.trim().split(/\s+/) ?? []; + const lines = Number(stat[0]) + Number(stat[1]); + if (!Number.isFinite(lines)) { refuse('is binary or unreadable', full); continue; } + if (lines > BOUNDED_LINES) { refuse(`changed ${lines} lines since review (at most ${BOUNDED_LINES})`, full); continue; } + if (!tagged || tagged.has(p)) { refuse(tagged ? 'carries a hunk the gate scan tagged' : 'could not be gate-scanned', full); continue; } + if (kind === 'bookkeeping' && (isGatePath(p) || /^\.(gstack|github)\//.test(p))) { refuse('is a gate path or under .gstack/ or .github/, never bookkeeping', full); continue; } + if (kind === 'audit prose') { + const recorded = i.audited?.[p]; + if (!recorded) { refuse('is not a doc the Step 14.5 audit snapshot recorded', 'include it in the audit (--docs), or ' + full); continue; } + if (blob(i.now, p) !== recorded) { refuse('is not the bytes the post-audit snapshot recorded (edited after the audit)', 'restore the audited bytes, or ' + full); continue; } + } + if (kind === 'lanes') { + const [refText, headText] = [i.reviewed, i.now].map(tree => git(repoRoot, ['show', `${tree}:${p}`])); + const [ref, head] = [refText, headText].map(t => t === null ? null : parseConfig(t, CHECK_IDS)); + if (!ref || !head || ref.errors.length || head.errors.length) { refuse('does not parse at both the reviewed and the current tree', full); continue; } + const canon = (v: unknown): string => Array.isArray(v) ? `[${v.map(canon)}]` : v && typeof v === 'object' ? `{${Object.keys(v).sort().map(k => `${JSON.stringify(k)}:${canon((v as Record)[k])}`)}}` : JSON.stringify(v); + const [was, now] = [refText!, headText!].map(t => JSON.parse(t) as Record); + const moved = [...new Set([...Object.keys(was), ...Object.keys(now)])].find(k => k !== 'dependencies' && canon(was[k]) !== canon(now[k])); + if (moved) { refuse(`changes ${moved}; a lanes edit changes only dependencies`, full); continue; } + const d = headPolicyDiff(ref.config, head.config, i.base); + if (d.loosens.length) { refuse(`loosens the reviewed policy: ${d.loosens[0]}`, 'restore what it removed (a [] declaration lands on the base first), or ' + full); continue; } + if (!Object.keys(d.adds).length) { refuse('adds no lane', full); continue; } + } + kinds[p] = kind; + } + return { kinds, refused }; +} + +export interface ScopedReview { paths: string[]; reviewer: string; verdict: 'clean'; head?: string; tree?: string; packet_sha256?: string } /** A clean scoped review that covers every context path at this head or tree, or why it does not stand. */ export function readScopedReview(file: string, bind: { head?: string; tree?: string }, paths: readonly string[]): { ok: true; review: ScopedReview } | { ok: false; message: string } { @@ -200,5 +314,5 @@ export function readScopedReview(file: string, bind: { head?: string; tree?: str const covered = Array.isArray(v.paths) ? v.paths.filter((p: unknown) => typeof p === 'string') : []; const missing = paths.filter(p => !covered.includes(p)); if (missing.length) return { ok: false, message: `the scoped review does not cover ${missing.slice(0, 5).join(', ')}` }; - return { ok: true, review: { paths: covered, reviewer: v.reviewer.trim(), verdict: 'clean', ...(v.head ? { head: v.head } : {}), ...(v.tree ? { tree: v.tree } : {}) } }; + return { ok: true, review: { paths: covered, reviewer: v.reviewer.trim(), verdict: 'clean', ...(v.head ? { head: v.head } : {}), ...(v.tree ? { tree: v.tree } : {}), ...(typeof v.packet_sha256 === 'string' ? { packet_sha256: v.packet_sha256 } : {}) } }; } diff --git a/lib/ship-policy-bookkeeping.ts b/lib/ship-policy-bookkeeping.ts new file mode 100644 index 000000000..021143689 --- /dev/null +++ b/lib/ship-policy-bookkeeping.ts @@ -0,0 +1,23 @@ +/** + * The `bookkeeping` policy key (GST-1311): tables a verify or ratchet step rewrites after review (size baselines, serial + * lists, limits). Their small edits take /ship finish's scoped reader instead of another review round + * (docs/ship-policy.md#bookkeeping). Split out of ship-policy.ts, with the shared glob check, to keep that module under the + * module-size ratchet. The two files import only functions from each other, so either may load first. + */ +import { containedRelative, globToRegExp } from './ship-policy'; +const TABLE = /\.(tsv|csv|txt)$/i; + +/** Why a policy glob is refused: it escapes the repository or does not compile (every glob-list key, bookkeeping included). */ +export function globError(g: string): string | null { + const why = containedRelative(g); + if (why) return why; + try { globToRegExp(g); } catch (e: any) { return e.message; } + return null; +} + +/** Why a bookkeeping glob is refused: it could match code or gstack's own policy files. */ +export function bookkeepingGlobError(g: string): string | null { + if (!TABLE.test(g)) return 'must end in .tsv, .csv or .txt (a bookkeeping table, never code)'; + if (/^\.(gstack|github)\//.test(g)) return 'must not name a path under .gstack/ or .github/'; + return null; +} diff --git a/lib/ship-policy-keys.ts b/lib/ship-policy-keys.ts index 1eb1d8e42..6a19e469f 100644 --- a/lib/ship-policy-keys.ts +++ b/lib/ship-policy-keys.ts @@ -45,6 +45,7 @@ export const REVIEW_POLICY_KEY_VERSION = '1.94.30.0'; export const MERGE_KEY_VERSION = '1.95.0.0'; export const LAND_KEY_VERSION = '1.96.0.0'; export const READERS_KEY_VERSION = '1.96.0.0'; +export const BOOKKEEPING_KEY_VERSION = '1.96.0.0'; export const POLICY_KEY_INTRODUCED: Record = { bump: '1.91.73.0', queue_mode: '1.91.73.0', release_tool: '1.91.73.0', release_outputs: '1.91.73.0', stamp_paths: '1.91.73.0', changelog: '1.91.73.0', mirror: '1.91.73.0', history: '1.91.73.0', preregistration_shas: '1.91.73.0', stamp_sensitive_gates: '1.91.73.0', @@ -54,4 +55,5 @@ export const POLICY_KEY_INTRODUCED: Record = { changelog_fragments: CHANGELOG_FRAGMENTS_KEY_VERSION, base_red: BASE_RED_KEY_VERSION, review_exclude_paths: REVIEW_EXCLUDE_KEY_VERSION, review: REVIEW_POLICY_KEY_VERSION, release_gate: RELEASE_GATE_KEY_VERSION, stamp_placeholders: STAMP_PLACEHOLDERS_KEY_VERSION, merge: MERGE_KEY_VERSION, land: LAND_KEY_VERSION, readers: READERS_KEY_VERSION, + bookkeeping: BOOKKEEPING_KEY_VERSION, }; diff --git a/lib/ship-policy.ts b/lib/ship-policy.ts index 7dd4d2688..f9fc428c4 100644 --- a/lib/ship-policy.ts +++ b/lib/ship-policy.ts @@ -24,6 +24,7 @@ import { READERS_EXPLANATION, readersErrors, type ReadersPolicy } from './ship-p import { cmpVersion, parseVersion } from './version-source'; import { POLICY_COMPAT_FLOOR, BUMP_NEVER_KEY_VERSION, TRAIN_GATES_KEY_VERSION, POLICY_KEY_INTRODUCED } from './ship-policy-keys'; import { placeholderErrors, templateErrors } from './ship-policy-stamps'; +import { bookkeepingGlobError, globError } from './ship-policy-bookkeeping'; export { VERSION_SLOT } from './ship-policy-stamps'; export const SHIP_POLICY_PATH = '.gstack/ship-policy.json'; @@ -93,6 +94,7 @@ export interface ShipPolicy { base_red: BaseRedPolicy | null; /** The release gate on the base branch (docs/ship-policy.md#release-gate); null keeps the per-push main backstop. */ release_gate: ReleaseGatePolicy | null; + /** Bookkeeping tables (`.tsv`, `.csv`, `.txt` globs) whose small post-review edits take /ship finish's scoped reader (docs/ship-policy.md#bookkeeping); read from the base only. */ bookkeeping: string[]; /** `direct` lands each READY PR at its own green head (`land --pin`, docs/ship-policy.md#land-direct); `train` keeps merge trains. */ merge: 'direct' | 'train'; /** What `land --pin` runs on the pin merged onto a base that moved past its merge base (lib/land-merged-checks.ts); null lands a stale base untested with a STALE_BASE: warn line. */ land: LandPolicy | null; } @@ -185,7 +187,7 @@ export const DEFAULT_POLICY: ShipPolicy = { min_gstack_version: null, train: null, docs: null, - base_red: null, merge: 'train', land: null, + base_red: null, merge: 'train', land: null, bookkeeping: [], release_gate: null, readers: { forbid_fetch: [], forbid_paths: [], brief_note: null }, }; @@ -223,6 +225,7 @@ export const POLICY_EXPLANATIONS: Record = { base_red: '{ setup, select, timeout_sec } or null: how /ship Step 5 proves a failure is the base branch\'s own (docs/ship-policy.md#base-red). gstack-base-red checks out the base head and the PR head in scratch worktrees, runs setup (dependency install, or null), then select with the failing test files in its {files} slot; only the same failures in both, in files the PR does not touch, are base-red, and /ship then pauses before READY instead of failing. Read from the base branch only. null keeps every failure in-branch.', release_gate: RELEASE_GATE_EXPLANATION, land: LAND_EXPLANATION, + bookkeeping: 'Root-anchored globs of bookkeeping tables, each ending in .tsv, .csv or .txt and never under .gstack/ or .github/, for example ["scripts/*-baseline.tsv"]. An edit of at most 20 lines to a matching file after review binds, first touched or not, takes /ship finish\'s scoped reader instead of Steps 5–11.5; the reader still reads every changed row. Read from the base branch only. Empty (default) declares none.', merge: 'train (default) | direct. direct: the Captain lands each READY PR at its own green head with `gstack-ship-queue land --pr --pin ` (no train, no stamp commit; the PR leaves VERSION, the manifest version and the CHANGELOG at the base and the release stamps them), and train build refuses. train: merge trains as before (docs/ship-policy.md#land-direct).', min_gstack_version: 'The oldest gstack that may read this file. Any key newer than 1.91.98.0 set to a non-default value requires it at that key’s introducing version (validate prints each); a gstack older than it refuses the file (POLICY_GSTACK_TOO_OLD).', }; @@ -305,13 +308,6 @@ function listErrors(key: string, value: unknown, out: PolicyError[], check: (ite }); } -function globError(g: string): string | null { - const why = containedRelative(g); - if (why) return why; - try { globToRegExp(g); } catch (e: any) { return e.message; } - return null; -} - /** `review_exclude_paths`: git glob pathspecs, each one repo-relative line with no pathspec magic of its own; every error is POLICY_INVALID at its index. */ function excludeErrors(value: unknown, out: PolicyError[]): void { if (!Array.isArray(value)) { out.push({ code: 'POLICY_INVALID', path: 'review_exclude_paths', message: 'must be an array of git glob pathspecs' }); return; } @@ -469,6 +465,7 @@ export function validatePolicy(raw: unknown, opts: { gstackVersion?: string } = if ('stamp_files' in obj) listErrors('stamp_files', obj.stamp_files, errors, globError); if ('stamp_patterns' in obj) templateErrors(obj.stamp_patterns, errors); if ('stamp_placeholders' in obj) placeholderErrors(obj.stamp_placeholders, errors); + if ('bookkeeping' in obj) listErrors('bookkeeping', obj.bookkeeping, errors, g => globError(g) ?? bookkeepingGlobError(g)); if ('restamp_command' in obj && obj.restamp_command !== null) { const t = obj.restamp_command; if (typeof t !== 'string' || !t.trim()) errors.push({ code: 'POLICY_INVALID', path: 'restamp_command', message: 'must be a non-empty string or null' }); diff --git a/lib/ship-release.ts b/lib/ship-release.ts index 6f6f6ab80..87f1296f6 100644 --- a/lib/ship-release.ts +++ b/lib/ship-release.ts @@ -14,7 +14,8 @@ import * as path from 'node:path'; import { mergeChangelogs, normalizeChangelog } from './changelog-check'; import { versionAtRef } from './restamp'; import { classifyPath } from './review-depth'; -import { comparePath, contextPacket, gateEditedPaths, isTestCode, PACKET_SECTION_CAP, readScopedReview, touchedPaths, type PatchSides } from './ship-patch'; +import { matchesAny } from './ship-policy'; +import { boundedPaths, comparePath, contextPacket, gateEditedPaths, isTestCode, PACKET_SECTION_CAP, readScopedReview, touchedPaths, type BoundedKind, type PatchSides } from './ship-patch'; import { locate, readRecord } from './approval-record'; import { reviewFreshness } from './review-evidence'; import { parseReviewLoopException, type ReviewLoopException } from './ship-receipt-depth'; @@ -199,8 +200,9 @@ function harnessRepin(cwd: string, from: string, to: string, p: string): boolean * stage 2.2 recheck. */ type FixCheckException = ReviewLoopException & { fix_check: NonNullable }; -type Routed = Record<'behavior' | 'docs' | 'metadata' | 'tests' | 'base' | 'context' | 'accepted' | 'fix_checked', string[]>; -function route(cwd: string, base: string, reviewed: string, reviewedBase: string | undefined, release: string[], opts: { docsRechecked: boolean; contextReviewed?: string; loop?: FixCheckException }, r: Report): Routed | null { +type Routed = Record<'behavior' | 'docs' | 'metadata' | 'tests' | 'base' | 'context' | 'bounded' | 'accepted' | 'fix_checked', string[]>; +type RouteOpts = { docsRechecked: boolean; contextReviewed?: string; loop?: FixCheckException; audited: Record | null; bookkeeping: readonly string[] }; +function route(cwd: string, base: string, reviewed: string, reviewedBase: string | undefined, release: string[], opts: RouteOpts, r: Report): Routed | null { const now = bin(cwd, 'gstack-wtree', [], 60_000).stdout.trim(); if (!/^[0-9a-f]{40,64}$/.test(now)) { r.stop('gstack-wtree printed no tree; the stage 2 comparison is blocked'); return null; } const changedSet = touchedPaths(cwd, reviewed, now); @@ -219,20 +221,23 @@ function route(cwd: string, base: string, reviewed: string, reviewedBase: string sides = { reviewedBase: rb, reviewed, currentBase: cb, current: now }; touched = new Set([...a, ...b]); } - const by: Routed = { behavior: [], docs: [], metadata: [], tests: [], base: [], context: [], accepted: [], fix_checked: [] }; + const by: Routed = { behavior: [], docs: [], metadata: [], tests: [], base: [], context: [], bounded: [], accepted: [], fix_checked: [] }; for (const p of [...changedSet].sort()) { const verdict = sides ? (touched.has(p) ? comparePath(cwd, sides, p) : 'same') : 'changed'; - by[verdict === 'same' ? 'base' : release.includes(p) || harnessRepin(cwd, reviewed, now, p) ? 'metadata' : sides && isTestCode(p) ? 'tests' : classifyPath(p, cwd) === 'docs' ? 'docs' : verdict === 'context' ? 'context' : 'behavior'].push(p); + by[verdict === 'same' ? 'base' : release.includes(p) || harnessRepin(cwd, reviewed, now, p) ? 'metadata' : sides && isTestCode(p) ? 'tests' : classifyPath(p, cwd) === 'docs' && !matchesAny(opts.bookkeeping, p) ? 'docs' : verdict === 'context' ? 'context' : 'behavior'].push(p); } const gated = gateEditedPaths(cwd, reviewed, now, by.tests) ?? new Set(by.tests); by.behavior.push(...by.tests.filter(p => gated.has(p))); by.tests = by.tests.filter(p => !gated.has(p)); - const packetBody = sides && (by.context.length || by.tests.length) ? contextPacket(cwd, sides, by.context, by.tests) : null; + const bounded = boundedPaths(cwd, { sides, reviewed, now, base: baseRef, candidates: by.behavior, audited: opts.audited, bookkeeping: opts.bookkeeping }); + by.bounded = Object.keys(bounded.kinds).sort(); + by.behavior = by.behavior.filter(p => !by.bounded.includes(p)); + const late = (): Record => Object.fromEntries(by.bounded.map(p => [p, bounded.kinds[p]!])); + const packetBody = sides && (by.context.length || by.tests.length || by.bounded.length) ? contextPacket(cwd, sides, by.context, by.tests, late()) : null; if (packetBody?.over.length) { r.add(`SCOPED_REVIEW: too large for the scoped review (over ${PACKET_SECTION_CAP} lines in a section): ${short(packetBody.over)}`); by.behavior.push(...packetBody.over); - by.context = by.context.filter(p => !packetBody.over.includes(p)); - by.tests = by.tests.filter(p => !packetBody.over.includes(p)); + for (const k of ['context', 'tests', 'bounded'] as const) by[k] = by[k].filter(p => !packetBody.over.includes(p)); } const fixRow = bin(cwd, 'gstack-review-read', [], 60_000).stdout.split('\n---CONFIG---')[0].split('\n') .flatMap(l => { try { const v = JSON.parse(l); return v?.skill === 'review-fix-check' ? [v] : []; } catch { return []; } }).at(-1); @@ -256,8 +261,8 @@ function route(cwd: string, base: string, reviewed: string, reviewedBase: string if (!range || !after) { r.stop(`git could not diff the fix-check range ${fixFrom.slice(0, 12)}..${fixCommit.slice(0, 12)} against the reviewed and current trees: stage 2 is blocked`); return null; } const fixSides = { reviewedBase: git(cwd, ['merge-base', baseRef, fixCommit]).stdout.trim(), reviewed: fixCommit, currentBase: sides?.currentBase ?? git(cwd, ['merge-base', baseRef, 'HEAD']).stdout.trim(), current: now }; const unmoved = (p: string) => !after.has(p) || (!!fixSides.reviewedBase && !!fixSides.currentBase && comparePath(cwd, fixSides, p) === 'same'); - by.fix_checked = [...by.behavior, ...by.tests, ...by.context].filter(p => range.has(p) && unmoved(p)).sort(); - for (const k of ['behavior', 'tests', 'context'] as const) by[k] = by[k].filter(p => !by.fix_checked.includes(p)); + by.fix_checked = [...by.behavior, ...by.tests, ...by.context, ...by.bounded].filter(p => range.has(p) && unmoved(p)).sort(); + for (const k of ['behavior', 'tests', 'context', 'bounded'] as const) by[k] = by[k].filter(p => !by.fix_checked.includes(p)); r.add(`FIX_CHECK: reviewed ${by.fix_checked.length ? short(by.fix_checked) : 'none'} (fix-check at ${fixCommit.slice(0, 12)}, range from ${fixFrom.slice(0, 12)}, grant ${grant!.id})`); } const loop = opts.loop; @@ -274,26 +279,30 @@ function route(cwd: string, base: string, reviewed: string, reviewedBase: string by.behavior = by.behavior.filter(p => !by.accepted.includes(p)); by.docs = by.docs.filter(p => !by.accepted.includes(p)); } - r.add(`ROUTE: reviewed=${reviewed.slice(0, 12)} current=${now.slice(0, 12)} behavior=${by.behavior.length} docs=${by.docs.length} metadata=${by.metadata.length} tests=${by.tests.length} base=${sides ? by.base.length : 'unknown'} context=${by.context.length}${counted ? ` fix_checked=${by.fix_checked.length}` : ''}${loop ? ` accepted=${by.accepted.length}` : ''}`); + r.add(`ROUTE: reviewed=${reviewed.slice(0, 12)} current=${now.slice(0, 12)} behavior=${by.behavior.length} docs=${by.docs.length} metadata=${by.metadata.length} tests=${by.tests.length} base=${sides ? by.base.length : 'unknown'} context=${by.context.length}${by.bounded.length ? ` bounded=${by.bounded.length}` : ''}${counted ? ` fix_checked=${by.fix_checked.length}` : ''}${loop ? ` accepted=${by.accepted.length}` : ''}`); if (loop) r.add(`REVIEW_LOOP_EXCEPTION: accepted ${by.accepted.length ? short(by.accepted) : 'none'} (changed since fix-check ${loop.fix_check.head.slice(0, 12)}; accepted by ${loop.accepted_by} ${loop.date} via ${loop.via})`); + for (const x of bounded.refused) if (by.behavior.includes(x.path)) r.add(`ROUTE_BOUNDED_REFUSED: ${x.path} ${x.reason} (fix: ${x.fix}; docs/troubleshooting.md#route-bounded-refused)`); if (by.behavior.length) { r.stop(`behavior, tests or build inputs changed since review (${short(by.behavior.sort())}): insert 5–11.5 → 12–14 → 16 before Step 17 (stage 2.1)`); return null; } - const scoped = [...by.context, ...by.tests].sort(); + const scoped = [...by.context, ...by.tests, ...by.bounded].sort(); if (scoped.length && sides) { + const packet = path.resolve(cwd, git(cwd, ['rev-parse', '--git-path', `gstack/scoped-review-${now.slice(0, 12)}.md`]).stdout.trim() || `.git/gstack/scoped-review-${now.slice(0, 12)}.md`); + const built = contextPacket(cwd, sides, by.context, by.tests, late()); if (!opts.contextReviewed) { - const packet = path.resolve(cwd, git(cwd, ['rev-parse', '--git-path', `gstack/scoped-review-${now.slice(0, 12)}.md`]).stdout.trim() || `.git/gstack/scoped-review-${now.slice(0, 12)}.md`); - const why = [by.context.length ? `base context moved next to the reviewed hunks in ${short(by.context)}` : '', by.tests.length ? `test code changed since review in ${short(by.tests)}` : ''].filter(Boolean).join('; '); + const why = [by.context.length ? `base context moved next to the reviewed hunks in ${short(by.context)}` : '', by.tests.length ? `test code changed since review in ${short(by.tests)}` : '', + by.bounded.length ? `bounded edits since review in ${by.bounded.map(p => `${p} (${bounded.kinds[p]})`).join(', ')}` : ''].filter(Boolean).join('; '); try { fs.mkdirSync(path.dirname(packet), { recursive: true }); - fs.writeFileSync(packet, contextPacket(cwd, sides, by.context, by.tests).text); + fs.writeFileSync(packet, built.text); } catch (e) { r.stop(`${why}, but the scoped-review packet ${packet} cannot be written (${firstLine((e as Error).message)}): make its directory writable, then rerun finish (stage 2.1)`); return null; } - r.stop(`${why}: one fix-check reviewer reads ${packet}, then rerun with --context-reviewed written beside the packet, outside the worktree ({"tree":"${now}","paths":[…],"reviewer":"","verdict":"clean"}); a blocking finding is a behavior change (stage 2.1)`); + r.stop(`${why}: one scoped reader (not a review round) reads ${packet}, then rerun with --context-reviewed written beside the packet, outside the worktree ({"tree":"${now}","paths":[…],"reviewer":"","verdict":"clean","packet_sha256":"${built.sha256}"}); a blocking finding is a behavior change (stage 2.1)`); return null; } const review = readScopedReview(opts.contextReviewed, { tree: now }, scoped); if (!review.ok) { r.stop(`--context-reviewed: ${review.message}`); return null; } + if (by.bounded.length && review.review.packet_sha256 !== built.sha256) { r.stop(`--context-reviewed: the record's packet_sha256 is ${review.review.packet_sha256?.slice(0, 12) ?? 'missing'}, not ${built.sha256.slice(0, 12)}: bounded edits need a record bound to the packet the reader read (${packet}); rerun the reader on it (stage 2.1)`); return null; } r.add(`SCOPED_REVIEW: clean by ${review.review.reviewer} (${short(scoped)})`); } if (by.docs.length && !opts.docsRechecked) { r.stop(`authored docs changed since review (${short(by.docs)}): recheck the plan items they touch (stage 2.2), then rerun with --docs-rechecked`); return null; } @@ -422,7 +431,10 @@ async function finishStages(argv: readonly string[], cwd: string): Promise | null = null; + try { const h = JSON.parse(fs.readFileSync(flag('--docs-post')!, 'utf8')).hashes; audited = h && typeof h === 'object' && !Array.isArray(h) ? h : null; } catch { audited = null; } + const bookkeeping = Array.isArray(policy?.bookkeeping) ? policy!.bookkeeping.filter((g: unknown): g is string => typeof g === 'string') : []; + const routed = route(cwd, base, flag('--reviewed-tree')!, flag('--reviewed-base'), release, { docsRechecked: has('--docs-rechecked'), contextReviewed: flag('--context-reviewed'), loop, audited, bookkeeping }, r); if (!routed) return r; if (!docsFresh(cwd, base, flag('--docs-post')!, [...routed.base, ...routed.context, ...stampedFiles(policy, cwd, flag('--reviewed-tree')!, base)], all('--docs-requested'), flag('--docs-result'), loop ? routed.accepted : undefined, r)) return r; if (!list.length && !has('--no-lanes-approved')) return r.stop('no test lanes: require Step 5\'s explicit untested-scope approval (rerun with --no-lanes-approved) or run Steps 5-15'); diff --git a/ship/SKILL.md b/ship/SKILL.md index e71db2284..769833bd7 100644 --- a/ship/SKILL.md +++ b/ship/SKILL.md @@ -688,7 +688,7 @@ Its `STOP:` names the route: Insert `5–11.5 → 12–14 → 16` before the pending Step 17. It excludes Step 14.5 (a rebuild can change generated docs; compare before stage 3) and Step 15 (`finish` STOPs on an uncommitted non-release file: commit it by Step 15's rules, then rerun). A STOP naming a scoped-review packet - instead takes its fix-check and `--context-reviewed` rerun; a blocking finding is this item. + instead takes one reader (no round) and `--context-reviewed` rerun; a blocker 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 diff --git a/ship/SKILL.md.tmpl b/ship/SKILL.md.tmpl index 0d66edee0..10f2071fe 100644 --- a/ship/SKILL.md.tmpl +++ b/ship/SKILL.md.tmpl @@ -409,7 +409,7 @@ Its `STOP:` names the route: Insert `5–11.5 → 12–14 → 16` before the pending Step 17. It excludes Step 14.5 (a rebuild can change generated docs; compare before stage 3) and Step 15 (`finish` STOPs on an uncommitted non-release file: commit it by Step 15's rules, then rerun). A STOP naming a scoped-review packet - instead takes its fix-check and `--context-reviewed` rerun; a blocking finding is this item. + instead takes one reader (no round) and `--context-reviewed` rerun; a blocker 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 diff --git a/test/fixtures/golden/agy-ship-SKILL.md b/test/fixtures/golden/agy-ship-SKILL.md index 712fba169..cfec5b43e 100644 --- a/test/fixtures/golden/agy-ship-SKILL.md +++ b/test/fixtures/golden/agy-ship-SKILL.md @@ -689,7 +689,7 @@ Its `STOP:` names the route: Insert `5–11.5 → 12–14 → 16` before the pending Step 17. It excludes Step 14.5 (a rebuild can change generated docs; compare before stage 3) and Step 15 (`finish` STOPs on an uncommitted non-release file: commit it by Step 15's rules, then rerun). A STOP naming a scoped-review packet - instead takes its fix-check and `--context-reviewed` rerun; a blocking finding is this item. + instead takes one reader (no round) and `--context-reviewed` rerun; a blocker 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 diff --git a/test/fixtures/golden/claude-ship-SKILL.md b/test/fixtures/golden/claude-ship-SKILL.md index e71db2284..769833bd7 100644 --- a/test/fixtures/golden/claude-ship-SKILL.md +++ b/test/fixtures/golden/claude-ship-SKILL.md @@ -688,7 +688,7 @@ Its `STOP:` names the route: Insert `5–11.5 → 12–14 → 16` before the pending Step 17. It excludes Step 14.5 (a rebuild can change generated docs; compare before stage 3) and Step 15 (`finish` STOPs on an uncommitted non-release file: commit it by Step 15's rules, then rerun). A STOP naming a scoped-review packet - instead takes its fix-check and `--context-reviewed` rerun; a blocking finding is this item. + instead takes one reader (no round) and `--context-reviewed` rerun; a blocker 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 diff --git a/test/fixtures/golden/codex-ship-SKILL.md b/test/fixtures/golden/codex-ship-SKILL.md index 3079a142e..b94c81878 100644 --- a/test/fixtures/golden/codex-ship-SKILL.md +++ b/test/fixtures/golden/codex-ship-SKILL.md @@ -691,7 +691,7 @@ Its `STOP:` names the route: Insert `5–11.5 → 12–14 → 16` before the pending Step 17. It excludes Step 14.5 (a rebuild can change generated docs; compare before stage 3) and Step 15 (`finish` STOPs on an uncommitted non-release file: commit it by Step 15's rules, then rerun). A STOP naming a scoped-review packet - instead takes its fix-check and `--context-reviewed` rerun; a blocking finding is this item. + instead takes one reader (no round) and `--context-reviewed` rerun; a blocker 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 diff --git a/test/fixtures/golden/factory-ship-SKILL.md b/test/fixtures/golden/factory-ship-SKILL.md index 8f667ba31..24e3df64c 100644 --- a/test/fixtures/golden/factory-ship-SKILL.md +++ b/test/fixtures/golden/factory-ship-SKILL.md @@ -693,7 +693,7 @@ Its `STOP:` names the route: Insert `5–11.5 → 12–14 → 16` before the pending Step 17. It excludes Step 14.5 (a rebuild can change generated docs; compare before stage 3) and Step 15 (`finish` STOPs on an uncommitted non-release file: commit it by Step 15's rules, then rerun). A STOP naming a scoped-review packet - instead takes its fix-check and `--context-reviewed` rerun; a blocking finding is this item. + instead takes one reader (no round) and `--context-reviewed` rerun; a blocker 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 diff --git a/test/fixtures/review-pins/files/lib/review-depth.ts.json b/test/fixtures/review-pins/files/lib/review-depth.ts.json index e16c7705b..960e473cd 100644 --- a/test/fixtures/review-pins/files/lib/review-depth.ts.json +++ b/test/fixtures/review-pins/files/lib/review-depth.ts.json @@ -1 +1 @@ -{"sha":"e8940804d37c63cd11284461b5aa0235bfd23ec50cc2df6baa4f15e8d9cef124","replay":"GST-1350"} +{"sha":"713ff6413608f1ab75f2c5306286cd1f275d2434ba84bc07b3b68876f4c81a3b","replay":"GST-1276"} diff --git a/test/fixtures/review-pins/replays/GST-1276.json b/test/fixtures/review-pins/replays/GST-1276.json new file mode 100644 index 000000000..4d1494c7a --- /dev/null +++ b/test/fixtures/review-pins/replays/GST-1276.json @@ -0,0 +1 @@ +{"replay":"GST-1276","justification":"GST-1276 no-loss replay (lane GSTA-275, 120 calls, $1.20): the bounded scoped-reader packet flagged every seeded post-review gate weakening (audit prose, bookkeeping, lanes) 3/3 on claude-opus-5-5 and gpt-6.1-sol and cleared 4/4 and 3/4 honest edits; the no-gstack arm missed 2/6 and 1/6. lib/review-depth.ts changes only GATE_PATH, adding .gstack/always-run.json, so a diff touching it now takes the gate class and full depth: a depth rule that can only raise depth, never drop a reader"} diff --git a/test/ship-bounded-route.test.ts b/test/ship-bounded-route.test.ts new file mode 100644 index 000000000..0c56e1c6b --- /dev/null +++ b/test/ship-bounded-route.test.ts @@ -0,0 +1,219 @@ +/** + * Bounded post-review edits (GST-1276, GST-1311): a required edit that lands after review binds (a docs-audit fix to + * code-classed policy prose, an additive lane mapping in .gstack/pregate.json, a declared bookkeeping file, a value-only + * test literal) takes one scoped reader at finish instead of another review round, and only when the diff proves its + * kind. Everything else still stops for Steps 5–11.5, now with a ROUTE_BOUNDED_REFUSED line naming why. + */ +import { afterEach, describe, expect, setDefaultTimeout, test } from 'bun:test'; +import { createHash } from 'node:crypto'; +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { spawnSync } from 'node:child_process'; +import { isGatePath } from '../lib/review-depth'; +import { BOOKKEEPING_KEY_VERSION } from '../lib/ship-policy'; +import { lineOf, RELEASE, shipFixture, type ShipFx } from './helpers/ship-bin-fixture'; + +setDefaultTimeout(90_000); +const BIN = path.dirname(RELEASE); +const LANE = 'tests=printf " 3 pass\\n 0 fail\\nRan 3 tests across 1 file. [5.00ms]\\n"'; +const ships: ShipFx[] = []; +afterEach(() => { while (ships.length) ships.pop()!.cleanup(); }); +const fx = (...a: Parameters) => { const f = shipFixture(...a); ships.push(f); return f; }; +const wtree = (f: ShipFx) => f.run(path.join(BIN, 'gstack-wtree'), []).stdout.trim(); +const mergeBase = (f: ShipFx) => f.git('merge-base', 'origin/main', 'HEAD').stdout.trim(); +const snapshot = (f: ShipFx, docs = ['README.md'], out = path.join(f.root, 'post.json')) => { + expect(f.run(path.join(BIN, 'gstack-docs-candidate'), ['snapshot', '--out', out, '--audit-id', 'a1', '--mode', 'edit', '--base', 'main', ...docs.flatMap(d => ['--docs', d])]).status).toBe(0); + return out; +}; +const finish = (f: ShipFx, reviewed: string, post: string, extra: string[] = []) => f.run(RELEASE, ['finish', '--base', 'main', '--reviewed-tree', reviewed, '--docs-post', post, '--lane', LANE, ...extra]); +const behaviorStop = (p: string) => `STOP: behavior, tests or build inputs changed since review (${p}): insert 5–11.5 → 12–14 → 16 before Step 17 (stage 2.1)`; +const packetOf = (out: string) => /reads (\S+\.md)[ ,]/.exec(out)?.[1] ?? ''; +const record = (f: ShipFx, paths: string[], packet: string | null, extra: Record = {}) => { + const file = path.join(f.root, 'bounded-reviewed.json'); + const sha = packet ? createHash('sha256').update(fs.readFileSync(packet)).digest('hex') : undefined; + fs.writeFileSync(file, JSON.stringify({ tree: wtree(f), paths, reviewer: 'anthropic/claude-opus-5-5', verdict: 'clean', ...(sha ? { packet_sha256: sha } : {}), ...extra })); + return file; +}; +const POLICY = (n: number, edit: Record = {}) => '# Review policy\n\n' + Array.from({ length: n }, (_, i) => edit[i + 1] ?? `- rule ${i + 1}: a reviewer reads the hunk.`).join('\n') + '\n'; + +describe('docs-audit edits to code-classed policy prose (GST-1276: #3970, #3985)', () => { + // The audit's own edit: the reviewed tree held the old bytes, the post-audit snapshot holds the new bytes, and the + // file still holds them. finish asks one scoped reader for those hunks, bound to the packet's hash. + const audited = (rel = 'docs/review-policy.md', edit: Record = { 3: '- rule 3: the binary exclusion is plain docs carry.' }) => { + const f = fx({ [rel]: POLICY(12) }); + const reviewed = wtree(f); + const reviewedBase = mergeBase(f); + f.write({ [rel]: POLICY(12, edit) }); f.commit('docs: audit fixes stale policy text'); + const post = snapshot(f, ['docs', 'review', 'AGENTS.md', 'README.md'].filter(d => fs.existsSync(path.join(f.work, d)))); + return { f, reviewed, reviewedBase, post, rel }; + }; + + test('an audit edit to docs/review-policy.md routes to one scoped reader, and a clean bound record lets finish go on', () => { + const { f, reviewed, reviewedBase, post, rel } = audited(); + const ask = finish(f, reviewed, post, ['--reviewed-base', reviewedBase]); + expect(lineOf(ask.stdout, 'ROUTE'), ask.stdout).toMatch(/ behavior=0 .* bounded=1$/); + expect(lineOf(ask.stdout, 'STOP')).toContain(`bounded edits since review in ${rel} (audit prose)`); + const packet = packetOf(ask.stdout); + expect(fs.readFileSync(packet, 'utf8')).toContain('the binary exclusion is plain docs carry'); + expect(fs.readFileSync(packet, 'utf8')).toContain('## money'); + const unbound = finish(f, reviewed, post, ['--reviewed-base', reviewedBase, '--context-reviewed', record(f, [rel], null)]); + expect([unbound.status, lineOf(unbound.stdout, 'STOP')]).toEqual([3, expect.stringContaining('packet_sha256')]); + const ok = finish(f, reviewed, post, ['--reviewed-base', reviewedBase, '--context-reviewed', record(f, [rel], packet)]); + expect(lineOf(ok.stdout, 'SCOPED_REVIEW'), ok.stdout).toBe(`SCOPED_REVIEW: clean by anthropic/claude-opus-5-5 (${rel})`); + expect(lineOf(ok.stdout, 'DOCS')).toContain('fresh'); + }); + + test('docs/pregate.md and AGENTS.md take the same route', () => { + for (const rel of ['docs/pregate.md', 'AGENTS.md']) { + const { f, reviewed, reviewedBase, post } = audited(rel); + expect([rel, lineOf(finish(f, reviewed, post, ['--reviewed-base', reviewedBase]).stdout, 'ROUTE')]).toEqual([rel, expect.stringMatching(/ behavior=0 .* bounded=1$/)]); + } + }); + + test('controls: no --reviewed-base, an edit after the post snapshot, over 20 lines or a gate-path brief still stop, with a reason', () => { + const noSides = audited(); + const a = finish(noSides.f, noSides.reviewed, noSides.post); + expect([a.status, lineOf(a.stdout, 'STOP')]).toEqual([3, behaviorStop(noSides.rel)]); + expect(lineOf(a.stdout, 'ROUTE_BOUNDED_REFUSED')).toContain(`${noSides.rel} needs --reviewed-base`); + + const late = audited(); + late.f.write({ [late.rel]: POLICY(12, { 3: '- rule 3: the binary exclusion is plain docs carry.', 9: '- rule 9: never mind.' }) }); late.f.commit('docs: edit after the audit'); + const b = finish(late.f, late.reviewed, late.post, ['--reviewed-base', late.reviewedBase]); + expect([b.status, lineOf(b.stdout, 'STOP')]).toEqual([3, behaviorStop(late.rel)]); + expect(lineOf(b.stdout, 'ROUTE_BOUNDED_REFUSED')).toContain('is not the bytes the post-audit snapshot recorded'); + + const big = audited('docs/review-policy.md', Object.fromEntries(Array.from({ length: 11 }, (_, i) => [i + 1, `- rule ${i + 1}: rewritten.`]))); + const c = finish(big.f, big.reviewed, big.post, ['--reviewed-base', big.reviewedBase]); + expect([c.status, lineOf(c.stdout, 'ROUTE_BOUNDED_REFUSED')]).toEqual([3, expect.stringContaining('changed 22 lines since review (at most 20)')]); + + const gate = audited('review/specialists/targeted.md'); + const d = finish(gate.f, gate.reviewed, gate.post, ['--reviewed-base', gate.reviewedBase]); + expect([d.status, lineOf(d.stdout, 'STOP')]).toEqual([3, behaviorStop('review/specialists/targeted.md')]); + }); +}); + +describe('an additive lane mapping in .gstack/pregate.json (GST-1276: gstack-evals#203)', () => { + const pregate = (deps: Record, extra: Record = {}) => JSON.stringify({ dependencies: deps, ...extra }, null, 2) + '\n'; + const mapped = (after: Record, [was, now]: Record[] = [{}, {}]) => { + const f = fx({ '.gstack/pregate.json': pregate({ 'src/a.ts': ['test/a.test.ts'] }, was) }); + const reviewed = wtree(f); + const reviewedBase = mergeBase(f); + const post = snapshot(f); + f.write({ '.gstack/pregate.json': pregate(after, now) }); f.commit('pregate: map the lane'); + return finish(f, reviewed, post, ['--reviewed-base', reviewedBase]); + }; + + test('a mapping that only adds tests takes the scoped reader', () => { + const r = mapped({ 'src/a.ts': ['test/a.test.ts'], 'src/b.ts': ['test/b.test.ts'] }); + expect(lineOf(r.stdout, 'ROUTE'), r.stdout).toMatch(/ behavior=0 .* bounded=1$/); + expect(lineOf(r.stdout, 'STOP')).toContain('.gstack/pregate.json (lanes)'); + }); + + test('a removed mapping or a new [] declaration loosens and stops', () => { + for (const [name, after, why] of [['removed', {}, 'is removed'], ['empty', { 'src/a.ts': ['test/a.test.ts'], 'docs/**': [] }, 'declares no lane needed']] as const) { + const r = mapped(after as Record); + expect([name, r.status, lineOf(r.stdout, 'STOP')]).toEqual([name, 3, behaviorStop('.gstack/pregate.json')]); + expect([name, lineOf(r.stdout, 'ROUTE_BOUNDED_REFUSED')]).toEqual([name, expect.stringContaining(why)]); + } + }); + + test('a lane added beside a cut CI-reuse job or a dropped sealed rule stops; unchanged keys still take the reader', () => { + const profile = { workflow: 'free-tests.yml', events: ['pull_request'], jobs: ['free-plan', 'free-tests'], artifact: 'tested-tree' }; + const was = { run_profiles: { 'ci-reuse/free-tests': profile }, sealed: { paths: ['evals/sealed/**'] } }; + const lane = { 'src/a.ts': ['test/a.test.ts'], 'src/b.ts': ['test/b.test.ts'] }; + const cut = { ...was, run_profiles: { 'ci-reuse/free-tests': { ...profile, jobs: ['free-plan'] } } }; + for (const [name, now] of [['run_profiles', cut], ['sealed', { run_profiles: was.run_profiles }]] as const) { + const r = mapped(lane, [was, now]); + expect([name, r.status, lineOf(r.stdout, 'STOP')]).toEqual([name, 3, behaviorStop('.gstack/pregate.json')]); + expect([name, lineOf(r.stdout, 'ROUTE_BOUNDED_REFUSED')]).toEqual([name, expect.stringContaining(`changes ${name}`)]); + } + expect(lineOf(mapped(lane, [was, was]).stdout, 'STOP')).toContain('.gstack/pregate.json (lanes)'); + }); +}); + +describe('declared bookkeeping files (GST-1311: gbrain#8523, #8525)', () => { + const policy = (globs: string[]) => JSON.stringify({ bookkeeping: globs, min_gstack_version: BOOKKEEPING_KEY_VERSION }) + '\n'; + const TSV = (rows: string[]) => 'function\tlines\n' + rows.join('\n') + '\n'; + + test('a renamed baseline row and a file first added after review take the scoped reader; an undeclared file stops', () => { + const f = fx({ '.gstack/ship-policy.json': policy(['scripts/*-baseline.tsv', 'scripts/serial-files.tsv']), 'scripts/function-size-baseline.tsv': TSV(['oldFn\t180', 'other\t160']) }); + const reviewed = wtree(f); + const reviewedBase = mergeBase(f); + const post = snapshot(f); + f.write({ 'scripts/function-size-baseline.tsv': TSV(['newFn\t180', 'other\t160']), 'scripts/serial-files.tsv': TSV(['test/a.test.ts\t1']) }); f.commit('verify: baseline rows'); + const r = finish(f, reviewed, post, ['--reviewed-base', reviewedBase]); + expect(lineOf(r.stdout, 'ROUTE'), r.stdout).toMatch(/ behavior=0 .* bounded=2$/); + f.write({ 'scripts/other.tsv': TSV(['x\t1']) }); f.commit('an undeclared table'); + const s = finish(f, reviewed, post, ['--reviewed-base', reviewedBase]); + expect([s.status, lineOf(s.stdout, 'STOP')]).toEqual([3, behaviorStop('scripts/other.tsv')]); + }); + + test('a declared .txt table takes the bounded route, not the docs recheck; a glob reaching .github/ is refused', () => { + const f = fx({ '.gstack/ship-policy.json': policy(['scripts/*-baseline.txt', '**/*.tsv']), 'scripts/skill-size-baseline.txt': 'a 10\nb 20\n', '.github/owners.tsv': TSV(['a\t1']) }); + const reviewed = wtree(f); + const reviewedBase = mergeBase(f); + const post = snapshot(f); + f.write({ 'scripts/skill-size-baseline.txt': 'a 12\nb 20\n' }); f.commit('verify: size baseline'); + const r = finish(f, reviewed, post, ['--reviewed-base', reviewedBase]); + expect(lineOf(r.stdout, 'ROUTE'), r.stdout).toMatch(/ behavior=0 docs=0 .* bounded=1$/); + f.write({ '.github/owners.tsv': TSV(['a\t2']) }); f.commit('a table under .github/'); + const s = finish(f, reviewed, post, ['--reviewed-base', reviewedBase]); + expect(lineOf(s.stdout, 'ROUTE_BOUNDED_REFUSED'), s.stdout).toContain('.github/owners.tsv is a gate path or under .gstack/ or .github/'); + expect([s.status, lineOf(s.stdout, 'STOP')]).toEqual([3, behaviorStop('.github/owners.tsv')]); + }); + + test('a bookkeeping glob that names code is refused at load', () => { + const f = fx({ '.gstack/ship-policy.json': policy(['scripts/*.ts']) }); + const v = f.run(path.join(BIN, 'gstack-ship-policy'), ['validate', '--json']); + expect(v.stdout + v.stderr).toContain('must end in .tsv, .csv or .txt'); + expect(v.status).not.toBe(0); + }); +}); + +describe('a value-only test literal after review (GST-1276 instance 5: #3984)', () => { + const spec = (key: string, body = " expect(scan(KEY)).toBe(false);\n") => `import { expect, test } from 'bun:test';\nconst KEY = '${key}';\nconst scan = (s: string) => s.startsWith('sk-live-');\ntest('fake key', () => {\n${body}});\n`; + test('a changed string literal routes as tests (scoped reader); a changed assertion still stops', () => { + const f = fx({}, { 'src/b.ts': 'export const b = 2;\n', 'test/key.test.ts': spec('sk-test-abc') }); + const reviewed = wtree(f); + const post = snapshot(f); + f.write({ 'test/key.test.ts': spec('fixture-value') }); f.commit('test: a value the secrets scan does not flag'); + const r = finish(f, reviewed, post, ['--reviewed-base', mergeBase(f)]); + expect(lineOf(r.stdout, 'ROUTE'), r.stdout).toMatch(/ behavior=0 docs=0 metadata=0 tests=1 /); + f.write({ 'test/key.test.ts': spec('fixture-value', " expect(scan(KEY)).toBe(true);\n") }); f.commit('test: flip the assertion'); + const s = finish(f, reviewed, post, ['--reviewed-base', mergeBase(f)]); + expect([s.status, lineOf(s.stdout, 'STOP')]).toEqual([3, behaviorStop('test/key.test.ts')]); + }); + + test('an expected value inside a multi-line assertion, or code in an f-string, still stops', () => { + const multi = (name: string, gap = '') => spec('fixture-value', ` expect({ name: KEY }).toEqual({\n${gap} name: '${name}',\n });\n`); + const tmpl = (v: string) => spec('fixture-value', ` expect(gen()).toBe(\`\nconst a = 1;\nconst b = '${v}';\n\`);\n`); + const py = (flag: string) => `import subprocess\nBIN = "bin/scan"\n\ndef test_scan():\n cmd = f"{BIN} ${flag}"\n assert subprocess.run(cmd, shell=True).returncode == 0\n`; + const f = fx({}, { 'src/b.ts': 'export const b = 2;\n', 'test/key.test.ts': multi('alice'), 'test/test_scan.py': py('--strict') }); + const reviewed = wtree(f); + const post = snapshot(f); + f.write({ 'test/key.test.ts': multi('bob') }); f.commit('test: change an expected value'); + const r = finish(f, reviewed, post, ['--reviewed-base', mergeBase(f)]); + expect([r.status, lineOf(r.stdout, 'STOP')]).toEqual([3, behaviorStop('test/key.test.ts')]); + f.write({ 'test/key.test.ts': multi('alice'), 'test/test_scan.py': py('--lenient') }); f.commit('test: change code in an f-string'); + const s = finish(f, reviewed, post, ['--reviewed-base', mergeBase(f)]); + expect([s.status, lineOf(s.stdout, 'STOP')]).toEqual([3, behaviorStop('test/test_scan.py')]); + for (const [before, after] of [[multi('alice', '\n'), multi('bob', '\n')], [tmpl('old'), tmpl('new')]] as const) { + f.write({ 'test/test_scan.py': py('--strict'), 'test/key.test.ts': before }); f.commit('test: reset'); + const at = wtree(f); + f.write({ 'test/key.test.ts': after }); f.commit('test: change an expected value'); + const t = finish(f, at, snapshot(f), ['--reviewed-base', mergeBase(f)]); + expect([t.status, lineOf(t.stdout, 'STOP')], after).toEqual([3, behaviorStop('test/key.test.ts')]); + } + }); +}); + +describe('gstack-diff-scope and gate paths for .gstack/ (GST-1276: GEVA-2 ask c)', () => { + test('a .gstack/-only diff is config, not unmatched; .gstack/always-run.json is a gate path', () => { + const f = fx({ '.gstack/pregate.json': '{"dependencies":{}}\n' }); + f.write({ '.gstack/pregate.json': '{"dependencies":{"src/b.ts":["test/b.test.ts"]}}\n' }); f.commit('lanes'); + const out = spawnSync(path.join(BIN, 'gstack-diff-scope'), ['main'], { cwd: f.work, encoding: 'utf8', timeout: 30_000 }); + expect(out.stdout).toContain('SCOPE_CONFIG=true'); + expect(out.stdout).not.toContain('SCOPE_ERROR'); + expect(isGatePath('.gstack/always-run.json')).toBe(true); + }); +}); diff --git a/test/ship-control-flow.test.ts b/test/ship-control-flow.test.ts index ced811b60..ff954b9e3 100644 --- a/test/ship-control-flow.test.ts +++ b/test/ship-control-flow.test.ts @@ -343,7 +343,7 @@ describe('ship source controller', () => { // 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.'); + expect(behavior).toContain('A STOP naming a scoped-review packet instead takes one reader (no round) and `--context-reviewed` rerun; a blocker 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 `.'); }); diff --git a/test/ship-rereview-triggers.test.ts b/test/ship-rereview-triggers.test.ts index d6441f1f5..274dcceb7 100644 --- a/test/ship-rereview-triggers.test.ts +++ b/test/ship-rereview-triggers.test.ts @@ -446,7 +446,7 @@ describe('test-only and audit-requested edits after the docs audit (GST-391, gbr f.write({ 'test/b.test.ts': `${spec('b')}test('b is even', () => expect(2 % 2).toBe(0));\n` }); f.commit('test: add a case'); const ask = finish(f, reviewed, post, ['--reviewed-base', mergeBase(f)]); expect(lineOf(ask.stdout, 'ROUTE')).toMatch(/ behavior=0 docs=0 metadata=0 tests=1 /); - expect([ask.status, lineOf(ask.stdout, 'STOP')]).toEqual([3, expect.stringContaining('STOP: test code changed since review in test/b.test.ts: one fix-check reviewer reads ')]); + expect([ask.status, lineOf(ask.stdout, 'STOP')]).toEqual([3, expect.stringContaining('STOP: test code changed since review in test/b.test.ts: one scoped reader (not a review round) reads ')]); const packet = /reads (\S+\.md), then/.exec(ask.stdout)![1]!; expect(fs.readFileSync(packet, 'utf8')).toContain("test('b is even'"); const record = path.join(f.root, 'tests-reviewed.json'); @@ -464,7 +464,7 @@ describe('test-only and audit-requested edits after the docs audit (GST-391, gbr const pw = snapshot(w); w.write({ 'test/gate.test.ts': after }); w.commit(`test: ${name}`); const out = finish(w, rw, pw, ['--reviewed-base', mergeBase(w)]); - expect([name, out.status, lineOf(out.stdout, 'STOP')]).toEqual([name, 3, expect.stringContaining('STOP: test code changed since review in test/gate.test.ts: one fix-check reviewer reads ')]); + expect([name, out.status, lineOf(out.stdout, 'STOP')]).toEqual([name, 3, expect.stringContaining('STOP: test code changed since review in test/gate.test.ts: one scoped reader (not a review round) reads ')]); } const long = fx({}, { 'src/b.ts': 'export const b = 2;\n', 'test/b.test.ts': spec('b') }); diff --git a/test/size-ledger/ship/gst-1276-bounded-route.json b/test/size-ledger/ship/gst-1276-bounded-route.json new file mode 100644 index 000000000..dcd00685e --- /dev/null +++ b/test/size-ledger/ship/gst-1276-bounded-route.json @@ -0,0 +1,4 @@ +{ + "bytes": -1, + "why": "GST-1276: a scoped-review STOP takes one reader (no round), not a fix-check" +}