docs, tests and changelog for review_exclude_paths (GST-508)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
garrytanandcapy-ai[bot] committed 2026-11-01 01:38:39 +00:00
1 parent b4b88df137
commit a7b87145c3
6 files changed
+503

No files matched your search

@@ -0,0 +1,15 @@
**/ship can leave a results PR's receipt files out of the review, and it stops instead of handing a reader a diff it cannot read.** A results PR in harness-evals or gbrain's eval lanes can carry thousands of receipt files. One HARN-1 results PR spent its outside-voice budget reading receipts and the pass died anyway (GST-507, GST-508). The repository owner can now list those folders in `.gstack/ship-policy.json`, and every reviewer reads the code without them.
### Added
- **`review_exclude_paths` in `.gstack/ship-policy.json`** (`["receipts/**"]`, with `min_gstack_version` at least the release that adds it). It is read from the base branch only, so a branch cannot exclude its own files; a branch that adds the key gets a normal review and `REVIEW_EXCLUDE: none (head sets review_exclude_paths; …)`. Patterns are git glob pathspecs, root-anchored, and git itself lists the matches.
- **Only content is excluded**: data files, docs, `.log` files and files git reports as binary. Code, tests, executables, gate paths, agent prose, config, `.gstack/` and `.github/` files, gate-tagged files and files with a credential line stay in every reader's diff, each with a reason code. A data file a code or test file names by its full path stays too. A basename, a directory reference or a data manifest that lists receipts does not keep anything.
- **`gstack-review-plan` prints `REVIEW_EXCLUDE:`** with the counts, the excluded files by kind, the kept files and a digest. Every reader's `DIFF_CMD:`, the fixture command the security specialist and the Red Team use, and the outside voice's diff leave the files out; kept files come back through `DIFF_KEEP:`. Every brief says what was left out. The kept-file command is bounded at 24,000 argument bytes, and a pattern over that bound is not applied and the plan says why.
- **Excluded lines no longer trigger the numeric-claim scan or the targeted money, gate and auth classes**, and they count as docs for the review depth.
- **The receipt records `review_exclude`.** `gstack-review-dispatch verify` prints `REVIEW_EXCLUDE_RECORD: digest=<d>` from the briefs the readers got, and `gstack-ship-receipt write --review-exclude <d>` must match the digest HEAD has under the base policy (`RECEIPT_INVALID field=review_exclude`, nothing written). Carry and the queue compare the digest, not the base commit, so a base merge that leaves the policy alone still carries.
- **A review ceiling.** `gstack-review-dispatch pass`, `stage` and `render` read each reader's diff once before writing anything. Over 2,000,000 bytes (about 500K tokens) is `REVIEW_DIFF_TOO_LARGE` (exit 3) with a fix for where the policy stands and pasteable JSON; a `git diff` that fails or runs past 120 seconds is `REVIEW_DIFF_UNREADABLE` (exit 1) with the reason, the timeout and a redacted stderr log.
### Changed
- A repository without the key plans, briefs and diffs byte for byte as before. The one change it can see is the ceiling: a reader's diff over 2,000,000 bytes now stops the pass instead of being read in part.
### Tests
- `test/review-exclude.test.ts`: what is excluded and kept (binary files under `config/`, a file named only by a data manifest or by its basename, a file a test names by full path), every brief's diff commands run against git, the numeric scan and targeted classes, the base policy read (no file, a missing blob, a bad commit, an invalid key), the argument-byte bound, both ceiling lines, and the receipt write, carry and queue checks.
+19
View File
@@ -338,6 +338,24 @@ Over the last 100 non-release commits on main (8663c3d2..8be7f213), money hit
which hit 43 of 100; those paths already get full depth, so the gate class was
narrowed to the gate scan's tagged hunks.
<a id="review-exclude-paths"></a>
## What readers read under `review_exclude_paths`
A repository can list content folders its reviewers leave out, on its base
branch only ([ship-policy.md](ship-policy.md#review-exclude-paths)). Under it,
every reader (specialists, the Red Team, the native and outside voices, the
targeted reader) reads the same selection: its `DIFF_CMD:` with one
`:(top,exclude,glob)` item per applied pattern, the fixture command with the
same items, and the `DIFF_KEEP:` command for the kept files under those
patterns. The outside voice's embedded diff is that selection. Each brief says
how many files and bytes were left out and that the reader must not report on
their absence. Excluded paths count as docs for the depth and never trigger the
numeric-claim scan or a targeted class; kept files keep their own class.
Independently of the key, no reader is handed more than 2,000,000 bytes of
diff: `gstack-review-dispatch` stops with `REVIEW_DIFF_TOO_LARGE` before it writes
anything, instead of a review of the part a reader happened to see.
## What the receipt records
`gstack-review-scope summary --since <REVIEW_SINCE>` reads this invocation's
@@ -532,6 +550,7 @@ ones, and the arm lost blockers. `bin/gstack-review-dispatch` closes both ends:
- `scripts/resolvers/review-army.ts`: the floor and `NO FINDINGS` in every specialist and Red Team prompt (rendered into `review/sections/review-army.md` and `ship/sections/review-army.md`), and the lead judgment's "On record" line that turns a [ruled class](#rulings-below-the-floor) into a note; in `/ship` every specialist and Red Team prompt it renders carries its diff base as a literal sha the parent writes in (`DIFF_BASE=<diff-base>`: the merge base on pass 1, `RANGE_BASE` from pass 2 on) beside its plan's `DIFF_CMD` excludes, because a reviewer is its own process and never sees the parent's shell variables; the fan-out batches above.
- `ship/sections/review-army.md` Step 9: the plan's `REVIEW_DEPTH`, `GATE_SCAN` and `--depth-floor` on every run, the depth fields of the review-log row, passes, `RANGE_BASE`, classify, defer, render, the cap check before any edit, the Step 11 late-defer row, scratch files under `.gstack/tmp/`, the cap decisions; `ship/sections/adversarial.md` Step 11: the outside voice reads the same range and scopes queued fixes the same way.
- `lib/review-exclude.ts`: the base policy's `review_exclude_paths` (matched set, kept reasons, the kept-file argument bound, the digest the receipt, carry and the queue compare) and the 2,000,000-byte reader ceiling (`test/review-exclude.test.ts`).
- `lib/review-depth.ts`, `bin/gstack-review-plan`: the diff class, the depth (`depthFor`, `DEPTH_RULES`, `depthOfCommitInput`), each dispatch's `exclude` pathspecs and `DIFF_CMD`, the transition table, the numeric-claim scan and each role's effort tier (`test/review-depth.test.ts`, `test/review-plan-cli.test.ts`).
- `lib/review-collect.ts`: `collectCommit` (git objects only, from any checkout) and `collectLive` (the working tree), per-path lines and the base commit's registry (`test/review-collect.test.ts`).
- `lib/review-scope.ts`, `bin/gstack-review-scope`: `classify`, `defer`, `render`, `summary` (with `fix_checks`, `dispatches`, `est_input_tokens`); refusals are `REVIEW_SCOPE_INPUT` and `REVIEW_SCOPE_RANGE` ([troubleshooting.md](troubleshooting.md#review-scope-input)).
+122
View File
@@ -58,6 +58,7 @@ refuses the file (`POLICY_GSTACK_TOO_OLD`). `validate` ends with
| `gate_command` | command or `null` | `null` | The repo's full pre-merge gate. `gstack-ship-queue gate-ahead` and `ready` print it for the caller to run (in the background or in CI); gstack never executes it. Needs `min_gstack_version`. |
| `full_e2e` | `true` \| `false` | `false` | `true`: every merge needs a passing full-gate result bound to a tree equal to the merging one modulo stamps; `gstack-ship-queue ready`, `stamp` and `verify` refuse without one (`QUEUE_GATE_MISSING`): a gate-ahead record, or the lane's `--gate-exit 0 --gate-log <file>` from its run on the gated head. Needs `min_gstack_version`. |
| `review_scope` | `frozen` \| `diff-plus-blocking` \| `all` | `frozen` | The finding scope of `/ship`'s review loop; see [review_scope](#review-scope). `frozen` fixes the diff on pass 1, files blocking findings in unchanged code as issues, and later passes read only the range since the previous pass. `diff-plus-blocking` is the same but fixes a blocking finding in unchanged code in this PR instead of filing it. `all` is the earlier loop: every finding is fixed on every pass. Needs `min_gstack_version` when not `frozen`. |
| `review_exclude_paths` | git glob pathspecs | `[]` | Content files (data, docs, binary, `.log`) `/ship`'s reviewers leave out; read from the base only, with code, gate, config, executable and named-by-code files kept. See [review_exclude_paths](#review-exclude-paths). Needs `min_gstack_version`. |
| `min_gstack_version` | version or `null` | `null` | The oldest gstack that may read the file; see the compatibility rule above. |
| `docs` | `{ caps: [{ path, max_bytes }] }` or `null` | `null` | Documentation files with a byte cap. `/ship` Step 14.5 runs `gstack-ship-receipt docs-caps --base <base>` (the caps come from `origin/<base>`'s policy, never the branch's copy; `--policy <file>` is for a repository bootstrapping its policy) before the verification gate and prints one `DOCS_CAP: <path> <bytes>/<max> ok\|over` line per cap; any `over` (an absent file counts) exits 1 and blocks the ship until the file is trimmed or the cap raised on the base branch (`DOCS_AUDIT_BLOCKED`). At most 64 caps, each path contained. See [the ship receipt](#ship-receipt). Needs `min_gstack_version`. |
| `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`. |
@@ -1094,6 +1095,127 @@ is not a `pass` claim. Text you write under the heading after that block, and
a `## Approvals` example inside a code fence, are still checked. Data PRs (evals,
measurements, sweeps) run it before READY so the body's numbers are the run's.
<a id="review-exclude-paths"></a>
## `review_exclude_paths`: what reviewers leave out
Since the release that added it (`REVIEW_EXCLUDE_KEY_VERSION` in
`lib/ship-policy.ts`). A results PR can carry thousands of receipt files that
no reviewer needs to read. `review_exclude_paths` lists git glob pathspecs
whose content files `/ship`'s reviewers leave out:
```json
{ "review_exclude_paths": ["receipts/**"], "min_gstack_version": "1.92.0.0" }
```
**Read from the base only.** `gstack-review-plan` reads the policy committed on
the base commit it already resolved (`origin/<base>`, or `<base>`), never
the branch's copy, so a branch cannot exclude its own files. A branch that adds
the key gets `REVIEW_EXCLUDE: none (head sets review_exclude_paths; it applies
after it lands on <base>)` and a normal review. A base policy that does not
validate stops the plan (`POLICY_INVALID`), and a base git cannot read stops it
(`POLICY_SOURCE_UNAVAILABLE`); neither turns exclusion off quietly.
**Git's glob dialect.** Each pattern is a `:(top,glob)` pathspec: root-anchored
and case-sensitive. The matched set comes from git itself (`git diff --raw -z`,
plus `git ls-files --others -z` for untracked files), so the plan, the receipt
and what readers see are one set.
| Pattern | gitignore would match | `review_exclude_paths` matches |
|---|---|---|
| `receipts/**` | everything under `receipts/` at any depth | everything under the top-level `receipts/` |
| `receipts/*` | files directly in `receipts/` | files directly in `receipts/` only (`*` never crosses `/`) |
| `data/` | any `data/` directory at any depth | nothing: write `data/**` (and `**/data/**` for any depth) |
A pattern that matches no changed file is listed as `unmatched=` on the plan line.
**What is excluded, and what is kept.** A matched file is excluded only when it
is content: a data file (JSON, JSONL, CSV, TSV, images), a docs-class file, a
`.log` file, or a file git reports as binary (`-\t-` in `--numstat`; readers
would only see `Binary files differ`). A binary file is content even under
`config/` or in a skill directory; only the first four reasons below keep it.
Everything else stays in every reader's diff with one reason code:
| Reason | Kept because |
|---|---|
| `policy-dir` | it is under `.gstack/` or `.github/` |
| `gate-path` | it is a gate path (lib/review-depth.ts) |
| `executable` | its old or new mode is executable |
| `agent-prose` | it is a skill, section, resolver, template or rule file |
| `config` | it is a data-shaped config file (`package.json`, `tsconfig*.json`, `config/`, dotfiles) |
| `gate-tagged` | the gate scan tagged one of its hunks |
| `code` | it is code or a test, not content |
| `secret-line` | an added line reads a credential (the targeted pass's secret rule) |
| `named-by-code` | a code or test file that stays in review (not docs, a data file, a `.log` or binary, and not itself excluded) names its full repository path, at the head or the merge base |
| `consumer-scan-unavailable` | the `git grep` for those names failed, so every candidate is kept |
Only a full path counts as a name. A basename such as `result.json` would keep
every receipt in the tree, and a directory reference such as `receipts/` (the
harness that writes the receipts names it) keeps nothing: the plan prints an
informational `REVIEW_EXCLUDE_REFERENCE:` line naming the code that reads the
folder. A data file never keeps another file: a results manifest such as
`docs/receipts-manifest.json` lists every receipt by full path, and it is
content, not a program that reads them. The name scan is one uncapped `git grep -F -f` over the candidates'
paths, not the collector's capped consumer scan.
**What readers run.** Every reader's `DIFF_CMD:` gains one
`':(top,exclude,glob)<pattern>'` item per applied pattern, and the fixture
command the security specialist, the Red Team and the native adversarial pass
read through gets the same items. A git exclude beats any positive pathspec, so
kept files under an applied pattern come back through a second command,
`DIFF_KEEP: <reader> git diff <diff-base> -- ':(top,literal)<path>' …`, which every
brief names. That command is bounded by its argument bytes (24,000 across all
patterns, under Windows' command-line limit): a pattern whose kept files would
pass the bound is not applied, and the plan line says so as
`skipped=<pattern>:kept=<n>:bytes=<b>`. Every brief also gets one line:
`Excluded by base policy <ref>@<sha12> (selection <digest>): <n> files, <bytes> bytes
on this branch matching <patterns>. They are not in your diff; do not report on
their absence.`
**Depth, numbers and the targeted pass.** Excluded paths count as docs for the
review depth everywhere it is computed (`gstack-review-plan`, the receipt
writer, carry and the queue), and their lines never trigger the numeric-claim
scan or the targeted money, gate, auth and secret classes. Defects inside a
correctly excluded data file are coverage the owner gives up by policy.
**The plan line.** `REVIEW_EXCLUDE: patterns=<applied>/<declared> excluded=<files>
bytes=<b> kept=<k> content=<kind>:<n>,… source=<ref>@<sha12> digest=<sha256>`
(`content=` counts the excluded files by kind: `binary`, `data`, `docs`, `log`), then up to 20
`REVIEW_EXCLUDE_KEPT: <reason> <path>` lines (`--json` lists every one). A
repository with no key anywhere prints none of these lines, and its plan, briefs
and diff commands are byte for byte what they were.
**The receipt, carry and the queue.** Each rendered brief records the digest it
names in the review ledger, and `gstack-review-dispatch verify` prints
`REVIEW_EXCLUDE_RECORD: digest=<d>` when every reader's newest brief names the
same one (a mixed set makes it `VERIFY: dirty`). `gstack-ship-receipt write
--review-exclude <d>`, with that digest, records `review_exclude` (source, patterns, counts, the gate-tagged kept
paths and three hashes). The `digest` hashes the declared patterns and the
excluded and kept lists; `source` is provenance only. Write, carry and the queue
each recompute the digest at the current base and compare it, so a base merge
or a moved base that leaves the patterns and the lists unchanged still carries
and binds. A different digest means the reviewers read another selection:
write refuses with `RECEIPT_INVALID field=review_exclude`, carry with
`RECEIPT_CARRY_REFUSED review_exclude changed`, and the queue with
`RECEIPT_INVALID field=review_exclude expected=<digest> recorded=<digest>`. The
fix is to rerun the review, never a receipt write alone.
**Adopting it.** (1) Open a PR on the base that adds the patterns and
`min_gstack_version` at least `REVIEW_EXCLUDE_KEY_VERSION`, and ship it like any
policy change. (2) Once it merges, bring the base into the results branch (a
crew needs its coordinator's word for that merge) and rerun `/ship`; the plan
prints `REVIEW_EXCLUDE:`. (3) Check the kept list: a file you expected to be
excluded names its reason.
<a id="review-diff-ceiling"></a>
**The review ceiling.** Independently of the key, `gstack-review-dispatch` reads
each reviewer's diff once before it writes a manifest or a brief (`pass`, `stage`
and `render`). A diff over 2,000,000 bytes (about 500K tokens) stops with
[`REVIEW_DIFF_TOO_LARGE`](troubleshooting.md#review-diff-too-large), exit 3, and a
fix chosen by where the policy stands; a `git diff` that fails or runs past
120 seconds stops with
[`REVIEW_DIFF_UNREADABLE`](troubleshooting.md#review-diff-unreadable), exit 1.
There is no override flag: the recovery is a policy PR on the base, or a split.
<a id="review-scope"></a>
## `review_scope`: how the review loop converges
+66
View File
@@ -3079,6 +3079,72 @@ checked-out HEAD: the receipt binds the tree the review read.
**Fix.** Regenerate it with `gstack-ship-receipt write`; never hand-edit the
block. For the `--head` refusal, check out that commit and write there.
**`field=review_exclude`.** `gstack-ship-receipt write` and the queue's receipt check also
use this code when the receipt's review_exclude digest (or the
`--review-exclude <digest>` write was given) is not the digest HEAD has under the
base policy's [`review_exclude_paths`](ship-policy.md#review-exclude-paths). The line
names `expected=` (now), `recorded=` (what the review read, `none` when it read
without exclusion) and `source=` (the base commit). The reviewers read another
selection than the one the head now has, so a receipt write alone cannot clear
it: rerun `/ship`'s review on the head (`gstack-review-dispatch pass --base <base>`),
run `gstack-review-dispatch verify --ledger <ledger>`, which prints
`REVIEW_EXCLUDE_RECORD: digest=<d>` from the briefs the readers got, then
`gstack-ship-receipt write --pr <n> --review-exclude <d> --body <body>`.
<a id="review-diff-too-large"></a>
### `REVIEW_DIFF_TOO_LARGE`
**Meaning.** One reviewer's diff is over the 2,000,000-byte review ceiling
(about 500K tokens). `gstack-review-dispatch pass`, `stage` and `render` read
each reader's diff once before writing anything, and stop (exit 3, nothing
written) rather than hand a reader more than it can read. The line names the
reader, its bytes, the five largest top-level folders (`top=<dir>:<bytes>`) and
the base policy commit.
**Fix.** The line prints the fix for where the base policy stands:
- **No key:** copy the printed JSON (`{"review_exclude_paths":[…],"min_gstack_version":"…"}`)
into `.gstack/ship-policy.json` on the base as its own PR. A lane cannot land on
the base itself: ask the repo owner or the coordinator. Once it merges, bring
the base into the branch and rerun once; if it is still over, split the PR.
- **The branch declares the key but the base does not:** only the base copy
applies; land that hunk on the base first, as above.
- **A pattern was not applied** (`skipped=` on the `REVIEW_EXCLUDE:` line): its kept
files need more than 24,000 argument bytes. Narrow the pattern, move the kept
files out of it, or split the PR.
- **Still over after exclusion:** add the printed folders to the base policy, or
split the PR.
There is no override flag. See [the review ceiling](ship-policy.md#review-diff-ceiling).
<a id="review-diff-unreadable"></a>
### `REVIEW_DIFF_UNREADABLE`
**Meaning.** git could not read a reviewer's diff, so the pass stopped (exit 1)
before writing a manifest or a brief. `reason=timeout` means `git diff` ran past
`timeout_ms` (120,000 ms; fixed, no flag); `reason=git-exit` gives git's exit code.
The line carries the diff base, git's first stderr line with remote URLs
replaced by `<url>`, and `log=`, a file under the run directory holding git's whole
stderr, redacted the same way. It is also printed when git cannot list the
files `review_exclude_paths` matches.
**Fix.** By reason: a timeout, rerun once unchanged, then split the PR or
exclude its largest data folders on the base; git exit 128 (a missing object or
an unfetched base), `git fetch origin <base>` and rerun once; anything else, stop
and ask a person with the log file.
<a id="review-exclude-reference"></a>
### `REVIEW_EXCLUDE_REFERENCE`
**Meaning.** Informational, never a stop. A kept code file names a folder that
`review_exclude_paths` excludes (for example the harness that writes
`receipts/`). A folder reference does not keep files; only a full file path
does, so readers see that code without the files it reads.
**Fix.** Nothing for the review itself. If the code's correctness depends on the
excluded files, narrow the pattern on the base so they stay in the diff.
<a id="execution-unknown"></a>
### `EXECUTION_UNKNOWN`
+277
View File
@@ -0,0 +1,277 @@
/**
* review_exclude_paths and the reviewer diff ceiling (GST-508, docs/ship-policy.md#review-exclude-paths): what the base
* policy excludes and keeps, what every brief's diff commands read, the receipt binding, the base-policy read, the
* argument-byte bound and the REVIEW_DIFF_TOO_LARGE / REVIEW_DIFF_UNREADABLE lines.
*/
import { afterEach, describe, expect, test } from 'bun:test';
import { spawnSync } from 'node:child_process';
import * as fs from 'node:fs';
import * as path from 'node:path';
import { EXIT } from '../lib/headless-artifacts';
import { KEEP_ARG_MAX_BYTES, readSelection, tooLargeLine, unreadableLine } from '../lib/review-exclude';
import { extractReceipt } from '../lib/ship-receipt';
import { bindShippedReceipt } from '../lib/ship-receipt-depth';
import { REVIEW_EXCLUDE_KEY_VERSION, isUnavailable, loadPolicyAt } from '../lib/ship-policy';
import { cleanup, commit, makeFixtureRepo, runBin, write, type FixtureRepo } from './helpers/pregate-fixture';
const repos: FixtureRepo[] = [];
afterEach(() => { while (repos.length) cleanup(repos.pop()!); });
const policyText = (patterns: string[]) => JSON.stringify({ review_exclude_paths: patterns, min_gstack_version: REVIEW_EXCLUDE_KEY_VERSION }) + '\n';
const GZ = Buffer.from([0x1f, 0x8b, 0, 1, 2, 3, 0, 0]);
const EXCLUDED = ['receipts/a/result.json', 'receipts/a/trial.gz', 'receipts/config/blob.bin', 'receipts/c/result.json', 'receipts/e/result.json', 'receipts/notes.md'];
const KEPT = ['receipts/b/result.json', 'receipts/d/fixture.json', 'receipts/run.sh'];
/** A results branch: receipts/** on the base policy, receipts of every kind on the branch. */
function census(patterns: string[] | null = ['receipts/**']): FixtureRepo {
const repo = makeFixtureRepo({ prefix: 'gstack-exclude-', baseFiles: patterns ? { '.gstack/ship-policy.json': policyText(patterns) } : {} });
repos.push(repo);
for (const [rel, bytes] of [['receipts/a/trial.gz', GZ], ['receipts/config/blob.bin', GZ]] as const) {
fs.mkdirSync(path.dirname(path.join(repo.work, rel)), { recursive: true });
fs.writeFileSync(path.join(repo.work, rel), bytes);
}
write(repo.work, 'receipts/run.sh', '#!/bin/sh\necho run\n');
fs.chmodSync(path.join(repo.work, 'receipts/run.sh'), 0o755);
commit(repo, {
'receipts/a/result.json': '{"cost_usd": 12.5, "pass_rate": "97%"}\n',
'receipts/b/result.json': '{"ok": true}\n',
'receipts/c/result.json': '{"ok": true}\n',
'receipts/d/fixture.json': '{"ok": true}\n',
'receipts/e/result.json': '{"ok": true}\n',
'receipts/notes.md': '# notes\n',
'docs/receipts-manifest.json': JSON.stringify(['receipts/c/result.json', 'receipts/a/trial.gz']) + '\n',
'lib/reader.js': "exports.p = 'receipts/b/result.json';\nexports.q = 'result.json';\n",
'test/d.test.js': "const f = 'receipts/d/fixture.json';\n",
});
return repo;
}
const plan = (repo: FixtureRepo, extra: string[] = []) => runBin(repo, 'gstack-review-plan', ['--base', 'main', '--gate-tagged', '0', ...extra]);
const mergeBase = (repo: FixtureRepo) => repo.git('merge-base', 'origin/main', 'HEAD');
/** The paths one diff command from a plan line or a brief reads (run as --name-only in the fixture). */
function pathsRead(repo: FixtureRepo, cmd: string, diffBase: string): string[] {
const named = cmd.replace(/^git diff( --stat| --name-status)?/, 'git diff --name-only').replace('<diff-base>', '"$DIFF_BASE"');
const r = spawnSync('bash', ['-c', named], { cwd: repo.work, encoding: 'utf8', timeout: 30_000, env: { ...process.env, ...repo.env, DIFF_BASE: diffBase } });
expect({ cmd: named, status: r.status, err: r.stderr }).toMatchObject({ cmd: named, status: 0 });
return r.stdout.split('\n').filter(Boolean);
}
/** Rewrite the base: commit on main, push, and merge it into the feature branch. */
function onBase(repo: FixtureRepo, files: Record<string, string>) {
repo.git('checkout', '-q', 'main');
commit(repo, files, 'base change');
repo.git('push', '-q', 'origin', 'main');
repo.git('checkout', '-q', 'feature');
repo.git('fetch', '-q', 'origin');
repo.git('merge', '-q', '--no-edit', 'origin/main');
}
describe('what review_exclude_paths excludes and keeps', () => {
// Value: protects=a results PR's receipts leave the reviewers' diff while code, executables and files a program names stay;
// fails_when=a binary file under config/ is kept, a file named only by a data manifest or by its basename is kept, or a
// file a test names by full path is excluded; why_new=GST-508 adds the selection; seam=bin/gstack-review-plan on a real
// repository and git running the printed commands; no_claim=references by relative path or directory (REVIEW_EXCLUDE_REFERENCE)
test('binary, data and docs files are excluded; only a code or test file naming the full path keeps one', () => {
const repo = census();
const p = JSON.parse(plan(repo, ['--json']).out);
expect(p.review_exclude.kept_files).toEqual([
{ path: 'receipts/b/result.json', reason: 'named-by-code' },
{ path: 'receipts/d/fixture.json', reason: 'named-by-code' },
{ path: 'receipts/run.sh', reason: 'executable' },
]);
expect(p.review_exclude).toMatchObject({ state: 'active', excluded: EXCLUDED.length, content: { binary: 2, data: 3, docs: 1, log: 0 } });
const text = plan(repo).out;
expect(text).toContain(`REVIEW_EXCLUDE: patterns=1/1 excluded=${EXCLUDED.length} `);
expect(text).toContain(' content=binary:2,data:3,docs:1 ');
const mb = mergeBase(repo);
const cmds = text.split('\n').filter(l => /^DIFF_(CMD|KEEP): /.test(l)).map(l => l.replace(/^DIFF_(CMD|KEEP): \S+ /, ''));
expect(cmds.length).toBeGreaterThan(2);
const read = new Set(cmds.flatMap(c => pathsRead(repo, c, mb)));
for (const f of EXCLUDED) expect(read.has(f)).toBe(false);
for (const f of KEPT) expect(read.has(f)).toBe(true);
}, 120_000);
// Value: protects=the Red Team, the security specialist, the adversarial pass and the outside voice read the same selection
// as the plan, including through the fixture-handling command; fails_when=any brief's git diff command (or the embedded
// outside-voice diff) shows an excluded file, or a kept file is unreachable from a brief; why_new=task 8 (the fixture
// command had no exclude slot); seam=gstack-review-dispatch pass/stage/render and git; no_claim=what a model does with the brief
test('every brief\'s git diff commands leave excluded files out and reach the kept ones', () => {
const repo = census();
const out = path.join(repo.root, 'pass');
expect(runBin(repo, 'gstack-review-dispatch', ['pass', '--base', 'main', '--out', out, '--pass', 'p1', '--', '--gate-tagged', '0']).status).toBe(EXIT.ok);
expect(runBin(repo, 'gstack-review-dispatch', ['stage', '--manifest', path.join(out, 'pass-manifest.json'), '--stage', 'voices']).status).toBe(EXIT.ok);
const m = JSON.parse(fs.readFileSync(path.join(out, 'pass-manifest.json'), 'utf8'));
const reader = m.readers.find((r: any) => r.role === 'adversarial-native');
const mb = mergeBase(repo);
const slots = ['--base', 'main', '--diff-base', mb, '--exclude', reader.exclude, '--keep', ['--', ...reader.keep.map((k: string) => `'${k}'`)].join(' '), '--disclosure', m.disclosure];
for (const role of [['specialist', '--name', 'security'], ['red-team', '--specialists', '1'], ['outside-voice']]) {
const r = runBin(repo, 'gstack-review-dispatch', ['render', '--role', ...role, '--out', path.join(out, 'briefs'), ...slots]);
expect(r.status).toBe(EXIT.ok);
}
const briefs = fs.readdirSync(path.join(out, 'briefs')).filter(f => f.endsWith('.md')).map(f => fs.readFileSync(path.join(out, 'briefs', f), 'utf8'));
expect(briefs.length).toBe(5);
const cmds = [...new Set(briefs.flatMap(b => [...b.matchAll(/`[^`\n]*?(git diff [^`\n]*)`/g)].map(x => x[1]!)))];
expect(cmds.some(c => c.includes("'*fixture*'"))).toBe(true);
const read = new Set(cmds.flatMap(c => pathsRead(repo, c, mb)));
for (const f of EXCLUDED) expect(read.has(f)).toBe(false);
for (const f of KEPT) expect(read.has(f)).toBe(true);
const outside = briefs.find(b => b.includes('## Branch diff'))!;
expect(outside).toContain(`git diff "$DIFF_BASE" -- . ${m.readers[0].policy_exclude} and git diff "$DIFF_BASE" -- '${reader.keep[0]}'`);
for (const f of EXCLUDED) expect(outside).not.toContain(`diff --git a/${f}`);
expect(outside).toContain('diff --git a/receipts/b/result.json');
for (const b of briefs) expect(b).toContain(`(selection ${m.review_exclude.digest})`);
}, 180_000);
// Value: protects=excluded receipts cannot dispatch the outside voice or the targeted money reader on lines no reader sees,
// and a credential line inside a matched file keeps it; fails_when=scanNumbers or targetedClasses read excluded lines, or
// a secret-bearing data file is excluded; why_new=task 9; seam=bin/gstack-review-plan; no_claim=pregate's credential scan
test('excluded lines feed neither the numeric scan nor the targeted classes; a credential line keeps its file', () => {
const files = { 'receipts/r.json': '{"cost_usd": 12.5, "price_usd": 3, "pass_rate": "97%", "latency_ms": 180}\n', 'docs/bench.md': '# bench\n', 'receipts/s.json': '{"cmd": "curl -H $OPENAI_API_KEY"}\n' };
const control = makeFixtureRepo({ prefix: 'gstack-exclude-' }); repos.push(control);
commit(control, files);
const with_ = makeFixtureRepo({ prefix: 'gstack-exclude-', baseFiles: { '.gstack/ship-policy.json': policyText(['receipts/**']) } }); repos.push(with_);
commit(with_, files);
const c = JSON.parse(plan(control, ['--json']).out);
const w = JSON.parse(plan(with_, ['--json']).out);
expect(c.numbers.publishes).toBe(true);
expect(c.targeted.classes).toEqual(['money', 'secret']);
expect(w.review_exclude.kept_files).toEqual([{ path: 'receipts/s.json', reason: 'secret-line' }]);
expect(w.numbers.publishes).toBe(false);
expect(w.targeted.classes).toEqual(['secret']);
}, 120_000);
});
describe('the base policy read', () => {
// Value: protects=a base the plan cannot read never turns exclusion off quietly; fails_when=a missing blob or a bad commit
// reads as no policy, or an invalid base key plans without exclusion; why_new=task 6; seam=loadPolicyAt and the plan CLI;
// no_claim=the head copy's validation (gstack-ship-policy validate)
test('no file reads as defaults; an unreadable object or commit is unavailable; an invalid base key refuses the plan', () => {
const none = census(null);
const sha = none.git('rev-parse', 'origin/main');
const absent = loadPolicyAt(none.work, sha, 'origin/main');
expect(isUnavailable(absent)).toBe(false);
expect(!isUnavailable(absent) && absent.policy.review_exclude_paths).toEqual([]);
expect(isUnavailable(loadPolicyAt(none.work, 'f'.repeat(40), 'origin/main'))).toBe(true);
const gone = census();
const blob = gone.git('rev-parse', 'origin/main:.gstack/ship-policy.json');
fs.rmSync(path.join(gone.work, '.git/objects', blob.slice(0, 2), blob.slice(2)));
const unread = loadPolicyAt(gone.work, gone.git('rev-parse', 'origin/main'), 'origin/main');
expect(isUnavailable(unread)).toBe(true);
const r = plan(gone);
expect(r.status).toBe(EXIT.refused);
expect(r.err).toContain('POLICY_SOURCE_UNAVAILABLE');
const bad = census([':(icase)receipts/**']);
const b = plan(bad);
expect(b.status).toBe(EXIT.refused);
expect(b.err).toContain('review_exclude_paths[0]');
expect(b.err).toContain('(POLICY_INVALID)');
expect(b.out).not.toContain('DISPATCH:');
}, 120_000);
});
describe('the kept-file command is bounded by argument bytes', () => {
// Value: protects=the DIFF_KEEP command stays under the Windows command line however many kept files a pattern has, and a
// disjoint small pattern still applies; fails_when=a pattern whose kept files pass KEEP_ARG_MAX_BYTES is applied, or the plan
// does not say why it was not; why_new=task 10; seam=bin/gstack-review-plan; no_claim=a real Windows launch
test('a pattern whose kept files pass the bound is not applied and the plan says why; a disjoint small pattern applies', () => {
const repo = makeFixtureRepo({ prefix: 'gstack-exclude-', baseFiles: { '.gstack/ship-policy.json': policyText(['big/**', 'small/**']) } }); repos.push(repo);
const long = 'x'.repeat(180);
for (let k = 0; k < 140; k++) {
write(repo.work, `big/${long}-${k}.sh`, '#!/bin/sh\n');
fs.chmodSync(path.join(repo.work, `big/${long}-${k}.sh`), 0o755);
}
commit(repo, { 'big/data.json': '{}\n', 'small/data.json': '{}\n', 'lib/a.js': 'exports.a = 1;\n' });
const text = plan(repo).out;
expect(text).toMatch(new RegExp(`REVIEW_EXCLUDE: patterns=1/2 .* skipped=big/\\*\\*:kept=140:bytes=\\d+ \\(kept files over ${KEEP_ARG_MAX_BYTES} argument bytes: pattern not applied\\)`));
const cmd = text.split('\n').find(l => l.startsWith('DIFF_CMD: '))!;
expect(cmd).toContain("':(top,exclude,glob)small/**'");
expect(cmd).not.toContain('big/**');
expect(text).not.toContain('DIFF_KEEP:');
}, 120_000);
});
describe('the review ceiling lines', () => {
const d = { bytes: 3_500_000, byDir: [['results', 3_000_000], ['src', 500_000]] as Array<[string, number]> };
// Value: protects=a REVIEW_DIFF_TOO_LARGE stop names the one fix that applies, with JSON a person can paste; fails_when=the
// fix ignores where the policy stands or prints a placeholder version; why_new=task 4; seam=tooLargeLine; no_claim=the bytes git reports
test('the fix branches on no key, head-only, a skipped pattern and still over, with pasteable JSON at the key version', () => {
const json = (line: string) => JSON.parse(line.match(/land (\{.*?\}) in /)![1]!);
const none = tooLargeLine('ctx', 'batched', d, { state: 'none', base: 'main', source: 'origin/main@abc' }).line;
expect(json(none)).toEqual({ review_exclude_paths: ['results/**'], min_gstack_version: REVIEW_EXCLUDE_KEY_VERSION });
expect(none).toContain('then bring main into this branch and rerun once; if it is still over, split the PR');
const head = tooLargeLine('ctx', 'batched', d, { state: 'head-only', base: 'main', source: 'origin/main@abc', headPatterns: ['results/**'] }).line;
expect(head).toContain('this branch declares review_exclude_paths, but only the copy on main applies');
const skipped = tooLargeLine('ctx', 'batched', d, { state: 'active', base: 'main', source: 'origin/main@abc', record: { patterns: ['results/**'], skipped: [{ pattern: 'results/**', kept: 900, bytes: 40_000 }], excluded: 0 } }).line;
expect(skipped).toContain('pattern results/** was not applied: its 900 kept files need 40000 argument bytes');
const over = tooLargeLine('ctx', 'batched', d, { state: 'active', base: 'main', source: 'origin/main@abc', floor: '1.300.0.0', record: { patterns: ['receipts/**'], skipped: [], excluded: 12 } }).line;
expect(over).toContain('still over after excluding 12 files');
expect(json(over)).toEqual({ review_exclude_paths: ['receipts/**', 'results/**'], min_gstack_version: '1.300.0.0' });
expect(over).toEndWith('nothing was written (REVIEW_DIFF_TOO_LARGE)');
});
// Value: protects=a REVIEW_DIFF_UNREADABLE stop says timeout or git exit, the timeout, and keeps full stderr in a file with
// remote URLs redacted; fails_when=a credential-bearing remote URL reaches the line or the log, or the fix ignores the
// reason; why_new=task 5; seam=readSelection and unreadableLine; no_claim=a real 120-second timeout
test('a git exit and a timeout name their reason, the timeout and a redacted log file', () => {
const repo = census();
const r = readSelection(repo.work, 'f'.repeat(40), []);
expect(r).toMatchObject({ ok: false, reason: 'git-exit', exit: 128 });
if (r.ok) return;
const logDir = path.join(repo.root, 'run');
const line = unreadableLine('ctx', 'specialist/security', { ...r, stderr: `fatal: unable to access ${['https://bot', 'tok_fixture'].join(':')}${'@'}example.invalid/o/r.git/\nmore\n` }, 'f'.repeat(40), 'main', logDir);
expect(line).toContain('reason=git-exit exit=128 timeout_ms=120000');
expect(line).toContain('fix: git fetch origin main, then rerun once');
expect(line).not.toContain('tok_fixture');
const log = fs.readFileSync(path.join(logDir, 'diff-specialist_security.stderr'), 'utf8');
expect(log).toContain('<url>');
expect(log).not.toContain('tok_fixture');
const slow = unreadableLine('ctx', 'batched', { ok: false, reason: 'timeout', exit: null, stderr: '', timeoutMs: 120_000 }, 'f'.repeat(40), 'main', logDir);
expect(slow).toContain('reason=timeout exit=none timeout_ms=120000');
expect(slow).toContain('rerun once unchanged; if it times out again, split the PR');
}, 60_000);
});
describe('the receipt binds the selection the reviewers read', () => {
const receiptIn = (body: string) => extractReceipt(fs.readFileSync(body, 'utf8')).receipt!;
const writeReceipt = (repo: FixtureRepo, body: string, extra: string[]) =>
runBin(repo, 'gstack-ship-receipt', ['write', '--pr', '7', '--base', 'main', '--version', '1.0.0', '--session-kind', 'unattended', '--body', body, ...extra]);
// Value: protects=a receipt names the selection the rendered briefs read, a base merge that leaves the policy alone still
// carries, and a policy change on the base (adding or removing a pattern) needs a new review, not a write; fails_when=write
// records a digest the briefs never named, carry compares the base sha, or a removed exclusion clears by writing the old
// digest; why_new=tasks 2 and 7; seam=gstack-review-dispatch verify, gstack-ship-receipt write/carry, bindShippedReceipt;
// no_claim=the merge train's own refusal wording
test('verify names the briefs\' digest; write binds it; a neutral base merge carries; a policy edit refuses write and carry', () => {
const repo = census();
const out = path.join(repo.root, 'pass');
expect(runBin(repo, 'gstack-review-dispatch', ['pass', '--base', 'main', '--out', out, '--pass', 'p1', '--', '--gate-tagged', '0']).status).toBe(EXIT.ok);
expect(runBin(repo, 'gstack-review-dispatch', ['stage', '--manifest', path.join(out, 'pass-manifest.json'), '--stage', 'voices']).status).toBe(EXIT.ok);
const v = runBin(repo, 'gstack-review-dispatch', ['verify', '--ledger', path.join(out, 'review-ledger.jsonl')]);
const digest = v.out.match(/^REVIEW_EXCLUDE_RECORD: digest=([0-9a-f]{64}) /m)?.[1];
expect(digest).toBe(JSON.parse(plan(repo, ['--json']).out).review_exclude.digest);
const body = path.join(repo.root, 'body.md');
const missing = writeReceipt(repo, body, []);
expect(missing.status).toBe(EXIT.fail);
expect(missing.err).toContain(`field=review_exclude expected=${digest} recorded=none source=origin/main@`);
expect(missing.err).toContain('gstack-review-dispatch verify --ledger <ledger>');
expect(fs.existsSync(body)).toBe(false);
expect(writeReceipt(repo, body, ['--review-exclude', digest!]).status).toBe(EXIT.ok);
expect(receiptIn(body).review_exclude).toMatchObject({ digest, excluded: EXCLUDED.length, kept: KEPT.length });
onBase(repo, { 'docs/other.md': '# other\n' });
const carried = runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--base', 'main']);
expect(carried.status).toBe(EXIT.ok);
expect(receiptIn(body).review_exclude!.digest).toBe(digest!);
onBase(repo, { '.gstack/ship-policy.json': policyText(['receipts/**', 'results/**']) });
const refused = runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--base', 'main']);
expect(refused.status).toBe(EXIT.refused);
expect(refused.err).toContain(`review_exclude changed: recorded=${digest}`);
const head = repo.git('rev-parse', 'HEAD');
const queue = bindShippedReceipt(repo.work, 'origin/main', fs.readFileSync(body, 'utf8').replace(/head: \S+/, `head: ${head}`), undefined, [head]);
expect(queue.ok).toBe(false);
onBase(repo, { '.gstack/ship-policy.json': JSON.stringify({}) + '\n' });
const removed = writeReceipt(repo, path.join(repo.root, 'body2.md'), ['--review-exclude', digest!]);
expect(removed.status).toBe(EXIT.fail);
expect(removed.err).toContain(`field=review_exclude expected=none recorded=${digest}`);
}, 240_000);
});
@@ -0,0 +1,4 @@
{
"bytes": 486,
"why": "GST-508: review_exclude_paths slots on the fixture and name-status commands, and the --review-exclude receipt step"
}