feat(review): proportional /ship review depth (light, standard, full)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
garrytanandcapy-ai[bot] committed 2026-10-28 23:08:53 +00:00
1 parent e97b857350
commit 1ec488583e
21 files changed
+802 -165

No files matched your search

+41 -38
View File
@@ -4,20 +4,25 @@
*
* gstack-review-plan --base <ref> [--pass <full passes run>] [--last none|full|fix-check] [--fixed-blocking] [--fixed-below]
* [--blocking-open] [--touched-gate] [--critical-found] [--adversarial-ran] [--gate-tagged <n>] [--gated a,b] [--force a,b|all]
* [--reviewed-numbers <fingerprint>] [--no-tiers] [--json]
* [--reviewed-numbers <fingerprint>] [--depth-floor standard|full] [--no-tiers] [--json]
* gstack-review-plan --fix-check-prompt --fix-list <file> --hunks <file> --tests <file> [--callers <file>]
*
* Reads the diff against the merge base with <ref> (committed, working tree and
* untracked), runs gstack-diff-scope and, unless --gate-tagged is given,
* gstack-gate-diff; prints REVIEW_CLASS:, REVIEW_PLAN:, one DISPATCH: line per
* reviewer and one SKIP: line per reviewer left out. Logic: lib/review-depth.ts.
* Reads the diff against the merge base with <ref> (committed, tracked uncommitted
* edits and untracked files, through lib/review-collect.ts collectLive), runs
* gstack-diff-scope and, unless --gate-tagged is given, gstack-gate-diff; prints
* REVIEW_CLASS:, REVIEW_DEPTH: (depth, runtime lines, the rules hash, reasons and
* result codes), GATE_SCAN:, REVIEW_PLAN:, one DEPTH_ERROR: per input that failed
* the depth closed, one DISPATCH: plus DIFF_CMD: (and DIFF_EXCLUDE: none) per
* reviewer and one SKIP: per reviewer left out. A plan that fails closed to full
* exits 0; a diff git cannot list exits 1. Logic: lib/review-depth.ts.
*/
import { spawnSync } from 'node:child_process';
import * as fs from 'node:fs';
import * as path from 'node:path';
import { spawnSync } from 'node:child_process';
import { EXIT, exitTableHelp } from '../lib/headless-artifacts';
import { runScriptSync } from '../lib/script-invocation';
import { planReview, reviewPlanLine, scanNumbers, type History } from '../lib/review-depth';
import { collectLive } from '../lib/review-collect';
import { diffCommand, planReview, reviewDepthLine, reviewPlanLine, scanNumbers, type Depth, type History } from '../lib/review-depth';
import { renderFixCheckPrompt } from '../scripts/resolvers/fix-check';
const HELP = `gstack-review-plan — the review depth of the next /ship step (docs/review-policy.md#review-depth)
@@ -25,19 +30,23 @@ const HELP = `gstack-review-plan — the review depth of the next /ship step (do
--base <ref> the base branch (merge base with HEAD is the diff base); required
--pass <n> full review passes already run in this invocation (default 0)
--last none|full|fix-check the step that just finished (default none: plan pass 1)
--fixed-blocking the finished step fixed a blocking finding
--fixed-blocking the finished step fixed a blocking finding (raises light to standard)
--fixed-below the finished step fixed findings below the floor
--blocking-open a blocking finding is still open in fix scope
--touched-gate a fix touched a gate-class path
--critical-found a specialist reported a CRITICAL finding (Red Team on code class)
--critical-found a specialist reported a CRITICAL finding (Red Team on code class; raises light to standard)
--adversarial-ran Step 11's native adversarial pass already ran (a repeat skips it)
--gate-tagged <n> gate hunks the gate scan tagged (default: run gstack-gate-diff; failure means unavailable)
--gated a,b specialists the hit-rate stats tag GATE_CANDIDATE
--force a,b|all specialists forced by prompt flags (--all-specialists is all)
--reviewed-numbers <fp> the numeric-claim fingerprint the outside voice already reviewed
--depth-floor standard|full raise the depth to at least this (never lowers it); /ship passes its prompt flag on every run
--no-tiers audits and watchers at the host's default effort too (as gstack-config review_effort_tiers off does)
--json the plan as JSON
--json the plan as JSON (depth, runtime_lines, rules, errors with input and repair, gate_scan, per-dispatch exclude)
--fix-check-prompt print the fix-check reviewer's prompt from --fix-list, --hunks, --tests and --callers files
Depth: light (runtime lines <= 50), standard (above 50), full (gate class or any DEPTH_ERROR). A full plan
from an error exits 0; exit 1 means no plan could be built (git could not list the diff).
${exitTableHelp()}`;
const args = process.argv.slice(2);
@@ -65,34 +74,17 @@ function main(): number {
if (!['none', 'full', 'fix-check'].includes(last)) { process.stderr.write('gstack-review-plan: --last must be none, full or fix-check\n'); return EXIT.usage; }
const pass = Number(flag('--pass') ?? '0');
if (!Number.isInteger(pass) || pass < 0) { process.stderr.write('gstack-review-plan: --pass needs an integer >= 0\n'); return EXIT.usage; }
const floor = flag('--depth-floor') as Depth | undefined;
if (has('--depth-floor') && floor !== 'standard' && floor !== 'full') { process.stderr.write('gstack-review-plan: --depth-floor must be standard or full (a floor only raises the depth)\n'); return EXIT.usage; }
const root = git(['rev-parse', '--show-toplevel'])?.trim();
if (!root) { process.stderr.write('gstack-review-plan: run it inside a git checkout\n'); return EXIT.fail; }
process.chdir(root);
const ref = git(['rev-parse', '--verify', '-q', `origin/${base}`]) !== null ? `origin/${base}` : base;
const mergeBase = git(['merge-base', ref, 'HEAD'])?.trim();
if (!mergeBase) { process.stderr.write(`gstack-review-plan: no merge base with ${ref} (fetch the base first)\n`); return EXIT.fail; }
const nameStatusRaw = git(['diff', '--name-status', '--no-renames', mergeBase]);
const untrackedRaw = git(['ls-files', '--others', '--exclude-standard']);
const numstatRaw = git(['diff', '--numstat', mergeBase]);
if (nameStatusRaw === null || untrackedRaw === null || numstatRaw === null) { process.stderr.write('gstack-review-plan: git could not list the diff; no plan is safer than a plan for an empty diff\n'); return EXIT.fail; }
const scratch = (p: string) => p.startsWith('.gstack/tmp/');
const nameStatus = nameStatusRaw.split('\n').filter(Boolean).map(l => l.split('\t')).filter(c => !scratch(c[1] ?? ''));
const untracked = untrackedRaw.split('\n').filter(Boolean).filter(p => !scratch(p));
const paths = [...new Set([...nameStatus.map(c => c[1]!).filter(Boolean), ...untracked])].sort();
const deleted = nameStatus.filter(c => c[0] === 'D').map(c => c[1]!);
const stampOnly = paths.filter(p => p.endsWith('package.json') && !untracked.includes(p)).filter(p => {
const changed = (git(['diff', '-U0', '--no-color', mergeBase, '--', p]) ?? '').split('\n').filter(l => /^[+-]/.test(l) && !/^(\+\+\+|---) /.test(l));
return changed.length > 0 && changed.every(l => /^[+-]\s*"version"\s*:/.test(l));
});
const diffLines = numstatRaw.split('\n').reduce((n, l) => { const [a, d] = l.split('\t'); return n + (Number(a) || 0) + (Number(d) || 0); }, 0)
+ untracked.reduce((n, p) => { try { return n + fs.readFileSync(p, 'utf8').split('\n').length; } catch { return n; } }, 0);
const added: Record<string, string[]> = {};
let current = '';
for (const line of (git(['diff', '-U0', '--no-color', mergeBase]) ?? '').split('\n')) {
if (line.startsWith('+++ b/')) { current = line.slice(6); continue; }
if (line.startsWith('+') && !line.startsWith('+++') && current) (added[current] ??= []).push(line.slice(1));
const collected = collectLive(root, base);
if (collected.errors.length) {
for (const e of collected.errors) process.stderr.write(`gstack-review-plan: git could not list the diff (${e.input}: ${e.detail}); fix: ${e.repair}; no plan is safer than a plan for an empty diff\n`);
return EXIT.fail;
}
for (const p of untracked) { try { added[p] = fs.readFileSync(p, 'utf8').split('\n'); } catch { /* unreadable: no claims */ } }
const head = git(['rev-parse', 'HEAD'])?.trim() ?? 'unknown';
const binDir = path.dirname(Bun.fileURLToPath(import.meta.url));
const scopeRun = runScriptSync(path.join(binDir, 'gstack-diff-scope'), [base], { encoding: 'utf8', timeout: 60_000 });
const scope: Record<string, string> = {};
@@ -109,14 +101,25 @@ function main(): number {
const force = flag('--force') === 'all' ? 'all' as const : list('--force');
const tiersSetting = runScriptSync(path.join(binDir, 'gstack-config'), ['get', 'review_effort_tiers'], { encoding: 'utf8', timeout: 15_000 });
const auditEffort = has('--no-tiers') || (tiersSetting.stdout ?? '').trim() === 'off' ? 'host' : 'light';
const { added, diffLines, paths, deleted, stampOnly, perPath, registry } = collected;
const plan = planReview({
paths, deleted, stampOnly, root, scope, gateTagged, diffLines, numbers: scanNumbers(added), force, gated: list('--gated'),
...collected, scope, gateTagged, numbers: scanNumbers(added), force, gated: list('--gated'), floor,
criticalFound: has('--critical-found'), adversarialRan: has('--adversarial-ran'), reviewedNumbers: flag('--reviewed-numbers'),
history: { fullPasses: pass, last, fixedBlocking: has('--fixed-blocking'), fixedBelow: has('--fixed-below'), blockingOpen: has('--blocking-open'), touchedGate: has('--touched-gate') },
});
if (has('--json')) { process.stdout.write(JSON.stringify({ ...plan, diff_lines: diffLines, pass, audit_effort: auditEffort, scope, gate_tagged: gateTagged, paths, deleted, stamp_only: stampOnly }) + '\n'); return EXIT.ok; }
const out = [`REVIEW_CLASS: ${plan.class} (${plan.reasons.slice(0, 3).join('; ') || 'no signals'})`, reviewPlanLine(plan, pass + (plan.next.step === 'full' ? 1 : 0)), `NEXT: ${plan.next.step} (${plan.next.reason})`];
for (const d of plan.dispatches) out.push(`DISPATCH: ${d.role} ${d.name} effort=${d.effort}`);
const gateScan = { tagged: gateTagged, head };
if (has('--json')) {
const { runtimeLines, stepDepth, depthReasons, ...rest } = plan;
process.stdout.write(JSON.stringify({ ...rest, step_depth: stepDepth, runtime_lines: runtimeLines, depth_reasons: depthReasons, depth_floor: floor ?? null, gate_scan: gateScan, registry: { sha: registry.sha, outputs: registry.outputs }, per_path: perPath, diff_lines: diffLines, pass, audit_effort: auditEffort, scope, gate_tagged: gateTagged, paths, deleted, stamp_only: stampOnly }) + '\n');
return EXIT.ok;
}
const out = [`REVIEW_CLASS: ${plan.class} (${plan.reasons.slice(0, 3).join('; ') || 'no signals'})`, reviewDepthLine(plan), `GATE_SCAN: ${gateTagged === null ? 'unavailable' : `tagged=${gateTagged}`} head=${head}`, reviewPlanLine(plan, pass + (plan.next.step === 'full' ? 1 : 0)), `NEXT: ${plan.next.step} (${plan.next.reason})`];
for (const e of plan.errors) out.push(`DEPTH_ERROR: ${e.code} input=${e.input} detail=${e.detail}; fix: ${e.repair}${e.result ? ` (${e.result})` : ''}`);
for (const d of plan.dispatches) {
const key = d.name.split(' ')[0];
out.push(`DISPATCH: ${d.role} ${d.name} effort=${d.effort}${d.checklists ? ` checklists=${d.checklists.join(',')}` : ''}${d.reason ? ` reason=${d.reason}` : ''}`, `DIFF_CMD: ${key} ${diffCommand(d)}`);
if (!d.exclude.length) out.push(`DIFF_EXCLUDE: ${key} none`);
}
for (const s of plan.skipped) out.push(`SKIP: ${s}`);
out.push(`AUDIT_EFFORT: ${auditEffort} (coverage, plan-completion and docs audits, decision-trail reviewers, watchers; reviewers above stay full)`);
process.stdout.write(out.join('\n') + '\n');
+79 -20
View File
@@ -128,26 +128,75 @@ applies no edits is its exit too.
<a id="review-depth"></a>
## Review depth by diff class
`gstack-review-plan --base <base>` classifies the diff and prints the reviewers
the next step dispatches (`REVIEW_CLASS:`, `REVIEW_PLAN:`, one `DISPATCH:` per
reviewer with its effort, one `SKIP:` per reviewer left out, `--json`). A mixed
diff takes its most restrictive class; a `gstack-diff-scope` error fails closed
to code, and code with the gate scan unavailable fails closed to gate.
`gstack-review-plan --base <base>` classifies the diff, picks a **depth** and prints
the reviewers the next step dispatches: `REVIEW_CLASS:`, `REVIEW_DEPTH: <d>
runtime_lines=<n> rules=<hash> reasons=<…> codes=<…>`, `GATE_SCAN: tagged=<n>|unavailable
head=<sha>`, one `DEPTH_ERROR:` per input that failed closed, one `DISPATCH:` per reviewer
with its effort and a shell-quoted `DIFF_CMD:` (plus `DIFF_EXCLUDE: <name> none` when it
reads everything), one `SKIP:` per reviewer left out, and `--json` with the same fields.
A plan that fails closed to full exits 0; a diff git cannot list exits 1.
| Class | What it is | Pass 1 dispatches |
**Classes** are unchanged. A mixed diff takes its most restrictive class: **docs**
(markdown no agent or gate reads), **tests** (test files and fixtures), **code**
(everything else, including every `SKILL.md` and template, markdown in a skill
directory, `*/sections/**`, `scripts/resolvers/**`, rule docs, checklists and bins) and
**gate** (code plus auth or migration scope, a hunk the gate scan tags, a deleted code
or test path, CI workflows, credential, state-root and the paths below).
**Runtime lines** are the changed lines of code-class paths, minus outputs the base
commit's `.gstack/generated.json` registers. Docs, tests and release metadata are never
runtime; a `.tmpl` edit plus its regenerated `SKILL.md` counts the template once.
| Depth | When | Pass 1 dispatches |
|---|---|---|
| **docs** | markdown no agent or gate reads: README, CHANGELOG, TODOS, how-to and explanation docs | the parent's core checklist only |
| **tests** | test files and fixtures only | core plus the testing specialist |
| **code** | everything else, including every `SKILL.md` and template, any markdown in a skill directory, `*/sections/**`, `scripts/resolvers/**`, `docs/*-policy.md`, `docs/captain.md`, `docs/unattended.md`, `docs/pregate.md`, checklists, specialists and bins | specialists by scope (as before), the native adversarial pass, the outside voice; the Red Team past 200 lines or after a CRITICAL |
| **gate** | code plus auth or migration scope, a hunk the gate scan tags (an RH rule: a relaxed threshold, a skipped test), a deleted code file, CI workflows, credential or state-root paths | code's set, plus the Red Team always, at full effort, and no hit-rate gating |
| **light**, runtime 0 | docs and tests only, or only registered outputs | the core checklist only; the outside voice when the diff publishes numbers |
| **light**, runtime 1–50 | `SMALL_DIFF_LINES` or fewer runtime lines | core, one **batched** specialist (testing, maintainability and security checklists, the specialists scope selects and the Red Team prompt in one context), the native adversarial pass (kept: backtest inconclusive, GSTA-147 D5) and the outside voice on numbers |
| **standard** | more than 50 runtime lines | the code set with every threshold (specialists at 50, simplification at 100, Red Team at 200) read on runtime lines; specialists leave docs and registered outputs out of their diff, and every specialist but Testing leaves tests out too; adversarial and outside voice read everything |
| **full** | gate class, or any classifier error | the gate set, unchanged: every reviewer on the whole diff, the Red Team always, no hit-rate gating |
**Numbers keep the outside voice.** On a docs or tests diff the outside voice is
skipped unless the diff publishes numeric claims: `gstack-review-plan` scans the
added lines of every path for percentages, money, multipliers, durations, token
or credit counts and score, rate or speed claims (`NUMBERS: yes`), and a
claim always dispatches it. The claims' fingerprint (`fingerprint=`) is passed
back as `--reviewed-numbers`; when a later fix changes a claim, only the
outside voice reruns, on the numbers.
A fixed blocking finding, a CRITICAL or an open blocker raises a light diff to
standard for the next step (`step=standard`): its specialists read at least 50 lines
and the Red Team runs, so a raise never dispatches less than light did. A fix-check
is the same at every depth.
**Fail closed.** Each of these makes the depth full and is listed in `errors[]` with
the input it could not read, a sanitized cause and the repair: a `gstack-diff-scope`
error, a gate scan that is unavailable, a base registry that cannot be read or is
invalid (`REGEN_REGISTRY_INVALID` or `REGEN_CONTAINMENT` in `codes=`), a numstat git
could not produce, and a binary code-class path (it is never counted as 0 lines).
**The registry rule.** Only the base commit's registry counts (the merge base's
`origin/<base>` commit for live /ship, the receipt's base sha for the queue), and only
entries with a `command`, which pregate's regen check proves for that tree. An entry
added on the branch never excludes anything; `command: null` entries, vendored code
and lockfiles stay runtime. To adopt it, commit an entry like this on the base branch:
```json
{
"$schema": "https://github.com/garrytan/gstack/docs/pregate.md#generated-v1",
"entries": [
{ "id": "skill-docs", "command": "bun run gen:skill-docs --host all", "outputs": ["*/SKILL.md", "*/sections/*.md"], "inputs": ["*/SKILL.md.tmpl", "*/sections/*.tmpl"] }
]
}
```
**Gate paths for the machinery itself.** An edit to `lib/review-depth.ts`,
`lib/review-collect.ts`, `bin/gstack-review-plan`, `lib/review-scope.ts`,
`lib/ship-queue*.ts`, `lib/regen.ts`, `.gstack/generated.json`,
`ship/sections/review-army.md.tmpl`, `ship/sections/adversarial.md.tmpl` or
`scripts/resolvers/review-army.ts` is gate class: changing how much review runs gets
the most review. `rules=` (`DEPTH_RULES`) hashes the deciding constants and patterns,
not file bytes, so a comment edit keeps it.
**`--depth-floor standard|full`** (a /ship prompt flag, passed to every plan run)
raises the depth and never lowers it.
**Numbers keep the outside voice.** At light depth the outside voice runs only when
the diff publishes numeric claims: `gstack-review-plan` scans the added lines of
prose and data files for percentages, money, multipliers, durations, token or credit
counts and score, rate or speed claims (`NUMBERS: yes`). The claims' fingerprint
(`fingerprint=`) is passed back as `--reviewed-numbers`; when a later fix changes a
claim, only the outside voice reruns, on the numbers. It always reads the whole diff.
**The adversarial and outside voices run once per invocation** (Step 11, on the
final tree), as the last full-pass plan lists them; after they ran, decision 2
@@ -166,6 +215,15 @@ default. `gstack-config set review_effort_tiers off` (or `--no-tiers`) runs
the audits and watchers at the host default too
([model-policy.md](model-policy.md#reasoning-effort)).
### Behavior changes
- A tests-only diff (runtime 0) no longer dispatches the Testing specialist: the
core checklist and the gate scan review it.
- Light no longer runs a separate Red Team: the batched specialist carries its
prompt. At runtime 0 light runs no adversarial pass either.
- Standard reads thresholds on runtime lines, not the whole diff, so a test-heavy
diff with few runtime lines dispatches fewer specialists.
## What the receipt records
`gstack-review-scope summary --since <REVIEW_SINCE>` reads this invocation's
@@ -268,9 +326,10 @@ trailers, appended }`.
## Where this is enforced
- `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), 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: 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-depth.ts`, `bin/gstack-review-plan`: the diff class, the transition table, the numeric-claim scan and each role's effort tier (`test/review-depth.test.ts`, `test/review-plan-cli.test.ts`).
- `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-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)).
- `lib/ship-policy.ts`: the `review_scope` key and its `ship: review_scope …` consequence line.
- `review/checklist.md`: `prereg-with-results` under Gate Integrity and `test-keyboard-count-loop` under Pass 2; `bin/gstack-coauthor-trailers` for the contributor-credit trailers of a port.
+137
View File
@@ -0,0 +1,137 @@
/**
* review-collect — the git reads behind a review depth (docs/review-policy.md#review-depth),
* kept out of lib/review-depth.ts so the policy stays pure. collectCommit reads git
* objects only (a range between two commits, from any checkout, dirty or not);
* collectLive reads the working tree as /ship reviews it (committed, tracked
* uncommitted edits and untracked files). Both return a ClassInput with per-path
* line counts, the base commit's generated-output registry and collector errors;
* the caller adds the gate-scan result.
*/
import { spawnSync } from 'node:child_process';
import * as fs from 'node:fs';
import * as path from 'node:path';
import { GENERATED_PATH, parseRegistry } from './regen';
import { RESULT_CODES } from './result-codes';
import type { ClassInput, DepthError, PathLines, RegistryInfo } from './review-depth';
export interface Collected extends ClassInput {
perPath: PathLines[];
registry: RegistryInfo;
errors: DepthError[];
deleted: string[];
stampOnly: string[];
/** Added lines per path, for scanNumbers. */
added: Record<string, string[]>;
diffLines: number;
mergeBase: string | null;
untracked: string[];
}
function git(root: string, args: string[]): { ok: boolean; out: string; err: string } {
const r = spawnSync('git', ['-c', 'core.quotePath=false', ...args], { cwd: root, encoding: 'utf8', timeout: 60_000, maxBuffer: 256 * 1024 * 1024 });
return { ok: r.status === 0, out: r.stdout ?? '', err: (r.stderr ?? '').trim() };
}
const scratch = (p: string) => p.startsWith('.gstack/tmp/');
const oneLine = (s: string) => s.replace(/[\0-\x1f]+/g, ' ').trim().slice(0, 200);
const unavailable = (input: string, detail: string): RegistryInfo => ({ sha: null, outputs: [], error: { code: 'registry_unavailable', detail: oneLine(detail), input, repair: 'fetch the base branch (git fetch origin <base>) so its commit and registry resolve' } });
/** The registry as committed at `sha`: outputs of entries with a command (a `command: null` entry never excludes); a missing file is an empty registry. */
export function loadRegistryAt(root: string, sha: string): RegistryInfo {
const commit = git(root, ['rev-parse', '--verify', '--quiet', `${sha}^{commit}`]);
if (!commit.ok) return unavailable(`${sha}:${GENERATED_PATH}`, `${sha} is not a commit in this repository`);
const resolved = commit.out.trim();
const input = `${resolved.slice(0, 12)}:${GENERATED_PATH}`;
if (!git(root, ['cat-file', '-e', `${resolved}:${GENERATED_PATH}`]).ok) return { sha: resolved, outputs: [] };
const show = git(root, ['show', `${resolved}:${GENERATED_PATH}`]);
if (!show.ok) return unavailable(input, show.err || 'git show failed');
const { registry, errors } = parseRegistry(show.out);
const e = errors[0];
if (e) return { sha: resolved, outputs: [], error: { code: 'registry_invalid', detail: oneLine(`${e.path || GENERATED_PATH}: ${e.message}`), input, repair: RESULT_CODES[e.code].fix, result: e.code } };
return { sha: resolved, outputs: registry.entries.filter(x => x.command !== null).flatMap(x => x.outputs) };
}
/** Numstat, deletions and the -U0 patch for `range` (`[mergeBase, head]` or `[mergeBase]` for the working tree). */
function readDiff(root: string, range: string[], errors: DepthError[]): Pick<Collected, 'perPath' | 'deleted' | 'stampOnly' | 'added'> {
const numstat = git(root, ['diff', '--numstat', '-z', '--no-renames', ...range]);
const status = git(root, ['diff', '--name-status', '-z', '--no-renames', '--diff-filter=D', ...range]);
const patch = git(root, ['diff', '-U0', '--no-color', '--no-ext-diff', '--no-renames', ...range]);
const empty = { perPath: [], deleted: [], stampOnly: [], added: {} };
for (const [r, what] of [[numstat, '--numstat'], [status, '--name-status'], [patch, '-U0']] as const) {
if (!r.ok) { errors.push({ code: 'numstat_failed', detail: oneLine(r.err || 'git diff failed'), input: `git diff ${what} ${range.join(' ')}`, repair: 'check that both commits exist locally (git fetch origin <base>) and rerun' }); return empty; }
}
const perPath: PathLines[] = [];
for (const rec of numstat.out.split('\0').filter(Boolean)) {
const [a, d, ...rest] = rec.split('\t');
const p = rest.join('\t');
if (!scratch(p)) perPath.push({ path: p, lines: (Number(a) || 0) + (Number(d) || 0), binary: a === '-' && d === '-' });
}
const statusParts = status.out.split('\0');
const deleted = statusParts.flatMap((s, k) => (s === 'D' && statusParts[k + 1] ? [statusParts[k + 1]!] : [])).filter(p => !scratch(p));
const added: Record<string, string[]> = {};
const changed: Record<string, string[]> = {};
let cur = '';
let minus = '';
let inHunk = false;
for (const line of patch.out.split('\n')) {
if (line.startsWith('diff --git ')) { inHunk = false; cur = ''; minus = ''; continue; }
if (!inHunk) {
if (line.startsWith('--- ')) minus = line === '--- /dev/null' ? '' : line.slice(6).replace(/\t$/, '');
else if (line.startsWith('+++ ')) cur = line === '+++ /dev/null' ? minus : line.slice(6).replace(/\t$/, '');
else if (line.startsWith('@@')) inHunk = true;
continue;
}
if (!cur || scratch(cur) || line.startsWith('@@')) continue;
if (line[0] === '+') (added[cur] ??= []).push(line.slice(1));
if (line[0] === '+' || line[0] === '-') (changed[cur] ??= []).push(line);
}
const stampOnly = perPath.map(e => e.path).filter(p => p.endsWith('package.json') && !!changed[p]?.length && changed[p]!.every(l => /^[+-]\s*"version"\s*:/.test(l)));
return { perPath, deleted, stampOnly, added };
}
function result(diff: ReturnType<typeof readDiff>, extra: Pick<Collected, 'root' | 'registry' | 'errors' | 'mergeBase' | 'untracked'>): Collected {
const paths = diff.perPath.map(e => e.path).sort();
return { ...diff, ...extra, paths, gateTagged: null, diffLines: diff.perPath.reduce((n, e) => n + e.lines, 0) };
}
/** The range `baseSha`...`headSha` from git objects alone; the registry is the base commit's, never the head's. */
export function collectCommit(root: string, baseSha: string, headSha: string): Collected {
const errors: DepthError[] = [];
const registry = loadRegistryAt(root, baseSha);
const mb = git(root, ['merge-base', baseSha, headSha]);
if (!mb.ok) {
errors.push({ code: 'numstat_failed', detail: oneLine(mb.err || 'no merge base'), input: `git merge-base ${baseSha} ${headSha}`, repair: 'check that both commits exist locally (git fetch origin <base>) and rerun' });
return result({ perPath: [], deleted: [], stampOnly: [], added: {} }, { root: [], registry, errors, mergeBase: null, untracked: [] });
}
const mergeBase = mb.out.trim();
const diff = readDiff(root, [mergeBase, headSha], errors);
const tree = git(root, ['ls-tree', '-r', '--name-only', '-z', headSha]);
const skillDirs = [...new Set(tree.out.split('\0').flatMap(p => /^([^/]+)\/SKILL\.md(\.tmpl)?$/.exec(p)?.[1] ?? []))].sort();
return result(diff, { root: skillDirs, registry, errors, mergeBase, untracked: [] });
}
/** The working tree against the merge base with origin/<base> (or <base>), untracked files counted per path; the registry is the base ref's commit. */
export function collectLive(root: string, base: string): Collected {
const errors: DepthError[] = [];
const ref = git(root, ['rev-parse', '--verify', '-q', `origin/${base}^{commit}`]).ok ? `origin/${base}` : base;
const registry = loadRegistryAt(root, ref);
const mb = git(root, ['merge-base', ref, 'HEAD']);
if (!mb.ok) {
errors.push({ code: 'numstat_failed', detail: oneLine(mb.err || `no merge base with ${ref}`), input: `git merge-base ${ref} HEAD`, repair: `fetch the base first (git fetch origin ${base})` });
return result({ perPath: [], deleted: [], stampOnly: [], added: {} }, { root, registry, errors, mergeBase: null, untracked: [] });
}
const mergeBase = mb.out.trim();
const diff = readDiff(root, [mergeBase], errors);
const others = git(root, ['ls-files', '--others', '--exclude-standard', '-z']);
if (!others.ok) errors.push({ code: 'numstat_failed', detail: oneLine(others.err || 'git ls-files failed'), input: 'git ls-files --others --exclude-standard', repair: 'run the command by hand and fix what git reports' });
const untracked = others.out.split('\0').filter(p => p && !scratch(p)).sort();
for (const p of untracked) {
let buf: Buffer;
try { buf = fs.readFileSync(path.join(root, p)); } catch { diff.perPath.push({ path: p, lines: 0, binary: true }); continue; }
const binary = buf.subarray(0, 8000).includes(0);
const text = binary ? '' : buf.toString('utf8');
const lines = text ? text.split('\n').length - (text.endsWith('\n') ? 1 : 0) : 0;
diff.perPath.push({ path: p, lines, binary });
if (!binary) diff.added[p] = text.split('\n');
}
return result(diff, { root, registry, errors, mergeBase, untracked });
}
+176 -43
View File
@@ -1,17 +1,22 @@
/**
* review-depth — the one policy owner for how much review a /ship run dispatches
* (docs/review-policy.md#review-depth). A pure function from the changed paths,
* gstack-diff-scope flags, the diff size, the gate scan and the pass history to a
* diff class, the next step of the loop and the reviewers that step dispatches,
* each with its reasoning-effort tier. bin/gstack-review-plan prints it; the
* (docs/review-policy.md#review-depth). A pure function from the changed paths and
* their line counts, gstack-diff-scope flags, the base registry's generated outputs,
* the gate scan and the pass history to a diff class, a review depth (light,
* standard, full), the next step of the loop and the reviewers that step
* dispatches, each with its effort tier and the pathspecs it leaves out. Git reads
* live in lib/review-collect.ts; bin/gstack-review-plan prints the plan; the
* review-army resolver and the skills read the same definitions.
*/
import { createHash } from 'node:crypto';
import * as fs from 'node:fs';
import * as path from 'node:path';
import type { ResultCodeName } from './result-codes';
import { matchesAny } from './ship-policy';
export type DiffClass = 'docs' | 'tests' | 'code' | 'gate';
export type PathClass = 'docs' | 'tests' | 'code';
export type Depth = 'light' | 'standard' | 'full';
export type Effort = 'light' | 'full' | 'host';
export type Role = 'specialist' | 'red-team' | 'adversarial-native' | 'outside-voice' | 'fix-check';
export type Step = 'full' | 'fix-check' | 'done' | 'stop';
@@ -22,7 +27,12 @@ export const FULL_PASS_CAP = 2;
export const SMALL_DIFF_LINES = 50;
export const RED_TEAM_LINES = 200;
export const LARGE_DIFF_LINES = 100;
/** The checklists light's batched specialist always reads, whatever the scope. */
export const LIGHT_FIXED: readonly Specialist[] = ['testing', 'maintainability', 'security'];
/** Why light with runtime above 0 keeps the native adversarial pass as its own dispatch. */
export const LIGHT_ADVERSARIAL_REASON = 'backtest inconclusive (GSTA-147 D5)';
const CLASS_RANK: Record<DiffClass, number> = { docs: 0, tests: 1, code: 2, gate: 3 };
const DEPTH_RANK: Record<Depth, number> = { light: 0, standard: 1, full: 2 };
const AGENT_PROSE = [/(^|\/)SKILL\.md(\.tmpl)?$/, /(^|\/)sections\//, /^scripts\/resolvers\//, /^docs\/[^/]*-policy\.md$/, /^docs\/(captain|unattended|pregate)\.md$/, /(^|\/)DRIVER\.md(\.tmpl)?$/, /(^|\/)templates\//, /(^|\/)(AGENTS|CLAUDE|ETHOS)\.md$/, /(^|\/)specialists\//, /(^|\/)checklist\.md$/, /^\.gstack\//];
/** gstack-diff-scope's TEST_PATH_RE (the test-bootstrap rule), plus fixture directories. */
@@ -30,19 +40,41 @@ const TEST_PATH = [/(^|\/)(tests?|spec|__tests__)\/|(^|\/)tests?\.py$|(^|\/)test
const DOC_PATH = /\.(md|mdx|markdown|rst|txt|adoc)$/i;
/** Release metadata a version stamp writes: never a reason for review depth on its own. */
const RELEASE_METADATA = [/^CHANGELOG\.md$/, /^VERSION$/, /^agents-digest\//];
const GATE_PATH = [/(^|\/)\.github\/workflows\//, /(^|\/)(secrets?|credentials?|keychain|token[s]?)([._/-]|$)/i, /state-root/, /(^|\/)(pregate|gate|ship-receipt|ship-policy)[^/]*\.(ts|sh|json)$/, /(^|\/)bin\/gstack-(pregate|gate|ship-receipt|verify-gate|ci-gate)$/];
/** Gate paths, the review-depth machinery included: an edit to how much review runs gets the most review. */
const GATE_PATH = [
/(^|\/)\.github\/workflows\//, /(^|\/)(secrets?|credentials?|keychain|token[s]?)([._/-]|$)/i, /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)\.ts$/, /^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$/,
];
/** One changed path's class. Markdown an agent or a gate reads (skills, sections, resolvers, policy and rule docs, templates, any markdown in a skill directory under `root`) is code: it is gstack's product. */
export function classifyPath(p: string, root?: string): PathClass {
/** A hash of the constants and patterns that decide a depth, not of this file's bytes: a comment edit keeps it, a threshold edit changes it. */
export const DEPTH_RULES = createHash('sha256').update(JSON.stringify({
SMALL_DIFF_LINES, RED_TEAM_LINES, LARGE_DIFF_LINES, FULL_PASS_CAP, LIGHT_FIXED,
patterns: Object.entries({ GATE_PATH, AGENT_PROSE, TEST_PATH, RELEASE_METADATA, DOC_PATH: [DOC_PATH] }).map(([k, v]) => [k, v.map(r => `/${r.source}/${r.flags}`)]),
})).digest('hex').slice(0, 16);
/**
* One changed path's class. Markdown an agent or a gate reads (skills, sections, resolvers, policy and rule docs,
* templates, any markdown in a skill directory) is code: it is gstack's product. `root` is the repository root
* (skill directories probed on disk) or the top-level skill directory names read from a commit.
*/
export function classifyPath(p: string, root?: string | readonly string[]): PathClass {
if (RELEASE_METADATA.some(r => r.test(p))) return 'docs';
if (AGENT_PROSE.some(r => r.test(p))) return 'code';
if (TEST_PATH.some(r => r.test(p))) return 'tests';
if (!DOC_PATH.test(p)) return 'code';
const top = p.includes('/') ? p.split('/')[0]! : '';
if (root && top && ['SKILL.md', 'SKILL.md.tmpl'].some(f => fs.existsSync(path.join(root, top, f)))) return 'code';
return 'docs';
if (!top || !root) return 'docs';
if (typeof root === 'string') return ['SKILL.md', 'SKILL.md.tmpl'].some(f => fs.existsSync(path.join(root, top, f))) ? 'code' : 'docs';
return root.includes(top) ? 'code' : 'docs';
}
export type DepthErrorCode = 'scope_error' | 'gate_scan_unavailable' | 'registry_unavailable' | 'registry_invalid' | 'numstat_failed' | 'binary_code_path';
/** Why a depth failed closed to full: the input it could not read, a sanitized cause and the repair; `result` is the result code behind it when one exists. */
export interface DepthError { code: DepthErrorCode; detail: string; input: string; repair: string; result?: ResultCodeName }
export interface PathLines { path: string; lines: number; binary: boolean }
/** The base commit's `.gstack/generated.json`: outputs of entries with a command only; `error` when it could not be read or validated. */
export interface RegistryInfo { sha: string | null; outputs: readonly string[]; error?: DepthError }
export interface ClassInput {
paths: readonly string[];
/** gstack-diff-scope output (SCOPE_* keys). */
@@ -53,23 +85,38 @@ export interface ClassInput {
deleted?: readonly string[];
/** Paths whose whole change is a version stamp (package.json's version key): classified as docs. */
stampOnly?: readonly string[];
/** The repository root: markdown inside a skill directory there is code. */
root?: string;
/** The repository root, or the skill directory names at the reviewed commit: markdown inside a skill directory is code. */
root?: string | readonly string[];
/** Changed lines per path; without it a code diff counts every line as runtime. */
perPath?: readonly PathLines[];
registry?: RegistryInfo;
/** Collector failures (a diff git could not list): each fails the depth closed. */
errors?: readonly DepthError[];
}
export interface ClassResult { class: DiffClass; reasons: string[] }
export interface ClassResult { class: DiffClass; reasons: string[]; errors: DepthError[]; runtimeLines: number | null }
/** The diff's class: the most restrictive of its paths, raised to gate by any gate signal; errors fail closed. */
const sanitize = (s: string) => s.replace(/[\0-\x1f]+/g, ' ').slice(0, 200);
const isOutput = (i: ClassInput, p: string) => !!i.registry?.outputs.length && matchesAny(i.registry.outputs, p);
const pathClass = (i: ClassInput, p: string): PathClass => ((i.stampOnly ?? []).includes(p) ? 'docs' : classifyPath(p, i.root));
/** The diff's class, raised to gate by any gate signal; `errors` lists every input that fails the depth closed; `runtimeLines` counts code-class lines outside the base registry's outputs. */
export function classifyDiff(i: ClassInput): ClassResult {
const reasons: string[] = [];
const errors: DepthError[] = [...(i.errors ?? [])];
let cls: DiffClass = 'docs';
const raise = (c: DiffClass, why: string) => { if (CLASS_RANK[c] > CLASS_RANK[cls]) cls = c; reasons.push(why); };
if (!i.paths.length) raise('docs', 'no changed paths');
for (const p of i.paths) { const c = (i.stampOnly ?? []).includes(p) ? 'docs' : classifyPath(p, i.root); if (CLASS_RANK[c] > CLASS_RANK[cls]) raise(c, `${c}: ${p}`); }
for (const p of i.paths) { const c = pathClass(i, p); if (CLASS_RANK[c] > CLASS_RANK[cls]) raise(c, `${c}: ${p}`); }
const s = i.scope ?? {};
if (s.SCOPE_ERROR) raise('code', `scope error: ${s.SCOPE_ERROR}`);
if (s.SCOPE_ERROR) {
raise('code', `scope error: ${s.SCOPE_ERROR}`);
errors.push({ code: 'scope_error', detail: sanitize(s.SCOPE_ERROR), input: 'gstack-diff-scope <base>', repair: 'run gstack-diff-scope <base> by hand and fix what it reports' });
}
if (CLASS_RANK[cls] >= CLASS_RANK.tests) {
if (i.gateTagged === null) raise('gate', 'gate scan unavailable');
else if (i.gateTagged > 0) raise('gate', `gate scan tagged ${i.gateTagged} hunk(s)`);
if (i.gateTagged === null) {
raise('gate', 'gate scan unavailable');
errors.push({ code: 'gate_scan_unavailable', detail: 'gstack-gate-diff reported no GATE_SUMMARY', input: 'gstack-gate-diff <base>', repair: 'run gstack-gate-diff <base> and read its GATE_ERROR line, or pass the recorded scan as --gate-tagged <n>' });
} else if (i.gateTagged > 0) raise('gate', `gate scan tagged ${i.gateTagged} hunk(s)`);
const deletedGuarded = (i.deleted ?? []).filter(p => classifyPath(p, i.root) !== 'docs');
if (deletedGuarded.length) raise('gate', `deletes ${deletedGuarded.length} code or test path(s)`);
}
@@ -77,8 +124,38 @@ export function classifyDiff(i: ClassInput): ClassResult {
if (s.SCOPE_AUTH === 'true') raise('gate', 'auth paths');
if (s.SCOPE_MIGRATIONS === 'true') raise('gate', 'migrations');
for (const p of i.paths) if (GATE_PATH.some(r => r.test(p))) { raise('gate', `gate path: ${p}`); break; }
if (i.registry?.error) errors.push(i.registry.error);
}
return { class: cls, reasons };
if (!i.perPath) return { class: cls, reasons, errors, runtimeLines: null };
let runtimeLines = 0;
for (const e of i.perPath) {
if (pathClass(i, e.path) !== 'code' || isOutput(i, e.path)) continue;
if (e.binary) errors.push({ code: 'binary_code_path', detail: 'git reports a binary change, so its lines cannot be counted', input: e.path, repair: "register it as an output of an entry with a command in the base branch's .gstack/generated.json, or accept the full review" });
else runtimeLines += e.lines;
}
return { class: cls, reasons, errors, runtimeLines };
}
/** Any classifier error or gate class is full; runtime above SMALL_DIFF_LINES is standard; otherwise light. A floor only raises. */
export function depthFor(o: { cls: DiffClass; runtimeLines: number; errors: readonly DepthError[]; floor?: Depth }): Depth {
const d: Depth = o.errors.length || o.cls === 'gate' ? 'full' : o.runtimeLines > SMALL_DIFF_LINES ? 'standard' : 'light';
return o.floor && DEPTH_RANK[o.floor] > DEPTH_RANK[d] ? o.floor : d;
}
export interface DepthResult { depth: Depth; class: DiffClass; runtimeLines: number; reasons: string[]; errors: DepthError[]; codes: ResultCodeName[]; rules: string }
function decideDepth(c: ClassResult, runtimeLines: number, floor?: Depth): DepthResult {
const unfloored = depthFor({ cls: c.class, runtimeLines, errors: c.errors });
const reasons = [...c.errors.map(e => `error:${e.code}`), ...(c.class === 'gate' ? ['gate-class'] : []), `runtime=${runtimeLines}${runtimeLines > SMALL_DIFF_LINES ? '>' : '<='}${SMALL_DIFF_LINES}`];
if (floor && DEPTH_RANK[floor] > DEPTH_RANK[unfloored]) reasons.push(`floor:${floor}`);
const codes = [...new Set(c.errors.flatMap(e => (e.result ? [e.result] : [])))];
return { depth: depthFor({ cls: c.class, runtimeLines, errors: c.errors, floor }), class: c.class, runtimeLines, reasons, errors: c.errors, codes, rules: DEPTH_RULES };
}
/** The depth of a collected range (lib/review-collect.ts) plus a recorded gate-scan result: what the queue recomputes at a head. */
export function depthOfCommitInput(input: ClassInput & { floor?: Depth }): DepthResult {
const c = classifyDiff(input);
return decideDepth(c, c.runtimeLines ?? 0, input.floor);
}
// ---------------------------------------------------------------------------
@@ -153,34 +230,48 @@ export interface PlanInput extends ClassInput {
force?: readonly string[] | 'all';
/** Specialists the hit-rate stats tag GATE_CANDIDATE. */
gated?: readonly string[];
/** A specialist produced a CRITICAL finding (Red Team activation on code class). */
/** A specialist produced a CRITICAL finding (Red Team activation on code class; raises light to standard). */
criticalFound?: boolean;
/** Step 11's native adversarial pass already ran in this invocation. */
adversarialRan?: boolean;
/** The numeric-claim fingerprint the outside voice last reviewed; a change reruns it. */
reviewedNumbers?: string;
/** --depth-floor: raises the depth, never lowers it. */
floor?: Depth;
}
export interface Dispatch { role: Role; name: string; effort: Effort }
export interface ReviewPlan { class: DiffClass; reasons: string[]; next: NextStep; numbers: NumbersResult; dispatches: Dispatch[]; skipped: string[] }
/** One reviewer. `exclude` holds git pathspec argv items the reviewer's diff leaves out; `checklists` names what light's batched specialist reads. */
export interface Dispatch { role: Role; name: string; effort: Effort; exclude: string[]; checklists?: string[]; reason?: string }
export interface ReviewPlan extends DepthResult {
/** The depth this step runs at: light raised to standard by a fixed blocker, a CRITICAL or an open blocker. */
stepDepth: Depth;
depthReasons: string[];
next: NextStep; numbers: NumbersResult; dispatches: Dispatch[]; skipped: string[];
}
const scopeWants: Partial<Record<Specialist, (s: Record<string, string>) => boolean>> = {
performance: s => s.SCOPE_BACKEND === 'true' || s.SCOPE_FRONTEND === 'true',
'data-migration': s => s.SCOPE_MIGRATIONS === 'true',
'api-contract': s => s.SCOPE_API === 'true',
design: s => s.SCOPE_FRONTEND === 'true',
'state-machines': s => s.SCOPE_STATE_MACHINE === 'true' || s.SCOPE_STATE_MACHINE === 'unknown' || s.SCOPE_STATE_MACHINE === undefined,
};
const forcedBy = (i: PlanInput) => (n: string) => i.force === 'all' || (Array.isArray(i.force) && i.force.includes(n));
/** The specialists a full pass selects for a class: class sets the eligible set, hit-rate gating applies inside it, gate class ignores gating. */
export function specialistsFor(cls: DiffClass, i: PlanInput, skipped: string[] = []): Specialist[] {
const s = i.scope ?? {};
const forced = (n: string) => i.force === 'all' || (Array.isArray(i.force) && i.force.includes(n));
const forced = forcedBy(i);
if (cls === 'docs') { skipped.push('specialists (docs class)'); return SPECIALISTS.filter(forced); }
if (cls === 'tests') { skipped.push('specialists except testing (tests class)'); return SPECIALISTS.filter(n => n === 'testing' || forced(n)); }
const want = (n: Specialist): boolean => {
if (forced(n)) return true;
if (n === 'state-machines') return s.SCOPE_STATE_MACHINE === 'true' || s.SCOPE_STATE_MACHINE === 'unknown' || s.SCOPE_STATE_MACHINE === undefined;
if (n === 'state-machines') return scopeWants[n]!(s);
if (i.diffLines < SMALL_DIFF_LINES) return false;
switch (n) {
case 'testing': case 'maintainability': return true;
case 'security': return s.SCOPE_AUTH === 'true' || (s.SCOPE_BACKEND === 'true' && i.diffLines > LARGE_DIFF_LINES);
case 'performance': return s.SCOPE_BACKEND === 'true' || s.SCOPE_FRONTEND === 'true';
case 'data-migration': return s.SCOPE_MIGRATIONS === 'true';
case 'api-contract': return s.SCOPE_API === 'true';
case 'design': return s.SCOPE_FRONTEND === 'true';
case 'simplification': return i.diffLines > LARGE_DIFF_LINES;
default: return scopeWants[n]!(s);
}
};
return SPECIALISTS.filter(n => {
@@ -190,34 +281,76 @@ export function specialistsFor(cls: DiffClass, i: PlanInput, skipped: string[] =
});
}
/** The reviewers the next step dispatches, with each role's effort tier. */
/** A path as one git exclude pathspec; a glob character or a leading colon makes it literal. */
const excludeSpec = (p: string) => `:(exclude${/[*?[\]\\]/.test(p) || p.startsWith(':') ? ',literal' : ''})${p}`;
const shellQuote = (s: string) => `'${s.replace(/'/g, "'\\''")}'`;
/** The diff command a dispatched reviewer runs; `<diff-base>` is the literal sha the parent writes in. */
export function diffCommand(d: Dispatch): string {
return d.exclude.length ? `git diff <diff-base> -- . ${d.exclude.map(shellQuote).join(' ')}` : 'git diff <diff-base>';
}
/** The reviewers the next step dispatches at its depth, with each role's effort tier and excluded pathspecs. */
export function planReview(i: PlanInput): ReviewPlan {
const { class: cls, reasons } = classifyDiff(i);
const c = classifyDiff(i);
const depth = decideDepth(c, c.runtimeLines ?? (CLASS_RANK[c.class] >= CLASS_RANK.code ? i.diffLines : 0), i.floor);
const { class: cls, runtimeLines } = depth;
const next = nextStep(i.history);
const h = i.history;
const raise = depth.depth === 'light' && (h.fixedBlocking || h.blockingOpen || i.criticalFound);
const stepDepth: Depth = raise ? 'standard' : depth.depth;
const depthReasons = [...depth.reasons, ...(raise ? [`history:${h.fixedBlocking ? 'fixed-blocking' : h.blockingOpen ? 'blocking-open' : 'critical-found'}`] : [])];
const skipped: string[] = [];
const dispatches: Dispatch[] = [];
const add = (role: Role, name: string, effort: Effort, exclude: string[] = [], extra: Partial<Dispatch> = {}) => dispatches.push({ role, name, effort, exclude, ...extra });
const docsAndOutputs = i.paths.filter(p => pathClass(i, p) === 'docs' || isOutput(i, p)).map(excludeSpec);
const tests = i.paths.filter(p => pathClass(i, p) === 'tests' && !isOutput(i, p)).map(excludeSpec);
const numbersChanged = i.numbers.publishes && i.reviewedNumbers !== undefined && i.reviewedNumbers !== i.numbers.fingerprint;
if (next.step === 'full') {
for (const n of specialistsFor(cls, i, skipped)) dispatches.push({ role: 'specialist', name: n, effort: 'full' });
const redTeam = cls === 'gate' || (cls === 'code' && (i.diffLines > RED_TEAM_LINES || !!i.criticalFound));
if (redTeam) dispatches.push({ role: 'red-team', name: 'red-team', effort: 'full' });
else skipped.push(`red-team (${cls} class${cls === 'code' ? `, ${i.diffLines} lines, no critical` : ''})`);
if (i.history.fullPasses === 0 || (!i.adversarialRan && (cls === 'code' || cls === 'gate'))) {
if (cls === 'docs' || cls === 'tests') skipped.push(`adversarial-native (${cls} class)`);
else dispatches.push({ role: 'adversarial-native', name: 'adversarial', effort: 'full' });
if (cls === 'code' || cls === 'gate' || i.numbers.publishes) dispatches.push({ role: 'outside-voice', name: 'outside-voice', effort: 'host' });
else skipped.push(`outside-voice (${cls} class, no numeric claims)`);
const firstVoices = h.fullPasses === 0 || !i.adversarialRan;
if (next.step === 'full' && stepDepth === 'light') {
const forced = forcedBy(i);
if (runtimeLines > 0) {
const scoped = SPECIALISTS.filter(n => !LIGHT_FIXED.includes(n) && (forced(n) || (scopeWants[n]?.(i.scope ?? {}) && !(i.gated ?? []).includes(n))));
add('specialist', 'batched', 'full', docsAndOutputs, { checklists: [...LIGHT_FIXED, ...scoped, 'red-team'] });
skipped.push('red-team (light depth: the batched specialist carries the Red Team prompt)');
if (firstVoices) add('adversarial-native', 'adversarial', 'full', [], { reason: LIGHT_ADVERSARIAL_REASON });
} else {
for (const n of SPECIALISTS.filter(forced)) add('specialist', n, 'full');
skipped.push('specialists, red-team and adversarial-native (light depth, runtime 0)');
}
if (h.fullPasses === 0 && i.numbers.publishes) add('outside-voice', 'outside-voice', 'host');
else if (numbersChanged) add('outside-voice', 'outside-voice (numbers changed)', 'host');
else skipped.push(`outside-voice (${cls} class, no numeric claims)`);
} else if (next.step === 'full') {
const full = stepDepth === 'full';
const dcls: DiffClass = full ? 'gate' : cls;
const raised = !full && runtimeLines <= SMALL_DIFF_LINES;
const lines = full ? i.diffLines : Math.max(runtimeLines, raised ? SMALL_DIFF_LINES : 0);
for (const n of specialistsFor(dcls, { ...i, diffLines: lines }, skipped)) add('specialist', n, 'full', full ? [] : n === 'testing' ? docsAndOutputs : [...docsAndOutputs, ...tests]);
const redTeam = dcls === 'gate' || (dcls === 'code' && (lines > RED_TEAM_LINES || !!i.criticalFound || raised));
if (redTeam) add('red-team', 'red-team', 'full');
else skipped.push(`red-team (${dcls} class${dcls === 'code' ? `, ${lines} runtime lines, no critical` : ''})`);
const codeLike = dcls === 'code' || dcls === 'gate';
if (h.fullPasses === 0 || (!i.adversarialRan && codeLike)) {
if (codeLike) add('adversarial-native', 'adversarial', 'full');
else skipped.push(`adversarial-native (${dcls} class)`);
if (codeLike || i.numbers.publishes) add('outside-voice', 'outside-voice', 'host');
else skipped.push(`outside-voice (${dcls} class, no numeric claims)`);
} else {
skipped.push('adversarial-native and outside-voice (run once per invocation)');
if (numbersChanged) dispatches.push({ role: 'outside-voice', name: 'outside-voice (numbers changed)', effort: 'host' });
if (numbersChanged) add('outside-voice', 'outside-voice (numbers changed)', 'host');
}
} else if (next.step === 'fix-check') {
dispatches.push({ role: 'fix-check', name: 'fix-check', effort: 'full' });
if (numbersChanged) dispatches.push({ role: 'outside-voice', name: 'outside-voice (numbers changed)', effort: 'host' });
add('fix-check', 'fix-check', 'full');
if (numbersChanged) add('outside-voice', 'outside-voice (numbers changed)', 'host');
}
return { class: cls, reasons, next, numbers: i.numbers, dispatches, skipped };
return { ...depth, stepDepth, depthReasons, reasons: c.reasons, next, numbers: i.numbers, dispatches, skipped };
}
export function reviewPlanLine(p: ReviewPlan, pass: number): string {
return `REVIEW_PLAN: class=${p.class} pass=${pass} next=${p.next.step} dispatches=${p.dispatches.length} numbers=${p.numbers.publishes ? `yes(${p.numbers.claims})` : 'no'} fingerprint=${p.numbers.fingerprint}`;
}
export function reviewDepthLine(p: ReviewPlan): string {
return `REVIEW_DEPTH: ${p.depth} runtime_lines=${p.runtimeLines} rules=${p.rules} reasons=${p.depthReasons.join(',')} codes=${p.codes.join(',') || 'none'}${p.stepDepth !== p.depth ? ` step=${p.stepDepth}` : ''}`;
}
+1 -1
View File
@@ -139,7 +139,7 @@ Every other specialist, including Testing, reads the full diff with fixtures.
5. Instructions:
"You are a specialist code reviewer. Read the checklist at {checklist path}, then run
\`${diffBase(ctx)} && git diff "$DIFF_BASE"\` to get the ${ctx.skillName === 'ship' ? 'diff (`<diff-base>`: write the sha in before dispatch, the merge base on pass 1 and `RANGE_BASE` from pass 2 on; never a shell variable)' : 'full diff'}. Apply the checklist against the diff.
\`${diffBase(ctx)} && git diff "$DIFF_BASE"${ctx.skillName === 'ship' ? ' <exclude>' : ''}\` to get the ${ctx.skillName === 'ship' ? 'diff (`<diff-base>`: write the sha in before dispatch, the merge base on pass 1 and `RANGE_BASE` from pass 2 on; never a shell variable; `<exclude>`: everything after `<diff-base>` on this reviewer\'s `DIFF_CMD:` line, verbatim; empty on `DIFF_EXCLUDE: none`)' : 'full diff'}. Apply the checklist against the diff.
\`NO FINDINGS\` alone is the expected, complete result for a clean diff; never invent an item. Otherwise output one JSON object per finding per line, 400 words at most:
{\\"severity\\":\\"CRITICAL|INFORMATIONAL\\",\\"confidence\\":N,\\"path\\":\\"file\\",\\"line\\":N,\\"category\\":\\"category\\",\\"summary\\":\\"description\\",\\"fix\\":\\"recommended fix\\",\\"fingerprint\\":\\"path:line:category\\",\\"specialist\\":\\"name\\"}
+14 -9
View File
@@ -17,7 +17,7 @@ import * as fs from 'node:fs';
import * as os from 'node:os';
import * as path from 'node:path';
import { runScriptSync } from '../lib/script-invocation';
import { planReview, specialistsFor, type DiffClass, type Dispatch, type History, type NumbersResult } from '../lib/review-depth';
import { planReview, specialistsFor, type DiffClass, type History, type NumbersResult, type PathLines } from '../lib/review-depth';
const ROOT = path.resolve(import.meta.dir, '..');
const BYTES_PER_TOKEN = 4;
@@ -30,7 +30,8 @@ const DEFAULT_OUT: Record<string, { full: number; light: number }> = {
'outside-voice': { full: 6000, light: 6000 }, 'fix-check': { full: 4000, light: 2000 }, audit: { full: 5000, light: 2500 }, 'autoplan-voice': { full: 9000, light: 4500 },
};
export interface Fixture { name: string; commit: string; paths: string[]; deleted: string[]; stamp_only?: string[]; scope: Record<string, string>; gate_tagged: number | null; diff_lines: number; diff_bytes: number; numbers: NumbersResult }
/** per_path and registry_outputs (captured since GSTA-147) feed the runtime-line count; an older fixture counts every code-class line as runtime. */
export interface Fixture { name: string; commit: string; paths: string[]; deleted: string[]; stamp_only?: string[]; scope: Record<string, string>; gate_tagged: number | null; diff_lines: number; diff_bytes: number; numbers: NumbersResult; per_path?: PathLines[]; registry_outputs?: string[] }
/** clean: pass 1 finds nothing; nits: pass 1 fixes below-floor findings; blocker: pass 1 fixes a blocker; adversarial-fix: Step 11 finds a should that is fixed; fix-regression: a pass-1 fix introduces a blocker the next step finds. */
type Scenario = 'clean' | 'nits' | 'blocker' | 'adversarial-fix' | 'fix-regression';
const SCENARIOS: Scenario[] = ['clean', 'nits', 'blocker', 'adversarial-fix', 'fix-regression'];
@@ -62,7 +63,7 @@ function legacyShip(f: Fixture, sc: Scenario): Call[] {
}
function base(f: Fixture) {
return { paths: f.paths, deleted: f.deleted, stampOnly: f.stamp_only ?? [], scope: f.scope, gateTagged: f.gate_tagged, diffLines: f.diff_lines, numbers: f.numbers };
return { paths: f.paths, deleted: f.deleted, stampOnly: f.stamp_only ?? [], scope: f.scope, gateTagged: f.gate_tagged, diffLines: f.diff_lines, numbers: f.numbers, perPath: f.per_path, registry: f.registry_outputs ? { sha: null, outputs: f.registry_outputs } : undefined };
}
/** The new policy, step by step through lib/review-depth's own transition table. */
@@ -121,10 +122,10 @@ function capture(sha: string, name: string, out: string): number {
if (r.status !== 0) throw new Error(`gstack-review-plan failed: ${r.stderr}`);
const plan = JSON.parse(r.stdout);
const diffBytes = Buffer.byteLength(git(['diff', `${sha}^`, sha], dir).stdout ?? '');
const fixture: Fixture = { name, commit: sha, paths: plan.paths, deleted: plan.deleted, stamp_only: plan.stamp_only, scope: plan.scope, gate_tagged: plan.gate_tagged, diff_lines: plan.diff_lines, diff_bytes: diffBytes, numbers: plan.numbers };
const fixture: Fixture = { name, commit: sha, paths: plan.paths, deleted: plan.deleted, stamp_only: plan.stamp_only, scope: plan.scope, gate_tagged: plan.gate_tagged, diff_lines: plan.diff_lines, diff_bytes: diffBytes, numbers: plan.numbers, per_path: plan.per_path, registry_outputs: plan.registry.outputs };
fs.mkdirSync(path.dirname(out), { recursive: true });
fs.writeFileSync(out, JSON.stringify(fixture, null, 2) + '\n');
process.stdout.write(`CAPTURED: ${name} ${sha} class=${plan.class} paths=${plan.paths.length} lines=${plan.diff_lines} -> ${out}\n`);
process.stdout.write(`CAPTURED: ${name} ${sha} class=${plan.class} depth=${plan.depth} runtime=${plan.runtime_lines} paths=${plan.paths.length} lines=${plan.diff_lines} -> ${out}\n`);
return 0;
} finally {
git(['worktree', 'remove', '--force', dir]);
@@ -135,12 +136,13 @@ function capture(sha: string, name: string, out: string): number {
export function compare(fixtures: Fixture[], out: typeof DEFAULT_OUT) {
const rows: Array<Record<string, string | number>> = [];
for (const f of fixtures) {
const cls: DiffClass = planReview({ ...base(f), history: { fullPasses: 0, last: 'none' } }).class;
const first = planReview({ ...base(f), history: { fullPasses: 0, last: 'none' } });
const cls: DiffClass = first.class;
for (const sc of SCENARIOS) {
const legacy = legacyShip(f, sc);
const next = newShip(f, sc);
const lt = tokens(legacy, out); const nt = tokens(next.calls, out);
rows.push({ fixture: f.name, class: cls, scenario: sc, legacy_dispatches: legacy.length, new_dispatches: next.calls.length, legacy_tokens: lt, new_tokens: nt, ratio: Math.round((nt / lt) * 100) / 100, ending: next.ending });
rows.push({ fixture: f.name, class: cls, depth: first.depth, runtime_lines: first.runtimeLines, scenario: sc, legacy_dispatches: legacy.length, new_dispatches: next.calls.length, legacy_tokens: lt, new_tokens: nt, ratio: Math.round((nt / lt) * 100) / 100, ending: next.ending });
}
}
return rows;
@@ -158,8 +160,8 @@ function main(argv: string[]): number {
const rows = compare(fixtures, out);
const ap = fixtures.map(f => ({ fixture: f.name, runs: autoplanRuns(planReview({ ...base(f), history: { fullPasses: 0, last: 'none' } }).class === 'docs') }));
if (argv.includes('--json')) { process.stdout.write(JSON.stringify({ tokens_source: tokFile ?? 'DEFAULT', ship: rows, autoplan: ap }) + '\n'); return 0; }
const lines = [`TOKENS: ${tokFile ? `measured (${tokFile})` : 'DEFAULT estimates (pass --tokens for measured output)'}`, '', '| fixture | class | scenario | /ship dispatches legacy → new | est. tokens legacy → new | ratio | ending |', '|---|---|---|---|---|---|---|'];
for (const r of rows) lines.push(`| ${r.fixture} | ${r.class} | ${r.scenario} | ${r.legacy_dispatches} → ${r.new_dispatches} | ${r.legacy_tokens} → ${r.new_tokens} | ${r.ratio} | ${r.ending} |`);
const lines = [`TOKENS: ${tokFile ? `measured (${tokFile})` : 'DEFAULT estimates (pass --tokens for measured output)'}`, '', '| fixture | class | depth (runtime lines) | scenario | /ship dispatches legacy → new | est. tokens legacy → new | ratio | ending |', '|---|---|---|---|---|---|---|---|'];
for (const r of rows) lines.push(`| ${r.fixture} | ${r.class} | ${r.depth} (${r.runtime_lines}) | ${r.scenario} | ${r.legacy_dispatches} → ${r.new_dispatches} | ${r.legacy_tokens} → ${r.new_tokens} | ${r.ratio} | ${r.ending} |`);
lines.push('', '| fixture | /autoplan scenario | voice dispatches legacy → new |', '|---|---|---|');
for (const a of ap) for (const r of a.runs) lines.push(`| ${a.fixture} | ${r.scenario} | ${r.legacy} → ${r.next} |`);
const median = (v: number[]) => { const m = Math.floor(v.length / 2); return v.length % 2 ? v[m]! : Math.round(((v[m - 1]! + v[m]!) / 2) * 100) / 100; };
@@ -168,6 +170,9 @@ function main(argv: string[]): number {
const byClass = new Map<string, number[]>();
for (const r of rows) byClass.set(String(r.class), [...(byClass.get(String(r.class)) ?? []), Number(r.ratio)].sort((x, y) => x - y));
lines.push(`SHIP_TOKEN_RATIO_BY_CLASS: ${[...byClass].map(([c, v]) => `${c} median=${median(v)} (n=${v.length})`).join('; ')}`);
const byDepth = new Map<string, number[]>();
for (const r of rows) byDepth.set(String(r.depth), [...(byDepth.get(String(r.depth)) ?? []), Number(r.ratio)].sort((x, y) => x - y));
lines.push(`SHIP_TOKEN_RATIO_BY_DEPTH: ${[...byDepth].map(([d, v]) => `${d} median=${median(v)} (n=${v.length})`).join('; ')}`);
process.stdout.write(lines.join('\n') + '\n');
return 0;
}
+5 -5
View File
@@ -202,11 +202,11 @@ token; `--finish` stamps the binding fields automatically. Never borrow or repla
`gstack-wtree` prints a Git tree hash covering tracked and non-ignored untracked files,
not a commit ID. Use `git diff <old-tree> <new-tree>` to compare these snapshots.
**Review policy** (docs/review-policy.md): a finding blocks another review pass
only when it would fail a test, break production, or lose data or secrets;
everything below that floor ships as a named note in the PR body. Pass 1 reviews
the diff; a later pass reads only the range; two full passes plus one fix-check
is the cap. `NO FINDINGS` is a complete reviewer result. `ship: review_scope` (Step 3)
**Review policy** (docs/review-policy.md): only a finding that would fail a test,
break production, or lose data or secrets blocks a pass; the rest is a PR-body
note. Step 9's plan sets both reviews' depth (`--depth-floor standard|full` only
raises it). Pass 1 reads the diff, later passes the range; the cap is two full
passes and a fix-check. `NO FINDINGS` is complete. `ship: review_scope` (Step 3)
is the knob; the receipt's `review_loop` records the outcome.
### Ship control flow
+5 -5
View File
@@ -68,11 +68,11 @@ token; `--finish` stamps the binding fields automatically. Never borrow or repla
`gstack-wtree` prints a Git tree hash covering tracked and non-ignored untracked files,
not a commit ID. Use `git diff <old-tree> <new-tree>` to compare these snapshots.
**Review policy** (docs/review-policy.md): a finding blocks another review pass
only when it would fail a test, break production, or lose data or secrets;
everything below that floor ships as a named note in the PR body. Pass 1 reviews
the diff; a later pass reads only the range; two full passes plus one fix-check
is the cap. `NO FINDINGS` is a complete reviewer result. `ship: review_scope` (Step 3)
**Review policy** (docs/review-policy.md): only a finding that would fail a test,
break production, or lose data or secrets blocks a pass; the rest is a PR-body
note. Step 9's plan sets both reviews' depth (`--depth-floor standard|full` only
raises it). Pass 1 reads the diff, later passes the range; the cap is two full
passes and a fix-check. `NO FINDINGS` is complete. `ship: review_scope` (Step 3)
is the knob; the receipt's `review_loop` records the outcome.
### Ship control flow
+2 -1
View File
@@ -2,7 +2,8 @@
<!-- Regenerate: bun run gen:skill-docs -->
**Step 9's last full-pass plan governs this step:** run the native pass only on
`DISPATCH: adversarial-native` and the outside voice only on
`DISPATCH: outside-voice`. Without the native line, print `ADVERSARIAL: skipped (review plan)`;
`DISPATCH: outside-voice`. Without the native line (light depth at runtime 0;
above 0 the plan lists it beside the batched specialist), print `ADVERSARIAL: skipped (review plan)`;
with neither line, take decision 3 of Finish the adversarial phase.
## Step 11: Adversarial review
+2 -1
View File
@@ -1,6 +1,7 @@
**Step 9's last full-pass plan governs this step:** run the native pass only on
`DISPATCH: adversarial-native` and the outside voice only on
`DISPATCH: outside-voice`. Without the native line, print `ADVERSARIAL: skipped (review plan)`;
`DISPATCH: outside-voice`. Without the native line (light depth at runtime 0;
above 0 the plan lists it beside the batched specialist), print `ADVERSARIAL: skipped (review plan)`;
with neither line, take decision 3 of Finish the adversarial phase.
{{ADVERSARIAL_STEP}}
+16 -9
View File
@@ -14,13 +14,19 @@ Every repeat starts before the checklist read and captures a fresh REVIEW_START.
Finish the complete review and QA before applying any fix in Step 9.4.
**Review plan.** Pass 1 runs `~/.claude/skills/gstack/bin/gstack-review-plan
--base <base>`; a repeat uses the plan run that picked it. Save its `REVIEW_PLAN:` line and
fingerprint. Dispatch exactly its `DISPATCH:` lines (specialists, Red Team, Step
11's voices), each at its `effort=` on the same model family, in place of 9.1's and
9.2's own selection rules; a `SKIP:` reviewer
logs `{"dispatched":false,"reason":"scope"}`. Audits run at `AUDIT_EFFORT:`. The
core checklist, gate scan and QA always run. Item 6's row adds
`"dispatches":[{"role":R,"prompt_bytes":N,"diff_bytes":N}]`.
--base <base>`, plus `--depth-floor <d>` on every plan run when the /ship prompt
gave `--depth-floor <d>` (it only raises); a repeat uses the plan run that picked it.
Save its `REVIEW_PLAN:` line and fingerprint, `REVIEW_DEPTH:` (depth,
`runtime_lines`, `rules`) and `GATE_SCAN:`. Dispatch exactly its `DISPATCH:` lines
(specialists, Red Team, Step 11's voices), each at its `effort=` on the same model
family and reading its `DIFF_CMD:`, in place of 9.1's and 9.2's own selection rules;
`specialist batched` (light depth) is one subagent reading every checklist its
`checklists=` names, Red Team included, tagging each finding with its checklist.
A `SKIP:` reviewer logs `{"dispatched":false,"reason":"scope"}`. Audits run at
`AUDIT_EFFORT:`. The core checklist, gate scan and QA always run. Item 6's row adds
`"dispatches":[{"role":R,"prompt_bytes":N,"diff_bytes":N}]`, `"depth"`,
`"runtime_lines"`, `"depth_rules"` (the `rules` hash) and
`"gate_scan":{"tagged":N|null,"head":"<sha>"}` from those lines.
**Pass 2+ reads the range, not the tree** (`review_scope frozen`): the core read
in item 2 becomes `git diff $RANGE_BASE`, and every dispatched prompt (specialists,
@@ -391,7 +397,7 @@ Every other specialist, including Testing, reads the full diff with fixtures.
5. Instructions:
"You are a specialist code reviewer. Read the checklist at {checklist path}, then run
`DIFF_BASE=<diff-base> && git diff "$DIFF_BASE"` to get the diff (`<diff-base>`: write the sha in before dispatch, the merge base on pass 1 and `RANGE_BASE` from pass 2 on; never a shell variable). Apply the checklist against the diff.
`DIFF_BASE=<diff-base> && git diff "$DIFF_BASE" <exclude>` to get the diff (`<diff-base>`: write the sha in before dispatch, the merge base on pass 1 and `RANGE_BASE` from pass 2 on; never a shell variable; `<exclude>`: everything after `<diff-base>` on this reviewer's `DIFF_CMD:` line, verbatim; empty on `DIFF_EXCLUDE: none`). Apply the checklist against the diff.
`NO FINDINGS` alone is the expected, complete result for a clean diff; never invent an item. Otherwise output one JSON object per finding per line, 400 words at most:
{\"severity\":\"CRITICAL|INFORMATIONAL\",\"confidence\":N,\"path\":\"file\",\"line\":N,\"category\":\"category\",\"summary\":\"description\",\"fix\":\"recommended fix\",\"fingerprint\":\"path:line:category\",\"specialist\":\"name\"}
@@ -802,7 +808,8 @@ After persistence, record missing output, CYCLES and fixes in the invocation rec
Decision 1 always applies first. Then, under `frozen`, run `gstack-review-plan --base <base> --pass $PASS --last full`
with each flag that holds (`--fixed-blocking`, `--fixed-below`, `--blocking-open`,
`--touched-gate` for a fix to an item 2.5 path, `--critical-found` when this pass
logged a CRITICAL, `--adversarial-ran` once Step 11's native pass ran, `--reviewed-numbers <fp>`);
logged a CRITICAL, `--adversarial-ran` once Step 11's native pass ran, `--reviewed-numbers <fp>`,
`--depth-floor <d>` when given);
its `NEXT:` alone picks the next decision: `stop` 2, `full` 3, `fix-check` 3.5, `done` 4.
Other scopes apply decisions 2–4 in order.
+15 -8
View File
@@ -12,13 +12,19 @@ Every repeat starts before the checklist read and captures a fresh REVIEW_START.
Finish the complete review and QA before applying any fix in Step 9.4.
**Review plan.** Pass 1 runs `~/.claude/skills/gstack/bin/gstack-review-plan
--base <base>`; a repeat uses the plan run that picked it. Save its `REVIEW_PLAN:` line and
fingerprint. Dispatch exactly its `DISPATCH:` lines (specialists, Red Team, Step
11's voices), each at its `effort=` on the same model family, in place of 9.1's and
9.2's own selection rules; a `SKIP:` reviewer
logs `{"dispatched":false,"reason":"scope"}`. Audits run at `AUDIT_EFFORT:`. The
core checklist, gate scan and QA always run. Item 6's row adds
`"dispatches":[{"role":R,"prompt_bytes":N,"diff_bytes":N}]`.
--base <base>`, plus `--depth-floor <d>` on every plan run when the /ship prompt
gave `--depth-floor <d>` (it only raises); a repeat uses the plan run that picked it.
Save its `REVIEW_PLAN:` line and fingerprint, `REVIEW_DEPTH:` (depth,
`runtime_lines`, `rules`) and `GATE_SCAN:`. Dispatch exactly its `DISPATCH:` lines
(specialists, Red Team, Step 11's voices), each at its `effort=` on the same model
family and reading its `DIFF_CMD:`, in place of 9.1's and 9.2's own selection rules;
`specialist batched` (light depth) is one subagent reading every checklist its
`checklists=` names, Red Team included, tagging each finding with its checklist.
A `SKIP:` reviewer logs `{"dispatched":false,"reason":"scope"}`. Audits run at
`AUDIT_EFFORT:`. The core checklist, gate scan and QA always run. Item 6's row adds
`"dispatches":[{"role":R,"prompt_bytes":N,"diff_bytes":N}]`, `"depth"`,
`"runtime_lines"`, `"depth_rules"` (the `rules` hash) and
`"gate_scan":{"tagged":N|null,"head":"<sha>"}` from those lines.
**Pass 2+ reads the range, not the tree** (`review_scope frozen`): the core read
in item 2 becomes `git diff $RANGE_BASE`, and every dispatched prompt (specialists,
@@ -216,7 +222,8 @@ After persistence, record missing output, CYCLES and fixes in the invocation rec
Decision 1 always applies first. Then, under `frozen`, run `gstack-review-plan --base <base> --pass $PASS --last full`
with each flag that holds (`--fixed-blocking`, `--fixed-below`, `--blocking-open`,
`--touched-gate` for a fix to an item 2.5 path, `--critical-found` when this pass
logged a CRITICAL, `--adversarial-ran` once Step 11's native pass ran, `--reviewed-numbers <fp>`);
logged a CRITICAL, `--adversarial-ran` once Step 11's native pass ran, `--reviewed-numbers <fp>`,
`--depth-floor <d>` when given);
its `NEXT:` alone picks the next decision: `stop` 2, `full` 3, `fix-check` 3.5, `done` 4.
Other scopes apply decisions 2–4 in order.
+5 -5
View File
@@ -202,11 +202,11 @@ token; `--finish` stamps the binding fields automatically. Never borrow or repla
`gstack-wtree` prints a Git tree hash covering tracked and non-ignored untracked files,
not a commit ID. Use `git diff <old-tree> <new-tree>` to compare these snapshots.
**Review policy** (docs/review-policy.md): a finding blocks another review pass
only when it would fail a test, break production, or lose data or secrets;
everything below that floor ships as a named note in the PR body. Pass 1 reviews
the diff; a later pass reads only the range; two full passes plus one fix-check
is the cap. `NO FINDINGS` is a complete reviewer result. `ship: review_scope` (Step 3)
**Review policy** (docs/review-policy.md): only a finding that would fail a test,
break production, or lose data or secrets blocks a pass; the rest is a PR-body
note. Step 9's plan sets both reviews' depth (`--depth-floor standard|full` only
raises it). Pass 1 reads the diff, later passes the range; the cap is two full
passes and a fix-check. `NO FINDINGS` is complete. `ship: review_scope` (Step 3)
is the knob; the receipt's `review_loop` records the outcome.
### Ship control flow
+5 -5
View File
@@ -202,11 +202,11 @@ token; `--finish` stamps the binding fields automatically. Never borrow or repla
`gstack-wtree` prints a Git tree hash covering tracked and non-ignored untracked files,
not a commit ID. Use `git diff <old-tree> <new-tree>` to compare these snapshots.
**Review policy** (docs/review-policy.md): a finding blocks another review pass
only when it would fail a test, break production, or lose data or secrets;
everything below that floor ships as a named note in the PR body. Pass 1 reviews
the diff; a later pass reads only the range; two full passes plus one fix-check
is the cap. `NO FINDINGS` is a complete reviewer result. `ship: review_scope` (Step 3)
**Review policy** (docs/review-policy.md): only a finding that would fail a test,
break production, or lose data or secrets blocks a pass; the rest is a PR-body
note. Step 9's plan sets both reviews' depth (`--depth-floor standard|full` only
raises it). Pass 1 reads the diff, later passes the range; the cap is two full
passes and a fix-check. `NO FINDINGS` is complete. `ship: review_scope` (Step 3)
is the knob; the receipt's `review_loop` records the outcome.
### Ship control flow
+5 -5
View File
@@ -200,11 +200,11 @@ token; `--finish` stamps the binding fields automatically. Never borrow or repla
`gstack-wtree` prints a Git tree hash covering tracked and non-ignored untracked files,
not a commit ID. Use `git diff <old-tree> <new-tree>` to compare these snapshots.
**Review policy** (docs/review-policy.md): a finding blocks another review pass
only when it would fail a test, break production, or lose data or secrets;
everything below that floor ships as a named note in the PR body. Pass 1 reviews
the diff; a later pass reads only the range; two full passes plus one fix-check
is the cap. `NO FINDINGS` is a complete reviewer result. `ship: review_scope` (Step 3)
**Review policy** (docs/review-policy.md): only a finding that would fail a test,
break production, or lose data or secrets blocks a pass; the rest is a PR-body
note. Step 9's plan sets both reviews' depth (`--depth-floor standard|full` only
raises it). Pass 1 reads the diff, later passes the range; the cap is two full
passes and a fix-check. `NO FINDINGS` is complete. `ship: review_scope` (Step 3)
is the knob; the receipt's `review_loop` records the outcome.
### Ship control flow
+5 -5
View File
@@ -202,11 +202,11 @@ token; `--finish` stamps the binding fields automatically. Never borrow or repla
`gstack-wtree` prints a Git tree hash covering tracked and non-ignored untracked files,
not a commit ID. Use `git diff <old-tree> <new-tree>` to compare these snapshots.
**Review policy** (docs/review-policy.md): a finding blocks another review pass
only when it would fail a test, break production, or lose data or secrets;
everything below that floor ships as a named note in the PR body. Pass 1 reviews
the diff; a later pass reads only the range; two full passes plus one fix-check
is the cap. `NO FINDINGS` is a complete reviewer result. `ship: review_scope` (Step 3)
**Review policy** (docs/review-policy.md): only a finding that would fail a test,
break production, or lose data or secrets blocks a pass; the rest is a PR-body
note. Step 9's plan sets both reviews' depth (`--depth-floor standard|full` only
raises it). Pass 1 reads the diff, later passes the range; the cap is two full
passes and a fix-check. `NO FINDINGS` is complete. `ship: review_scope` (Step 3)
is the knob; the receipt's `review_loop` records the outcome.
### Ship control flow
+110
View File
@@ -0,0 +1,110 @@
import { afterEach, describe, expect, test } from 'bun:test';
import { spawnSync } from 'node:child_process';
import * as fs from 'node:fs';
import * as os from 'node:os';
import * as path from 'node:path';
import { collectCommit, collectLive, loadRegistryAt } from '../lib/review-collect';
import { depthOfCommitInput } from '../lib/review-depth';
const dirs: string[] = [];
afterEach(() => { while (dirs.length) fs.rmSync(dirs.pop()!, { recursive: true, force: true }); });
const lines = (n: number, tag = 'v') => Array.from({ length: n }, (_, i) => `export const ${tag}${i} = ${i};`).join('\n') + '\n';
const REGISTRY = JSON.stringify({ entries: [{ id: 'docs', command: 'bun run gen', outputs: ['*/SKILL.md'] }, { id: 'manual', command: null, outputs: ['src/hand.ts'] }] });
function repo(baseFiles: Record<string, string | Buffer>) {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'review-collect-'));
dirs.push(dir);
const git = (...a: string[]) => { const r = spawnSync('git', a, { cwd: dir, encoding: 'utf8', timeout: 10_000 }); if (r.status !== 0) throw new Error(`git ${a.join(' ')}: ${r.stderr}`); return r.stdout.trim(); };
const write = (files: Record<string, string | Buffer | null>) => { for (const [p, text] of Object.entries(files)) { const f = path.join(dir, p); if (text === null) { fs.rmSync(f); continue; } fs.mkdirSync(path.dirname(f), { recursive: true }); fs.writeFileSync(f, text); } };
git('init', '-q', '-b', 'main'); git('config', 'user.email', 't@t'); git('config', 'user.name', 't'); git('config', 'commit.gpgsign', 'false');
write({ 'README.md': '# x\n', ...baseFiles }); git('add', '-A'); git('commit', '-qm', 'base');
const baseSha = git('rev-parse', 'HEAD');
git('checkout', '-qb', 'feat');
const commit = (files: Record<string, string | Buffer | null>) => { write(files); git('add', '-A'); git('commit', '-qm', 'feat'); return git('rev-parse', 'HEAD'); };
return { dir, git, write, commit, baseSha };
}
describe('collectCommit: git objects only', () => {
test('a registered glob output is excluded, a command:null entry is not, and a .tmpl edit counts once', () => {
const r = repo({ '.gstack/generated.json': REGISTRY, 'ship/SKILL.md.tmpl': 'a\n', 'ship/SKILL.md': 'a\n' });
const head = r.commit({ 'ship/SKILL.md.tmpl': 'a\nb\nc\n', 'ship/SKILL.md': lines(120), 'src/hand.ts': lines(5) });
const c = collectCommit(r.dir, r.baseSha, head);
expect(c.errors).toEqual([]);
expect(c.registry).toEqual({ sha: r.baseSha, outputs: ['*/SKILL.md'] });
const d = depthOfCommitInput({ ...c, gateTagged: 0 });
expect([d.depth, d.runtimeLines]).toEqual(['light', 2 + 5]);
});
test('a head-only registry entry never excludes: the registry is the base commit\'s', () => {
const r = repo({ 'src/keep.ts': lines(1) });
const head = r.commit({ '.gstack/generated.json': JSON.stringify({ entries: [{ id: 'gen', command: 'gen', outputs: ['src/gen.ts'] }] }), 'src/gen.ts': lines(60) });
const c = collectCommit(r.dir, r.baseSha, head);
expect(c.registry).toEqual({ sha: r.baseSha, outputs: [] });
const d = depthOfCommitInput({ ...c, gateTagged: 0 });
expect(d.runtimeLines).toBeGreaterThanOrEqual(60);
expect(d.depth).toBe('full');
});
test('a rename into src/ counts the new path as runtime', () => {
const r = repo({ 'notes/a.txt': lines(30) });
fs.mkdirSync(path.join(r.dir, 'src'));
r.git('mv', 'notes/a.txt', 'src/a.ts');
r.git('commit', '-qm', 'mv');
const c = collectCommit(r.dir, r.baseSha, r.git('rev-parse', 'HEAD'));
expect(c.deleted).toEqual(['notes/a.txt']);
expect(c.perPath.find(e => e.path === 'src/a.ts')).toEqual({ path: 'src/a.ts', lines: 30, binary: false });
expect(depthOfCommitInput({ ...c, gateTagged: 0 })).toMatchObject({ depth: 'light', runtimeLines: 30 });
});
test('a binary code-class path is an error (full), never 0 lines; a deleted test is full', () => {
const r = repo({ 'test/old.test.ts': 'x\n' });
const head = r.commit({ 'lib/x.wasm': Buffer.from([0, 97, 115, 109, 0, 1, 2]) });
const d = depthOfCommitInput({ ...collectCommit(r.dir, r.baseSha, head), gateTagged: 0 });
expect(d.depth).toBe('full');
expect(d.errors).toEqual([expect.objectContaining({ code: 'binary_code_path', input: 'lib/x.wasm' })]);
const t = repo({ 'test/old.test.ts': 'x\n', 'test/keep.test.ts': 'x\n' });
const del = t.commit({ 'test/old.test.ts': null });
const deleted = depthOfCommitInput({ ...collectCommit(t.dir, t.baseSha, del), gateTagged: 0 });
expect([deleted.depth, deleted.runtimeLines, deleted.errors]).toEqual(['full', 0, []]);
});
test('an invalid base registry is an error with its result code; an unresolvable sha is unavailable', () => {
const r = repo({ '.gstack/generated.json': '{"entries": [{"id": "x"}]}' });
const head = r.commit({ 'src/a.ts': lines(3) });
const d = depthOfCommitInput({ ...collectCommit(r.dir, r.baseSha, head), gateTagged: 0 });
expect(d.depth).toBe('full');
expect(d.codes).toEqual(['REGEN_REGISTRY_INVALID']);
expect(d.errors[0]).toMatchObject({ code: 'registry_invalid', result: 'REGEN_REGISTRY_INVALID' });
expect(d.errors[0]!.repair).toContain('gstack-regen validate');
expect(loadRegistryAt(r.dir, 'f'.repeat(40)).error?.code).toBe('registry_unavailable');
const missing = collectCommit(r.dir, 'f'.repeat(40), head);
expect(missing.errors.map(e => e.code)).toContain('numstat_failed');
expect(depthOfCommitInput({ ...missing, gateTagged: 0 }).depth).toBe('full');
});
test('from a checkout at another HEAD with a dirty tree it equals the clean result at head', () => {
const r = repo({ '.gstack/generated.json': REGISTRY, 'src/a.ts': lines(2), 'ship/SKILL.md': 'a\n', 'autoplan/SKILL.md.tmpl': 'x\n' });
const head = r.commit({ 'src/a.ts': lines(40), 'ship/SKILL.md': lines(9), 'docs/b.md': '# b\n12% faster\n', 'autoplan/notes.md': 'n\n', 'package.json': '{"version":"1.0.1"}\n' });
const clean = collectCommit(r.dir, r.baseSha, head);
r.git('checkout', '-q', r.baseSha);
r.write({ 'src/a.ts': lines(500, 'dirty'), 'src/untracked.ts': lines(80), 'autoplan/SKILL.md.tmpl': null });
const dirty = collectCommit(r.dir, r.baseSha, head);
expect(dirty).toEqual(clean);
expect(depthOfCommitInput({ ...dirty, gateTagged: 0 })).toEqual(depthOfCommitInput({ ...clean, gateTagged: 0 }));
expect(clean.root).toEqual(['autoplan', 'ship']);
expect(clean.added['docs/b.md']).toEqual(['# b', '12% faster']);
});
});
describe('collectLive: the working tree', () => {
test('tracked uncommitted edits and untracked files count per path; scratch under .gstack/tmp/ does not', () => {
const r = repo({ 'src/a.ts': lines(1) });
r.commit({ 'docs/a.md': '# a\n' });
r.write({ 'src/a.ts': lines(11), 'src/new.ts': lines(20), '.gstack/tmp/x.ts': lines(500) });
const c = collectLive(r.dir, 'main');
expect(c.errors).toEqual([]);
expect(c.untracked).toEqual(['src/new.ts']);
expect(c.perPath.find(e => e.path === 'src/new.ts')).toEqual({ path: 'src/new.ts', lines: 20, binary: false });
expect(c.paths).toEqual(['docs/a.md', 'src/a.ts', 'src/new.ts']);
expect(depthOfCommitInput({ ...c, gateTagged: 0 })).toMatchObject({ depth: 'light', runtimeLines: 20 + 10 });
});
test('no merge base is a collector error the CLI refuses on', () => {
const r = repo({});
expect(collectLive(r.dir, 'no-such-branch').errors.map(e => e.code)).toEqual(['numstat_failed']);
});
});
+126 -4
View File
@@ -1,6 +1,9 @@
import { describe, test, expect } from 'bun:test';
import { spawnSync } from 'node:child_process';
import * as fs from 'node:fs';
import * as os from 'node:os';
import * as path from 'node:path';
import { classifyDiff, classifyPath, nextStep, planReview, scanNumbers, FULL_PASS_CAP, type PlanInput } from '../lib/review-depth';
import { classifyDiff, classifyPath, depthFor, depthOfCommitInput, diffCommand, nextStep, planReview, scanNumbers, DEPTH_RULES, FULL_PASS_CAP, type DepthError, type PlanInput } from '../lib/review-depth';
const ROOT = path.join(import.meta.dir, '..');
const none = scanNumbers({});
@@ -73,9 +76,10 @@ describe('planReview', () => {
const p = planReview(base({ paths: ['docs/bench.md'], scope: {}, numbers: scanNumbers({ 'docs/bench.md': ['3x faster'] }) }));
expect(roles(p)).toEqual(['outside-voice:outside-voice:host']);
});
test('tests class: testing specialist only', () => {
test('tests class: runtime 0 is light, core only (Behavior changes: no Testing specialist)', () => {
const p = planReview(base({ paths: ['test/a.test.ts'], scope: { SCOPE_TESTS: 'true' } }));
expect(roles(p)).toEqual(['specialist:testing:full']);
expect(p.depth).toBe('light');
expect(roles(p)).toEqual([]);
});
test('code class pass 1: every reviewer at full effort, the outside voice at its own', () => {
const p = planReview(base({}));
@@ -88,7 +92,7 @@ describe('planReview', () => {
});
test('code class honours hit-rate gating and small-diff skip', () => {
expect(roles(planReview(base({ gated: ['simplification'] })))).not.toContain('specialist:simplification:full');
expect(planReview(base({ diffLines: 20 })).dispatches.filter(d => d.role === 'specialist')).toEqual([]);
expect(planReview(base({ diffLines: 20 })).dispatches.filter(d => d.role === 'specialist').map(d => d.name)).toEqual(['batched']);
});
test('pass 2 reruns the army on the range but not the adversarial or outside voice once Step 11 ran', () => {
const p = planReview(base({ adversarialRan: true, history: { fullPasses: 1, last: 'full', fixedBlocking: true } }));
@@ -167,3 +171,121 @@ describe('GSTA-131 coverage: Red Team and specialist arms', () => {
expect(roles(planReview(base({ paths: ['docs/a.md'], force: ['security'] })))).toEqual(['specialist:security']);
});
});
const per = (o: Record<string, number>) => Object.entries(o).map(([p, lines]) => ({ path: p, lines, binary: false }));
const mixed = (o: Partial<PlanInput> = {}) => base({ paths: Object.keys(o.perPath ? Object.fromEntries(o.perPath.map(e => [e.path, 1])) : {}), ...o });
const err = (code: DepthError['code'], result?: DepthError['result']): DepthError => ({ code, detail: 'd', input: 'i', repair: 'r', ...(result ? { result } : {}) });
describe('review depth: the matrix', () => {
test('runtime 0 (tests and docs only) is light: core only, nothing dispatched', () => {
const p = planReview(mixed({ perPath: per({ 'test/a.test.ts': 300, 'docs/a.md': 40 }), diffLines: 340 }));
expect([p.depth, p.runtimeLines, p.dispatches]).toEqual(['light', 0, []]);
});
test('runtime 1-50 is light plus one batched specialist: the fixed set, scope picks and the Red Team in one context', () => {
const p = planReview(mixed({ perPath: per({ 'src/app.ts': 48, 'test/a.test.ts': 190, 'docs/a.md': 9 }), diffLines: 247 }));
expect([p.depth, p.runtimeLines]).toEqual(['light', 48]);
expect(p.dispatches).toEqual([
{ role: 'specialist', name: 'batched', effort: 'full', exclude: [':(exclude)docs/a.md'], checklists: ['testing', 'maintainability', 'security', 'performance', 'red-team'] },
{ role: 'adversarial-native', name: 'adversarial', effort: 'full', exclude: [], reason: 'backtest inconclusive (GSTA-147 D5)' },
]);
expect(p.skipped.join('; ')).toContain('red-team (light depth');
expect(planReview(mixed({ perPath: per({ 'src/app.ts': 48 }), adversarialRan: true, history: { fullPasses: 1, last: 'full', touchedGate: true, fixedBelow: true } })).dispatches.some(d => d.role === 'adversarial-native')).toBe(false);
});
test('runtime 51 is standard: thresholds read runtime lines, reviewers read their own hunks', () => {
const p = planReview(mixed({ perPath: per({ 'src/app.ts': 51, 'test/a.test.ts': 400, 'README.md': 3 }), diffLines: 454 }));
expect([p.depth, p.runtimeLines]).toEqual(['standard', 51]);
expect(roles(p)).toEqual(['specialist:testing:full', 'specialist:maintainability:full', 'specialist:performance:full', 'adversarial-native:adversarial:full', 'outside-voice:outside-voice:host']);
const ex = Object.fromEntries(p.dispatches.map(d => [d.name, d.exclude]));
expect(ex.testing).toEqual([':(exclude)README.md']);
expect(ex.maintainability).toEqual([':(exclude)README.md', ':(exclude)test/a.test.ts']);
expect([ex.adversarial, ex['outside-voice']]).toEqual([[], []]);
});
test.each([
['scope error', { scope: { SCOPE_ERROR: 'boom' } }, 'scope_error', []],
['gate scan unavailable', { gateTagged: null }, 'gate_scan_unavailable', []],
['base registry invalid', { registry: { sha: 'abc', outputs: [], error: err('registry_invalid', 'REGEN_REGISTRY_INVALID') } }, 'registry_invalid', ['REGEN_REGISTRY_INVALID']],
['base registry unavailable', { registry: { sha: null, outputs: [], error: err('registry_unavailable') } }, 'registry_unavailable', []],
['numstat failure', { errors: [err('numstat_failed')] }, 'numstat_failed', []],
['binary code path', { perPath: [...per({ 'src/app.ts': 5 }), { path: 'lib/x.wasm', lines: 0, binary: true }], paths: ['src/app.ts', 'lib/x.wasm'] }, 'binary_code_path', []],
] as const)('%s fails closed to full with its code', (_n, o, code, codes) => {
const p = planReview(mixed({ perPath: per({ 'src/app.ts': 5 }), ...(o as Partial<PlanInput>) }));
expect(p.depth).toBe('full');
expect(p.errors.map(e => e.code)).toContain(code);
expect(p.codes).toEqual([...codes]);
expect(roles(p)).toContain('red-team:red-team:full');
});
test.each([
['a gate path', { paths: ['src/app.ts', '.github/workflows/ci.yml'] }],
['a gate tag', { gateTagged: 1 }],
['a deleted test', { deleted: ['test/old.test.ts'] }],
['an edit to the registry', { paths: ['.gstack/generated.json'] }],
['an edit to SMALL_DIFF_LINES', { paths: ['lib/review-depth.ts'] }],
['the collector', { paths: ['lib/review-collect.ts'] }],
['the plan CLI', { paths: ['bin/gstack-review-plan'] }],
['the queue', { paths: ['lib/ship-queue-train.ts'] }],
['the Step 9 template', { paths: ['ship/sections/review-army.md.tmpl'] }],
] as const)('%s is full', (_n, o) => {
const paths = (o as any).paths ?? ['src/app.ts'];
expect(planReview(mixed({ perPath: per(Object.fromEntries(paths.map((p: string) => [p, 2]))), ...(o as Partial<PlanInput>) })).depth).toBe('full');
});
test('a registered glob output is excluded from runtime; a .tmpl edit plus its generated SKILL.md counts the tmpl only', () => {
const p = planReview(mixed({ perPath: per({ 'ship/SKILL.md.tmpl': 30, 'ship/SKILL.md': 200 }), registry: { sha: 'abc', outputs: ['*/SKILL.md'] } }));
expect([p.depth, p.runtimeLines]).toEqual(['light', 30]);
expect(p.dispatches[0]!.exclude).toEqual([':(exclude)ship/SKILL.md']);
});
test('a path no base-registry entry names stays runtime', () => {
expect(planReview(mixed({ perPath: per({ 'src/gen.ts': 60 }), registry: { sha: 'abc', outputs: ['lib/dom.js'] } })).runtimeLines).toBe(60);
});
test('numbers in light dispatch the outside voice with an empty exclude', () => {
const p = planReview(mixed({ perPath: per({ 'docs/bench.md': 4 }), numbers: scanNumbers({ 'docs/bench.md': ['3x faster'] }) }));
expect(p.depth).toBe('light');
expect(p.dispatches).toEqual([{ role: 'outside-voice', name: 'outside-voice', effort: 'host', exclude: [] }]);
});
test('a fixed blocker in light raises the next full step to the standard set, Red Team included', () => {
const p = planReview(mixed({ perPath: per({ 'src/app.ts': 30 }), history: { fullPasses: 1, last: 'full', fixedBlocking: true } }));
expect([p.depth, p.stepDepth]).toEqual(['light', 'standard']);
expect(roles(p)).toEqual(['specialist:testing:full', 'specialist:maintainability:full', 'specialist:performance:full', 'red-team:red-team:full', 'adversarial-native:adversarial:full', 'outside-voice:outside-voice:host']);
});
test('--depth-floor raises and never lowers', () => {
expect(depthFor({ cls: 'code', runtimeLines: 10, errors: [], floor: 'full' })).toBe('full');
expect(depthFor({ cls: 'code', runtimeLines: 10, errors: [], floor: 'standard' })).toBe('standard');
expect(depthFor({ cls: 'gate', runtimeLines: 10, errors: [], floor: 'standard' })).toBe('full');
expect(depthFor({ cls: 'code', runtimeLines: 80, errors: [] })).toBe('standard');
const p = planReview(mixed({ perPath: per({ 'src/app.ts': 10 }), floor: 'full' }));
expect(p.depth).toBe('full');
expect(p.depthReasons).toContain('floor:full');
});
test('depthOfCommitInput gives the same depth as the plan for the same input', () => {
const i = { paths: ['src/app.ts', 'test/a.test.ts'], perPath: per({ 'src/app.ts': 51, 'test/a.test.ts': 9 }), gateTagged: 0 };
expect(depthOfCommitInput(i)).toMatchObject({ depth: 'standard', runtimeLines: 51, rules: DEPTH_RULES, errors: [] });
expect(depthOfCommitInput({ ...i, gateTagged: null }).depth).toBe('full');
});
});
describe('review depth: diff commands and the rules hash', () => {
test('DIFF_CMD shell-quotes each pathspec; a glob character makes the pathspec literal', () => {
const p = planReview(mixed({ perPath: per({ 'src/app.ts': 10, "docs/my notes*.md": 3, "docs/it's.md": 1 }) }));
const ex = p.dispatches[0]!.exclude;
expect(ex).toEqual([':(exclude,literal)docs/my notes*.md', ":(exclude)docs/it's.md"]);
expect(diffCommand(p.dispatches[0]!)).toBe("git diff <diff-base> -- . ':(exclude,literal)docs/my notes*.md' ':(exclude)docs/it'\\''s.md'");
expect(diffCommand({ role: 'red-team', name: 'red-team', effort: 'full', exclude: [] })).toBe('git diff <diff-base>');
});
test('DEPTH_RULES hashes the deciding constants: a comment-only edit keeps it, a threshold edit changes it', () => {
expect(DEPTH_RULES).toMatch(/^[0-9a-f]{16}$/);
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'depth-rules-'));
try {
const src = fs.readFileSync(path.join(ROOT, 'lib/review-depth.ts'), 'utf8');
fs.symlinkSync(path.join(ROOT, 'lib/ship-policy.ts'), path.join(dir, 'ship-policy.ts'));
const hashOf = (text: string) => {
fs.writeFileSync(path.join(dir, 'review-depth.ts'), text);
fs.writeFileSync(path.join(dir, 'print.ts'), "import { DEPTH_RULES } from './review-depth';\nconsole.log(DEPTH_RULES);\n");
const r = spawnSync(process.execPath, ['--no-env-file', '--config=/dev/null', path.join(dir, 'print.ts')], { encoding: 'utf8', timeout: 30_000 });
expect(r.status, r.stderr).toBe(0);
return r.stdout.trim();
};
expect(hashOf('// a comment-only edit\n' + src)).toBe(DEPTH_RULES);
expect(hashOf(src.replace('SMALL_DIFF_LINES = 50', 'SMALL_DIFF_LINES = 60'))).not.toBe(DEPTH_RULES);
} finally { fs.rmSync(dir, { recursive: true, force: true }); }
});
});
+1 -1
View File
@@ -34,7 +34,7 @@ describe('scripts/review-dispatch-compare.ts', () => {
const table = run(['--tokens', tok]);
expect(table).toContain(`TOKENS: measured (${tok})`);
expect(table).toMatch(/^SHIP_TOKEN_RATIO: median=0\.\d+ min=0\.\d+ max=\d\.\d+ \(n=10\)$/m);
expect(table).toMatch(/^SHIP_TOKEN_RATIO_BY_CLASS: docs median=0\.\d+ \(n=5\); code median=0\.\d+ \(n=5\)$/m);
expect(table).toMatch(/^SHIP_TOKEN_RATIO_BY_CLASS: docs median=0\.\d+ \(n=5\); gate median=0\.\d+ \(n=5\)$/m);
} finally {
fs.rmSync(dir, { recursive: true, force: true });
}
+48
View File
@@ -109,3 +109,51 @@ describe('gstack-review-plan', () => {
expect(ran).toContain('SKIP: adversarial-native and outside-voice (run once per invocation)');
});
});
describe('gstack-review-plan: review depth', () => {
const code = (n: number) => Array.from({ length: n }, (_, i) => `export const v${i} = ${i};`).join('\n') + '\n';
test('prints REVIEW_DEPTH with runtime lines and the rules hash, and GATE_SCAN bound to the head', () => {
const r = repo({ 'src/a.ts': code(10), 'test/a.test.ts': code(200) });
const out = run(r, []).stdout;
const head = spawnSync('git', ['rev-parse', 'HEAD'], { cwd: r.dir, encoding: 'utf8', timeout: 10_000 }).stdout.trim();
expect(out).toMatch(/^REVIEW_DEPTH: light runtime_lines=10 rules=[0-9a-f]{16} reasons=runtime=10<=50 codes=none$/m);
expect(out).toContain(`GATE_SCAN: tagged=0 head=${head}`);
expect(out).toContain('DISPATCH: specialist batched effort=full checklists=testing,maintainability,security,performance,state-machines,red-team');
expect(out).toContain('DISPATCH: adversarial-native adversarial effort=full reason=backtest inconclusive (GSTA-147 D5)');
});
test('DIFF_CMD is shell-quoted per dispatch and excludes what the reviewer does not read; DIFF_EXCLUDE: none when empty', () => {
const r = repo({ 'src/a.ts': code(80), 'docs/my notes*.md': '# n\n', 'test/a.test.ts': code(3) });
const out = run(r, []).stdout;
expect(out).toContain("DIFF_CMD: testing git diff <diff-base> -- . ':(exclude,literal)docs/my notes*.md'");
expect(out).toContain("DIFF_CMD: maintainability git diff <diff-base> -- . ':(exclude,literal)docs/my notes*.md' ':(exclude)test/a.test.ts'");
expect(out).toContain('DIFF_CMD: adversarial git diff <diff-base>\nDIFF_EXCLUDE: adversarial none');
const cmd = /^DIFF_CMD: maintainability (.*)$/m.exec(out)![1]!.replace('<diff-base>', 'main');
const diff = spawnSync('bash', ['-c', cmd.replace('git diff main', 'git diff --name-only main')], { cwd: r.dir, encoding: 'utf8' });
expect(diff.stdout.trim().split('\n')).toEqual(['src/a.ts']);
});
test('--json carries depth, runtime_lines, rules, errors, gate_scan and every dispatch\'s exclude array', () => {
const r = repo({ 'src/a.ts': code(80), 'README.md': '# y\n' });
const plan = JSON.parse(run(r, ['--json']).stdout);
expect(plan).toMatchObject({ depth: 'standard', step_depth: 'standard', runtime_lines: 80, errors: [], codes: [], gate_scan: { tagged: 0 } });
expect(plan.rules).toMatch(/^[0-9a-f]{16}$/);
expect(plan.dispatches.find((d: any) => d.name === 'maintainability').exclude).toEqual([':(exclude)README.md']);
expect(plan.dispatches.find((d: any) => d.role === 'outside-voice').exclude).toEqual([]);
});
test('--depth-floor full raises a light diff, never lowers a full one, and an invalid floor is a usage error', () => {
const r = repo({ 'src/a.ts': code(5) });
expect(run(r, ['--depth-floor', 'full']).stdout).toMatch(/^REVIEW_DEPTH: full runtime_lines=5 .*reasons=runtime=5<=50,floor:full /m);
const gate = repo({ 'src/a.ts': code(5), '.github/workflows/ci.yml': 'on: push\n' });
expect(run(gate, ['--depth-floor', 'standard']).stdout).toMatch(/^REVIEW_DEPTH: full /m);
expect(run(r, ['--depth-floor', 'light']).status).toBe(2);
});
test('a conservative full plan exits 0 with its errors; a plan that cannot be built exits 1', () => {
const r = repo({ 'lib/x.wasm': '\u0000asm\u0000' });
const res = run(r, ['--json']);
expect(res.status).toBe(0);
expect(JSON.parse(res.stdout)).toMatchObject({ depth: 'full', errors: expect.arrayContaining([expect.objectContaining({ code: 'binary_code_path', input: 'lib/x.wasm' })]) });
expect(run(r, []).stdout).toContain('DEPTH_ERROR: binary_code_path input=lib/x.wasm');
const none = spawnSync(BIN, ['--base', 'no-such-branch', '--gate-tagged', '0'], { cwd: r.dir, encoding: 'utf8', timeout: 60_000, env: { ...process.env, GSTACK_STATE_ROOT: r.state } });
expect(none.status).toBe(1);
});
});
@@ -0,0 +1,4 @@
{
"bytes": 745,
"why": "GSTA-147 review depth: Step 9 saves REVIEW_DEPTH/GATE_SCAN, passes --depth-floor, records depth fields and reads each reviewer's DIFF_CMD; the batched light specialist; Step 11's light note; the skeleton's Review policy paragraph names the depth and --depth-floor (trimmed to hold the skeleton cap)"
}