mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 16:34:00 +01:00
Merge remote-tracking branch 'origin/gsta-167-review-packets' into gsta-167-batched-review
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> # Conflicts: # bin/gstack-review-packet # lib/review-packet-collect.ts # lib/review-packet.ts # scripts/resolvers/review-army.ts # ship/sections/review-army.md # ship/sections/test-coverage.md # ship/sections/test-coverage.md.tmpl # test/review-packet.test.ts # test/size-ledger/ship/gsta167-review-packets.json
This commit is contained in:
commit
cc28dd35c5
21 files changed
+512
-75
No files matched your search
@@ -1,5 +1,27 @@
|
||||
# Changelog
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
**/ship reviewers now read one packet file each, and the parent reads a findings record of at most 60 lines instead of every reviewer's transcript.** On this PR's own /ship, pass 2's eight reviewers cost 8.6M tokens on the root counter, against 52.6M for pass 1's eleven (pass 2 read the fix range, pass 1 the whole diff, so the two are not directly comparable).
|
||||
|
||||
### Added
|
||||
- **`gstack-review-packet build`** writes one packet per `DISPATCH:` line of the review plan: the role's instructions and findings contract, its checklists, that reviewer's diff, a plan excerpt, a read budget and the output path. Steps 7 and 8 audits, the specialists, the Red Team, the fix-check and Step 11's adversarial pass and outside voice all dispatch this way. A packet over 200 KB is split by path; no hunk is cut and no path is dropped. Audits are never split.
|
||||
- Each `PACKET:` line names its `budget_role` and `read=` line ranges of at most 30,000 bytes, and build prints `BUDGET_NEXT: dispatch:<roles>` for `gstack-ship-budget check --next`. A reviewer reads its packet with parallel range reads and finishes in about three calls.
|
||||
- **`gstack-review-packet collect`** parses, validates and dedupes every reviewer's output, prints one line per reviewer plus each CRITICAL, and writes `merged.jsonl` and `informational.md` for the PR body. Missing or unusable output exits 3 with `REVIEW_COVERAGE_INCOMPLETE`.
|
||||
|
||||
### Changed
|
||||
- Every reviewer prompt lives in `lib/review-prompts.ts`; the resolvers import it, so a packet and the generated SKILL.md prose cannot drift. /review's generated output is unchanged.
|
||||
- Large `.json`, `.jsonl`, `.ndjson`, `.csv` and `.tsv` changes (over 400 changed lines, not package or config manifests) reach reviewers as a summary built from the diff alone. The testing reviewer and the coverage audit still read data under `test/` or `fixtures/` in full.
|
||||
- The Capy dispatch paragraph in `capy/skill/gstack/SKILL.md` builds packets, starts reviewers on the shared machine, polls with `transcript: "none"` and returns only collect's output.
|
||||
|
||||
### Fixed
|
||||
- `--base origin/main` no longer becomes `origin/origin/main` in audit prompts.
|
||||
- A partial build keeps other reviewers' results only at the same head, diff base and diff content; a Red Team build against a changed tree refuses with `STALE_MANIFEST`.
|
||||
- Collect counts a state-machines matrix record only from the state-machines reviewer, and a CRITICAL line that fails to parse or validate makes its reviewer invalid instead of disappearing.
|
||||
|
||||
### Tests
|
||||
- `test/review-packet.test.ts`: 41 tests covering packet contents, sharding, audits, budget roles, read ranges, data summaries (including a symlinked CSV that must not leak its target), stale manifests and collect's validation. Each fix in this entry has a test that failed before it.
|
||||
|
||||
## [1.91.324.0] - 2026-10-29
|
||||
|
||||
**A /ship token cap now projects each step and reviewer from what the run has actually spent, so real caps stop refusing work that costs a fraction of the old guess.** On GSTA-152 a 29.28M cap refused its third reviewer because every dispatch was projected at 7.43M while real ones cost 0.23 to 1.02M; on GSTA-2 an 8M cap stopped before Step 6 after Steps 1 to 5 cost about 0.27M each, because every step was projected at 2.25M.
|
||||
|
||||
+18
-10
@@ -20,7 +20,8 @@ import { EXIT, exitTableHelp } from '../lib/headless-artifacts';
|
||||
import { runScriptSync } from '../lib/script-invocation';
|
||||
import { reviewPlanLine } from '../lib/review-depth';
|
||||
import { flagReader, livePlan, parsePlanFlags, planPass, PLAN_FLAGS_HELP } from '../lib/review-plan-live';
|
||||
import { AUDITS, auditCodePaths, auditDraft, draftFor, packetLine, packSpecialists, PACKET_CAP_BYTES, PACKINGS, readManifest, SPECIALIST_PACKING, writeManifest, writePackets, type AuditName, type BuildContext, type PacketEntry, type Packing } from '../lib/review-packet';
|
||||
import { AUDITS, auditCodePaths, auditDraft, budgetRole, contentKey, draftFor, packetLine, packSpecialists, PACKET_CAP_BYTES, PACKINGS, READ_CHUNK_BYTES, readManifest, SPECIALIST_PACKING, writeManifest, writePackets, type AuditName, type BuildContext, type PacketEntry, type Packing } from '../lib/review-packet';
|
||||
import { resolveBaseRef } from '../lib/review-collect';
|
||||
import { collect } from '../lib/review-packet-collect';
|
||||
|
||||
const BIN = 'gstack-review-packet';
|
||||
@@ -28,7 +29,8 @@ const HELP = `${BIN} — one packet file per /ship reviewer; collect reads their
|
||||
|
||||
build --base <ref> --out-dir <dir> write one packet per DISPATCH line of the plan gstack-review-plan prints
|
||||
--fanout batched|parallel batched (default ${SPECIALIST_PACKING}): a pass's specialist lines are one packet
|
||||
per shard inlining every checklist; parallel: one packet per specialist
|
||||
per shard inlining every checklist (a specialist --only names keeps its own);
|
||||
parallel: one packet per specialist
|
||||
--range <sha> the diff base from pass 2 on (RANGE_BASE; fix-check: PASS_HEAD); default the merge base
|
||||
--plan-file <path> add at most 8 KB of the plan to each packet (the plan-completion audit gets it whole)
|
||||
--only a,b only these reviewers (names or roles: adversarial, outside-voice, red-team, ...)
|
||||
@@ -42,11 +44,12 @@ ${PLAN_FLAGS_HELP}
|
||||
collect --dir <dir> read every packet's output: one REVIEWER: line each, every CRITICAL on one
|
||||
line, INFORMATIONAL counts; writes merged.jsonl and informational.md in <dir>
|
||||
|
||||
build prints PACKET: <role> <name> effort=<e> bytes=<n> tokens~=<n/4> file=<path> out=<path> per packet
|
||||
(a reviewer past ${PACKET_CAP_BYTES} bytes is sharded by path as <name>.1ofK, never truncated),
|
||||
PACKED: specialist batched checklists=<a,b> shards=<k> per batched reviewer, and PACKETS: n=<k> total_bytes=<n> fingerprint=<plan fingerprint>. collect exits 3 with
|
||||
REVIEW_COVERAGE_INCOMPLETE when any packet's output is missing or unusable, or a batched packet's
|
||||
COVERED: line leaves out a checklist it inlined.
|
||||
build prints PACKET: <role> <name> effort=<e> bytes=<n> tokens~=<n/4> budget_role=<r> read=<from-to,...> file=<path> out=<path>
|
||||
per packet (a reviewer past ${PACKET_CAP_BYTES} bytes is sharded by path as <name>.1ofK, never truncated; read= are the line
|
||||
ranges, each under ${READ_CHUNK_BYTES} bytes, a reviewer reads in parallel), PACKED: specialist batched checklists=<a,b> shards=<k>
|
||||
per batched reviewer, PACKETS: n=<k> total_bytes=<n> fingerprint=<plan fingerprint> and BUDGET_NEXT: dispatch:<one budget_role
|
||||
per packet> for gstack-ship-budget check --next. collect exits 3 with REVIEW_COVERAGE_INCOMPLETE when any packet's output is
|
||||
missing or unusable, or a batched packet's COVERED: line leaves out a checklist it inlined.
|
||||
${exitTableHelp()}`;
|
||||
|
||||
const args = process.argv.slice(2);
|
||||
@@ -82,8 +85,12 @@ function build(): number {
|
||||
if (range !== undefined && spawnSync('git', ['rev-parse', '--verify', '-q', `${range}^{commit}`], { cwd: root, encoding: 'utf8', timeout: 15_000 }).status !== 0) return fail(`--range ${range} is not a commit`);
|
||||
const diffBase = range ?? collected.mergeBase!;
|
||||
fs.mkdirSync(absOut, { recursive: true, mode: 0o700 });
|
||||
const ctx: BuildContext = { root, skillRoot: path.resolve(binDir, '..'), binDir, outDir: absOut, head, diffBase, fingerprint: plan.numbers.fingerprint, learnings: learningsFor(binDir),
|
||||
const ctx: BuildContext = { root, skillRoot: path.resolve(binDir, '..'), binDir, outDir: absOut, head, diffBase, baseRef: resolveBaseRef(root, flags.base), fingerprint: plan.numbers.fingerprint, learnings: learningsFor(binDir),
|
||||
...(planText !== undefined ? { plan: { path: flag('--plan-file')!, text: planText } } : {}) };
|
||||
const content = contentKey(ctx);
|
||||
const old = has('--findings') ? readManifest(absOut) : null;
|
||||
const stale = old && ([['head', head], ['diff_base', diffBase], ['content', content]] as const).find(([k, v]) => old[k] !== v)?.[0];
|
||||
if (stale) { process.stdout.write(`STALE_MANIFEST: ${stale} changed since the build in ${outDir} that produced --findings; the Red Team would review another tree than the specialists. Rebuild that pass's packets first.\n`); return EXIT.refused; }
|
||||
const out: string[] = [];
|
||||
let entries: PacketEntry[] = [];
|
||||
if (audit) {
|
||||
@@ -106,7 +113,7 @@ function build(): number {
|
||||
const findingsText = readFlag('--findings');
|
||||
if (findingsText === null) return fail(`--findings ${flag('--findings')} is not readable`);
|
||||
const selected = plan.dispatches.filter(d => wanted(d.role, d.name.split(' ')[0]!));
|
||||
for (const d of fanout === 'batched' ? packSpecialists(selected) : selected) {
|
||||
for (const d of fanout === 'batched' ? packSpecialists(selected, x => only.includes(x.name.split(' ')[0]!)) : selected) {
|
||||
if (d.role === 'red-team' && findingsText === undefined) {
|
||||
out.push(`PACKET_DEFERRED: red-team (after collect: build --only red-team --findings ${path.join(outDir, 'merged.jsonl')} with these flags)`);
|
||||
entries.push({ role: 'red-team', name: 'red-team', reviewer: 'red-team', effort: d.effort, file: '', out: '', bytes: 0, checklists: ['red-team'], fixtures: 'summary', shard: null, deferred: true });
|
||||
@@ -129,9 +136,10 @@ function build(): number {
|
||||
}
|
||||
out.unshift(reviewPlanLine(plan, planPass(plan, flags.pass)));
|
||||
}
|
||||
writeManifest(absOut, { version: 1, head, diff_base: diffBase, fingerprint: plan.numbers.fingerprint }, entries, has('--only'));
|
||||
writeManifest(absOut, { version: 1, head, diff_base: diffBase, fingerprint: plan.numbers.fingerprint, content }, entries, has('--only'));
|
||||
const built = entries.filter(e => !e.deferred);
|
||||
out.push(...built.map(packetLine), `PACKETS: n=${built.length} total_bytes=${built.reduce((n, e) => n + e.bytes, 0)} fingerprint=${plan.numbers.fingerprint}`);
|
||||
if (built.length) out.push(`BUDGET_NEXT: dispatch:${built.map(e => budgetRole(e.role)).join(',')}`);
|
||||
process.stdout.write(out.join('\n') + '\n');
|
||||
return EXIT.ok;
|
||||
}
|
||||
|
||||
@@ -44,7 +44,7 @@ Capy checklist for `/autoplan`: drive `gstack-autoplan`, never a hand adaptation
|
||||
|
||||
Token caps on /ship: pass a brief's cap as `--token-cap <n>`. /ship reads `cost.tokens` from `tools.thread.get({ thread_id: "this" })` (one execute program returning only that number) into `gstack-ship-budget --reading <n>` and obeys `verdict=` (`docs/ship-budget.md`).
|
||||
|
||||
/ship reviewers on Capy (`docs/review-policy.md#reviewer-packets`): one execute program runs `gstack-review-packet build` with the plan run's flags, creates one subagent per `PACKET:` line with `machine: "shared"` on the lane's model at reasoning `medium` (red-team and adversarial keep the lane's reasoning; a cross-family outside voice stays as it is), each briefed only with the packet brief plus "run `install.sh --check --for review-voice` in the same bash call that reads the packet", and starts at most the free slots of the 8-running-subagent cap per batch. The same program polls `tools.subagent.get` every ~20 s until every reviewer settles, bounded by a timeout, then runs `gstack-review-packet collect --dir <dir>` and returns only its output; never read a reviewer's transcript.
|
||||
/ship reviewers on Capy (`docs/review-policy.md#reviewer-packets`): one execute program runs `gstack-review-packet build` with the plan run's flags, creates one subagent per `PACKET:` line (`tools.subagent.create` takes at most 10 per call) with `machine: "shared"` on the lane's model at reasoning `medium` (red-team and adversarial keep the lane's reasoning; a cross-family outside voice stays as it is), and starts at most the free slots of the running-subagent cap, retrying refused starts as slots free. Each brief is the packet brief with that packet's `read=` ranges, plus one more parallel call in turn 1: `install.sh --check --for review-voice` in bash. A reviewer then finishes in about three calls: the parallel reads, at most one turn of extra ranges, one write. Under a token cap, `gstack-ship-budget check --next` takes build's `BUDGET_NEXT:` value. The same program polls `tools.subagent.get` with `transcript: "none"` every ~20 s until every reviewer settles, then runs `gstack-review-packet collect --dir <dir>` and returns only its output; keep one poll program under ~8 minutes and run it again until collect has run (a lost program loses no reviewer), and never read a reviewer's transcript.
|
||||
|
||||
## Plan, approve, ship
|
||||
|
||||
|
||||
+25
-4
@@ -348,15 +348,36 @@ writes one packet per `DISPATCH:` line holding:
|
||||
- its `DIFF_CMD` diff at `-U8` against the literal diff base (`--range <sha>` from
|
||||
pass 2 on), exclusions honored and untracked files included, under a stat; the
|
||||
security specialist, the Red Team and the native adversarial pass see tests and
|
||||
fixtures as a stat only;
|
||||
fixtures as a stat only; a data-format file (`.json`, `.jsonl`, `.ndjson`, `.csv`,
|
||||
`.tsv`, not a package or config manifest) past 400 changed lines reaches every role
|
||||
as a `DATA SUMMARY` (changed-line count, top-level keys, first and last 40 changed
|
||||
lines), except the testing reviewer and the coverage audit for data under `test/` or
|
||||
`fixtures/`;
|
||||
- at most 8 KB of `--plan-file`, and past learnings for specialists;
|
||||
- a read budget: at most 5 more reads of named ranges (200 lines at most) of files in
|
||||
the diff, no repository search, no other packets, no transcript;
|
||||
- its output path and the reply `DONE <name> crit=<n> info=<n>`.
|
||||
- a read budget: at most 2 more named ranges (200 lines at most) of files in the diff,
|
||||
in one turn, no repository search, no other packets, no transcript;
|
||||
- its output path and the reply `DONE <name> crit=<n> info=<n>`; its last line is `END OF PACKET <name>`.
|
||||
|
||||
Each `PACKET:` line carries `budget_role=` (the `gstack-ship-budget` dispatch role:
|
||||
audits reserve as `specialist`, shards as their parent role) and `read=`, line ranges
|
||||
under 30,000 bytes each. Capy's read tool returns about 37,800 characters of a file and
|
||||
drops the rest without a notice, and shell output past about 35,000 characters spills to
|
||||
a log, so a reviewer reads its packet with the read tool, one parallel call per range,
|
||||
and finishes in about three calls. `BUDGET_NEXT: dispatch:<roles>` is the value for
|
||||
`gstack-ship-budget check --next`. A partial build (`--only`) keeps other reviewers'
|
||||
entries only at the same head, diff base and diff content; any other build starts the
|
||||
manifest over and removes the old outputs, except a Red Team build (`--findings`), which refuses
|
||||
with `STALE_MANIFEST` (exit 3) so it never reviews another tree than the specialists did.
|
||||
Collect counts a matrix record only from a reviewer that applies the state-machines
|
||||
checklist, and a line naming `CRITICAL` in any case that fails to parse or validate makes its
|
||||
reviewer `invalid`.
|
||||
|
||||
A packet over 200,000 bytes is split by path into `<name>.1ofK` packets of the same
|
||||
reviewer, each with the whole instructions and checklist: no hunk is cut and no path
|
||||
dropped, and a hunk alone over the cap gets its own shard with a `SHARD_OVERSIZE` note.
|
||||
A Step 7/8 audit packet is never split: the parent gates on one coverage or plan
|
||||
verdict with one generation allowance, and a plan item built in another shard's paths
|
||||
would read NOT DONE.
|
||||
The Red Team's packet waits for the specialists (`--only red-team --findings
|
||||
<dir>/merged.jsonl`; until then collect lists it `pending`), the fix-check packet takes
|
||||
`--fix-list`, `--tests` and `--callers` with `--range $PASS_HEAD`, and `--audit
|
||||
|
||||
+4
-1
@@ -80,7 +80,10 @@ gstack-review-log --start review --reservation <id>
|
||||
```
|
||||
|
||||
Roles are review-depth's: `specialist`, `red-team`, `adversarial-native`, `outside-voice`,
|
||||
`fix-check`. `--finish <token>` is the pass's completion receipt: it settles that reservation and
|
||||
`fix-check`. /ship passes the `BUDGET_NEXT: dispatch:<roles>` value that the batch's
|
||||
`gstack-review-packet build` prints, one role per packet: a Step 7/8 audit reserves as
|
||||
`specialist` and a shard as its reviewer's role
|
||||
([review-policy.md](review-policy.md#reviewer-packets)). `--finish <token>` is the pass's completion receipt: it settles that reservation and
|
||||
every other dispatch reservation started in the same step, so reviewers admitted one at a time
|
||||
do not stay reserved. Name a reviewer that came back in your next check
|
||||
(`check ... --returned <id>[,<id>...]`), or, when other work comes first, `settle --reservation <id>
|
||||
|
||||
@@ -109,10 +109,15 @@ export function collectCommit(root: string, baseSha: string, headSha: string): C
|
||||
return result(diff, { root: skillDirs, registry, errors, mergeBase, untracked: [] });
|
||||
}
|
||||
|
||||
/** The ref a base names: origin/<base> when it resolves, else <base> as given (so origin/main stays origin/main). */
|
||||
export function resolveBaseRef(root: string, base: string): string {
|
||||
return git(root, ['rev-parse', '--verify', '-q', `origin/${base}^{commit}`]).ok ? `origin/${base}` : base;
|
||||
}
|
||||
|
||||
/** 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 ref = resolveBaseRef(root, base);
|
||||
const registry = loadRegistryAt(root, ref);
|
||||
const mb = git(root, ['merge-base', ref, 'HEAD']);
|
||||
if (!mb.ok) {
|
||||
|
||||
@@ -40,7 +40,7 @@ export function parseOutput(packet: PacketEntry, text: string | null): ReviewerR
|
||||
const r: ReviewerResult = { packet, status: 'missing', findings: [], records: 0, invalid: 0 };
|
||||
if (text === null || !text.trim()) return r;
|
||||
if (packet.role === 'outside-voice') return parseOutside(r, text);
|
||||
let clean = false;
|
||||
let clean = false, badCritical = false;
|
||||
for (const raw of text.split('\n')) {
|
||||
const line = raw.trim();
|
||||
if (!line || line.startsWith('```')) continue;
|
||||
@@ -49,13 +49,17 @@ export function parseOutput(packet: PacketEntry, text: string | null): ReviewerR
|
||||
const covered = COVERED_LINE.exec(line);
|
||||
if (covered) { r.covered = covered[1]!.split(',').map(c => c.trim()).filter(Boolean); continue; }
|
||||
let v: unknown;
|
||||
try { v = JSON.parse(line); } catch { r.invalid++; continue; }
|
||||
if (v && typeof v === 'object' && 'matrix' in v) { r.records++; continue; }
|
||||
try { v = JSON.parse(line); } catch { r.invalid++; if (/\bCRITICAL\b/i.test(line)) badCritical = true; continue; }
|
||||
if (v && typeof v === 'object' && !Array.isArray(v) && 'matrix' in v) {
|
||||
const m = (v as { matrix: unknown }).matrix;
|
||||
if (packet.checklists.includes('state-machines') && m && typeof m === 'object') r.records++; else r.invalid++;
|
||||
continue;
|
||||
}
|
||||
const f = validFinding(v, packet.role);
|
||||
if (!f) { r.invalid++; continue; }
|
||||
if (!f) { r.invalid++; if (/^critical$/i.test(String((v as { severity?: unknown } | null)?.severity))) badCritical = true; continue; }
|
||||
r.findings.push({ ...f, specialist: str(f.specialist) ? f.specialist : packet.reviewer });
|
||||
}
|
||||
r.status = r.findings.length ? 'findings' : clean || r.records || r.covered ? 'clean' : 'invalid';
|
||||
r.status = badCritical ? 'invalid' : r.findings.length ? 'findings' : clean || r.records || r.covered ? 'clean' : 'invalid';
|
||||
return r;
|
||||
}
|
||||
|
||||
|
||||
+81
-20
@@ -4,12 +4,14 @@
|
||||
* contract (lib/review-prompts.ts, the text the generated prose renders), its
|
||||
* checklists inlined, its DIFF_CMD diff at -U8 against the literal diff base with
|
||||
* a stat, at most 8 KB of the plan, a read budget, and the output path its result
|
||||
* goes to. A packet over PACKET_CAP_BYTES is sharded by path, never truncated. At every depth
|
||||
* a pass's specialist lines become one batched packet per shard (SPECIALIST_PACKING).
|
||||
* goes to. A reviewer packet over PACKET_CAP_BYTES is sharded by path, never
|
||||
* truncated; a Step 7/8 audit is one packet (one verdict, one generation allowance). At every
|
||||
* depth a pass's specialist lines become one batched packet per shard (SPECIALIST_PACKING).
|
||||
* Selection is lib/review-depth.ts planReview's (through lib/review-plan-live.ts);
|
||||
* collect is lib/review-packet-collect.ts.
|
||||
*/
|
||||
import { spawnSync } from 'node:child_process';
|
||||
import { createHash } from 'node:crypto';
|
||||
import * as fs from 'node:fs';
|
||||
import * as path from 'node:path';
|
||||
import { classifyPath, type Dispatch, type Role } from './review-depth';
|
||||
@@ -17,6 +19,10 @@ import * as P from './review-prompts';
|
||||
|
||||
export const PACKET_CAP_BYTES = 200_000;
|
||||
export const PLAN_EXCERPT_BYTES = 8 * 1024;
|
||||
/** One read-tool call's share of a packet: Capy's read returns ~37.8K chars and drops the rest silently, so packets are read as parallel ranges under this. */
|
||||
export const READ_CHUNK_BYTES = 30_000;
|
||||
/** A data-format file (.json/.jsonl/.ndjson/.csv/.tsv, not a package or config manifest) past this many changed lines reaches reviewers as a summary. */
|
||||
export const DATA_SUMMARY_LINES = 400;
|
||||
export const MANIFEST = 'manifest.json';
|
||||
export const AUDITS = ['test-coverage', 'plan-completion'] as const;
|
||||
export type AuditName = (typeof AUDITS)[number];
|
||||
@@ -31,9 +37,9 @@ export const PACKINGS: readonly Packing[] = ['batched', 'parallel'];
|
||||
*/
|
||||
export const SPECIALIST_PACKING: Packing = 'batched';
|
||||
|
||||
/** Every per-specialist dispatch of a pass as one `specialist batched` dispatch in the first one's place: checklists in plan order, the diff all of them read (the excludes they share). */
|
||||
export function packSpecialists(dispatches: readonly Dispatch[]): Dispatch[] {
|
||||
const each = dispatches.filter(d => d.role === 'specialist' && !d.checklists);
|
||||
/** Every per-specialist dispatch of a pass as one `specialist batched` dispatch in the first one's place: checklists in plan order, the diff all of them read (the excludes they share). `keep` leaves a specialist its own packet (build's --only naming it). */
|
||||
export function packSpecialists(dispatches: readonly Dispatch[], keep: (d: Dispatch) => boolean = () => false): Dispatch[] {
|
||||
const each = dispatches.filter(d => d.role === 'specialist' && !d.checklists && !keep(d));
|
||||
if (each.length < 2) return [...dispatches];
|
||||
const first = each[0]!;
|
||||
const batched: Dispatch = { role: 'specialist', name: 'batched', effort: first.effort, exclude: first.exclude.filter(x => each.every(d => d.exclude.includes(x))), checklists: each.map(d => d.name) };
|
||||
@@ -50,13 +56,17 @@ export function auditCodePaths(paths: readonly string[], root: string): string[]
|
||||
export interface PacketEntry {
|
||||
role: PacketRole; name: string; reviewer: string; effort: string; file: string; out: string; bytes: number;
|
||||
checklists: string[]; fixtures: 'full' | 'summary'; shard: { index: number; total: number; oversize: boolean } | null;
|
||||
/** Line ranges (`from-to`, comma-joined) that read the packet whole, each under READ_CHUNK_BYTES. */
|
||||
read?: string;
|
||||
/** The Red Team before collect: listed so collect reports it pending until its packet is built. */
|
||||
deferred?: boolean;
|
||||
}
|
||||
export interface Manifest { version: 1; head: string; diff_base: string; fingerprint: string; packets: PacketEntry[] }
|
||||
export interface Manifest { version: 1; head: string; diff_base: string; fingerprint: string; content?: string; packets: PacketEntry[] }
|
||||
|
||||
export interface BuildContext {
|
||||
root: string; skillRoot: string; binDir: string; outDir: string; head: string; diffBase: string; fingerprint: string;
|
||||
/** The ref `origin/<base>` stands for in prompt text (lib/review-collect.ts resolveBaseRef). */
|
||||
baseRef?: string;
|
||||
plan?: { path: string; text: string };
|
||||
/** Merged findings for the Red Team (collect's merged.jsonl), one summary line each. */
|
||||
findings?: string[];
|
||||
@@ -100,7 +110,31 @@ export function checklistPath(skillRoot: string, name: string): string {
|
||||
export interface ReviewerDiff { stat: string; chunks: string[]; fixtureStat: string | null; paths: string[] }
|
||||
|
||||
/** The reviewer's DIFF_CMD at -U8 (tracked and untracked, exclusions honored) split per file; fixtures go to a stat in summary mode. */
|
||||
export function reviewerDiff(root: string, diffBase: string, exclude: readonly string[], summary: boolean, skip: (p: string) => boolean): ReviewerDiff {
|
||||
const DATA_FILE = /\.(json|jsonl|ndjson|csv|tsv)$/i;
|
||||
const MANIFEST_FILE = /^(package(-lock)?\.json|composer\.json|deno\.jsonc?|tsconfig[^/]*\.json|[^/]*\.config\.json)$/i;
|
||||
|
||||
/** Keys or columns read from the diff's own lines (added, else removed), never from the working tree, which a symlink can point outside the diff. */
|
||||
function topLevelKeys(file: string, changed: string[]): string {
|
||||
const side = changed.some(l => l.startsWith('+')) ? '+' : '-';
|
||||
const text = changed.filter(l => l.startsWith(side)).map(l => l.slice(1)).join('\n');
|
||||
try {
|
||||
if (/\.(csv|tsv)$/i.test(file)) return `columns: ${text.split('\n')[0]!.slice(0, 300)}`;
|
||||
const v = /\.json$/i.test(file) ? JSON.parse(text) : JSON.parse(text.split('\n').find(l => l.trim()) ?? 'null');
|
||||
const keys = (o: unknown) => (o && typeof o === 'object' && !Array.isArray(o) ? Object.keys(o).slice(0, 30).join(', ') : '');
|
||||
return Array.isArray(v) ? `array of ${v.length}; first item keys: ${keys(v[0]) || '(none)'}` : `top-level keys: ${keys(v) || '(none)'}`;
|
||||
} catch { return 'top-level keys: (not parsed)'; }
|
||||
}
|
||||
|
||||
/** A large data-format file's chunk as a summary: its changed-line count, keys, and the first and last 40 changed lines. */
|
||||
function dataSummary(p: string, chunk: string): string | null {
|
||||
if (!DATA_FILE.test(p) || MANIFEST_FILE.test(path.basename(p))) return null;
|
||||
const changed = chunk.split('\n').filter(l => /^[+-]/.test(l) && !/^(\+\+\+|---) /.test(l));
|
||||
if (changed.length <= DATA_SUMMARY_LINES) return null;
|
||||
return [`diff --git a/${p} b/${p}`, `DATA SUMMARY: ${p} (${changed.length} changed lines, a data file shown as a summary; ask the parent for a line range if a finding needs one)`,
|
||||
topLevelKeys(p, changed), 'first 40 changed lines:', ...changed.slice(0, 40), '…', 'last 40 changed lines:', ...changed.slice(-40), ''].join('\n');
|
||||
}
|
||||
|
||||
export function reviewerDiff(root: string, diffBase: string, exclude: readonly string[], summary: boolean, skip: (p: string) => boolean, keepData: (p: string) => boolean = () => false): ReviewerDiff {
|
||||
const fixtureSpecs = summary ? P.FIXTURE_PATHSPECS.map(p => `:(exclude)${p}`) : [];
|
||||
const spec = ['--', '.', ...exclude, ...fixtureSpecs];
|
||||
const diff = git(root, ['diff', '-U8', '--no-color', '--no-ext-diff', '--no-renames', diffBase, ...spec]);
|
||||
@@ -119,7 +153,8 @@ export function reviewerDiff(root: string, diffBase: string, exclude: readonly s
|
||||
fixtureStat = [git(root, ['diff', '--stat=200', '--no-renames', diffBase, ...fx]).trimEnd(), ...listed.map(p => ` ${p} | untracked`)].filter(Boolean).join('\n') || '(no test or fixture changes)';
|
||||
}
|
||||
const paths = chunks.map(c => /^diff --git a\/(.*?) b\//.exec(c)?.[1] ?? '?');
|
||||
return { stat: stat || '(empty diff)', chunks, fixtureStat, paths };
|
||||
const shown = chunks.map((c, i) => (keepData(paths[i]!) ? c : dataSummary(paths[i]!, c) ?? c));
|
||||
return { stat: stat || '(empty diff)', chunks: shown, fixtureStat, paths };
|
||||
}
|
||||
|
||||
/** Diff pieces packed into shards under `budget` bytes; a file over it splits at hunks; a hunk over it is its own oversize shard. */
|
||||
@@ -193,7 +228,21 @@ function render(draft: PacketDraft, name: string, out: string, shard: { index: n
|
||||
`## Output\n\nOutput path: \`${out}\``, '', ...draft.tail,
|
||||
`## Diff stat\n\n${block(draft.diff.stat)}`, '',
|
||||
...(draft.diff.fixtureStat !== null ? [`## Tests and fixtures (summary mode)\n\n${block(draft.diff.fixtureStat)}`, ''] : []),
|
||||
`## ${draft.diffLabel} (\`${draft.diffCmd}\`, untracked files included)\n\n${block(diffText, 'diff')}`, ''].join('\n');
|
||||
`## ${draft.diffLabel} (\`${draft.diffCmd}\`, untracked files included)\n\n${block(diffText, 'diff')}`, '', `END OF PACKET ${name}`, ''].join('\n');
|
||||
}
|
||||
|
||||
/** Line ranges that read `text` whole, each at most READ_CHUNK_BYTES (a single longer line is its own range). */
|
||||
export function readRanges(text: string): string {
|
||||
const lines = text.replace(/\n$/, '').split('\n');
|
||||
const out: string[] = [];
|
||||
let from = 1, size = 0;
|
||||
lines.forEach((l, i) => {
|
||||
const b = bytes(l) + 1;
|
||||
if (size && size + b > READ_CHUNK_BYTES) { out.push(`${from}-${i}`); from = i + 1; size = 0; }
|
||||
size += b;
|
||||
});
|
||||
out.push(`${from}-${lines.length}`);
|
||||
return out.join(',');
|
||||
}
|
||||
|
||||
/** One reviewer's packet files (one, or k shards), written under outDir; returns their manifest entries. */
|
||||
@@ -203,7 +252,7 @@ export function writePackets(draft: PacketDraft, ctx: BuildContext): PacketEntry
|
||||
const fileOf = (name: string) => path.join(ctx.outDir, `${name}.packet.md`);
|
||||
const fixed = bytes(render(draft, `${draft.name}.99of99`, outOf(`${draft.name}.99of99`), { index: 99, total: 99, oversize: true }, ''));
|
||||
const budget = Math.max(cap - fixed - 64, 1);
|
||||
let shards = shardChunks(draft.diff.chunks, budget);
|
||||
let shards = draft.role === 'audit' ? [{ text: draft.diff.chunks.join(''), oversize: false }] : shardChunks(draft.diff.chunks, budget);
|
||||
const total = draft.diff.chunks.reduce((n, c) => n + bytes(c), 0);
|
||||
for (let b = Math.ceil((total / shards.length) * 1.02); shards.length > 1 && b < budget; b = Math.ceil(b * 1.05)) {
|
||||
const even = shardChunks(draft.diff.chunks, b);
|
||||
@@ -215,7 +264,7 @@ export function writePackets(draft: PacketDraft, ctx: BuildContext): PacketEntry
|
||||
const text = render(draft, name, outOf(name), shard, s.text);
|
||||
fs.writeFileSync(fileOf(name), text, { mode: 0o600 });
|
||||
fs.rmSync(outOf(name), { force: true });
|
||||
return { role: draft.role, name, reviewer: draft.name, effort: draft.effort, file: fileOf(name), out: outOf(name), bytes: bytes(text), checklists: draft.checklists, fixtures: draft.summary ? 'summary' : 'full', shard };
|
||||
return { role: draft.role, name, reviewer: draft.name, effort: draft.effort, file: fileOf(name), out: outOf(name), bytes: bytes(text), checklists: draft.checklists, fixtures: draft.summary ? 'summary' : 'full', shard, read: readRanges(text) };
|
||||
});
|
||||
}
|
||||
|
||||
@@ -223,6 +272,7 @@ const diffCmd = (base: string, exclude: readonly string[], summary: boolean) =>
|
||||
const specs = [...exclude, ...(summary ? P.FIXTURE_PATHSPECS.map(p => `:(exclude)${p}`) : [])];
|
||||
return `git diff -U8 ${base}${specs.length ? ` -- . ${specs.map(x => `'${x}'`).join(' ')}` : ''}`;
|
||||
};
|
||||
const underTests = (p: string) => /(^|\/)(tests?|fixtures?|__tests__)\//.test(p);
|
||||
const insideOut = (ctx: BuildContext) => (p: string) => p.startsWith('.gstack/tmp/') || path.resolve(ctx.root, p).startsWith(path.resolve(ctx.outDir) + path.sep);
|
||||
|
||||
function common(ctx: BuildContext, stack: string, testFw: string, learnings: string | null, budget: string): string[] {
|
||||
@@ -244,37 +294,43 @@ export function draftFor(d: Dispatch, ctx: BuildContext): PacketDraft {
|
||||
`## Instructions\n\n${t.instructions.join('\n\n')}`, '', `## Findings contract\n\n${t.contract}\n\n${t.reply}`, '',
|
||||
...t.extra.flatMap(s => [s, '']), ...common(ctx, stack, testFw, learnings, P.READ_BUDGET), ...checklists.flatMap(s => [s, ''])];
|
||||
return { role: d.role, name: d.name.split(' ')[0]!, effort: d.effort, checklists: t.checklists, summary: t.summary, head, tail: [],
|
||||
diff: reviewerDiff(ctx.root, ctx.diffBase, d.exclude, t.summary, insideOut(ctx)), diffLabel: t.diffLabel, diffCmd: diffCmd(ctx.diffBase, d.exclude, t.summary) };
|
||||
diff: reviewerDiff(ctx.root, ctx.diffBase, d.exclude, t.summary, insideOut(ctx), d.role === 'specialist' && (d.checklists ? d.checklists.includes('testing') : d.name === 'testing') ? underTests : undefined), diffLabel: t.diffLabel, diffCmd: diffCmd(ctx.diffBase, d.exclude, t.summary) };
|
||||
}
|
||||
|
||||
/** An audit's prompt is the generated section's text fence: the packet and the SKILL.md prose are one text. */
|
||||
export function auditPrompt(skillRoot: string, audit: AuditName, base: string): string {
|
||||
export function auditPrompt(skillRoot: string, audit: AuditName, base: string, baseRef = `origin/${base}`): string {
|
||||
const section = fs.readFileSync(path.join(skillRoot, 'ship', 'sections', `${audit}.md`), 'utf8');
|
||||
const m = /^(`{4,})text\n([\s\S]*?)\n\1$/m.exec(section);
|
||||
if (!m) throw new Error(`ship/sections/${audit}.md has no prompt fence`);
|
||||
return m[2]!.replaceAll('<base>', base);
|
||||
return m[2]!.replaceAll('origin/<base>', baseRef).replaceAll('<base>', base);
|
||||
}
|
||||
|
||||
export function auditDraft(audit: AuditName, ctx: BuildContext, base: string, effort: string): PacketDraft {
|
||||
const { stack, testFw } = detectStack(ctx.root);
|
||||
const budget = audit === 'test-coverage' ? P.AUDIT_READ_ONLY : 'The diff is below; do not rerun git diff. Read only files in the diff, their tests and the files the plan names, in named line ranges of at most 200 lines; no whole-repository search, no other packets, no transcript. Writes and test runs only as the audit instructions permit.';
|
||||
const plan = audit === 'plan-completion' && ctx.plan ? [`## Plan (whole, \`${ctx.plan.path}\`)\n\n${block(ctx.plan.text)}`, ''] : [];
|
||||
const head = [`Role: ${audit} audit (effort=${effort}).`, '', `## Instructions\n\n${auditPrompt(ctx.skillRoot, audit, base).replace('<allowed|audit-only>', 'audit-only')}`, '',
|
||||
const head = [`Role: ${audit} audit (effort=${effort}).`, '', `## Instructions\n\n${auditPrompt(ctx.skillRoot, audit, base, ctx.baseRef).replace('<allowed|audit-only>', 'audit-only')}`, '',
|
||||
`## Output\n\nWrite your whole final answer to the output path below, its LAST line the JSON object the instructions name. Then reply with exactly one line, \`DONE <NAME>\`.`, '',
|
||||
...(ctx.context ? [`## Parent context\n\n${block(ctx.context.trim())}`, ''] : []),
|
||||
...common({ ...ctx, plan: audit === 'plan-completion' ? undefined : ctx.plan }, stack, testFw, null, budget), ...plan];
|
||||
return { role: 'audit', name: audit, effort, checklists: [], summary: false, head, tail: [], diff: reviewerDiff(ctx.root, ctx.diffBase, [], false, insideOut(ctx)), diffLabel: 'Diff', diffCmd: diffCmd(ctx.diffBase, [], false) };
|
||||
return { role: 'audit', name: audit, effort, checklists: [], summary: false, head, tail: [], diff: reviewerDiff(ctx.root, ctx.diffBase, [], false, insideOut(ctx), audit === 'test-coverage' ? underTests : undefined), diffLabel: 'Diff', diffCmd: diffCmd(ctx.diffBase, [], false) };
|
||||
}
|
||||
|
||||
/** The reviewed content's identity: the whole diff a reviewer could be shown, hashed; a partial build keeps other reviewers only when it is unchanged. */
|
||||
export function contentKey(ctx: BuildContext): string {
|
||||
return createHash('sha256').update(reviewerDiff(ctx.root, ctx.diffBase, [], false, insideOut(ctx), () => true).chunks.join('')).digest('hex').slice(0, 16);
|
||||
}
|
||||
|
||||
export function readManifest(dir: string): Manifest | null {
|
||||
try { return JSON.parse(fs.readFileSync(path.join(dir, MANIFEST), 'utf8')) as Manifest; } catch { return null; }
|
||||
}
|
||||
|
||||
/** Write the manifest; a partial build (--only) replaces entries of the same reviewer and keeps the rest. */
|
||||
/** Write the manifest; a partial build (--only) at the same head, diff base and content replaces entries of the same reviewer and keeps the rest; any other build starts over. */
|
||||
export function writeManifest(dir: string, m: Omit<Manifest, 'packets'>, packets: PacketEntry[], partial: boolean): Manifest {
|
||||
const prior = partial ? readManifest(dir) : null;
|
||||
const old = readManifest(dir);
|
||||
const prior = partial && old && old.head === m.head && old.diff_base === m.diff_base && old.content === m.content ? old : null;
|
||||
const rebuilt = new Set(packets.map(p => p.reviewer));
|
||||
if (!partial) for (const old of readManifest(dir)?.packets ?? []) { if (old.deferred) continue; fs.rmSync(old.out, { force: true }); if (!packets.some(p => p.file === old.file)) fs.rmSync(old.file, { force: true }); }
|
||||
if (!prior) for (const old of readManifest(dir)?.packets ?? []) { if (old.deferred) continue; fs.rmSync(old.out, { force: true }); if (!packets.some(p => p.file === old.file)) fs.rmSync(old.file, { force: true }); }
|
||||
const kept = (prior?.packets ?? []).filter(p => !rebuilt.has(p.reviewer));
|
||||
const manifest: Manifest = { ...m, packets: [...kept, ...packets] };
|
||||
fs.writeFileSync(path.join(dir, MANIFEST), JSON.stringify(manifest, null, 2) + '\n', { mode: 0o600 });
|
||||
@@ -282,5 +338,10 @@ export function writeManifest(dir: string, m: Omit<Manifest, 'packets'>, packets
|
||||
}
|
||||
|
||||
export function packetLine(p: PacketEntry): string {
|
||||
return `PACKET: ${p.role} ${p.name} effort=${p.effort} bytes=${p.bytes} tokens~=${Math.ceil(p.bytes / 4)} file=${p.file} out=${p.out}`;
|
||||
return `PACKET: ${p.role} ${p.name} effort=${p.effort} bytes=${p.bytes} tokens~=${Math.ceil(p.bytes / 4)} budget_role=${budgetRole(p.role)} read=${p.read ?? '1-1'} file=${p.file} out=${p.out}`;
|
||||
}
|
||||
|
||||
/** The gstack-ship-budget dispatch role a packet reserves as: audits as a specialist reader, shards as their parent role. */
|
||||
export function budgetRole(role: PacketRole): Exclude<PacketRole, 'audit'> {
|
||||
return role === 'audit' ? 'specialist' : role;
|
||||
}
|
||||
@@ -74,10 +74,10 @@ export function renderFixCheckPrompt(i: FixCheckInput): string {
|
||||
}
|
||||
|
||||
/** What every reviewer packet allows beyond itself. */
|
||||
export const READ_BUDGET = 'Everything you need is in this packet. You may make at most 5 more reads, each a named line range of at most 200 lines of a file in this diff (code around a hunk, a caller the diff changed). No whole-repository search, no git commands, no other packets, no transcript, no web.';
|
||||
export const READ_BUDGET = 'Everything you need is in this packet. You may read at most 2 more named line ranges, each at most 200 lines of a file in this diff (code around a hunk, a caller the diff changed), both in one turn as parallel calls. No whole-repository search, no git commands, no other packets, no transcript, no web.';
|
||||
|
||||
/** The coverage audit packet's budget: an audit, never a writer; the parent writes and runs the tests it proposes. */
|
||||
export const AUDIT_READ_ONLY = 'This audit is read-only and takes three calls: read this packet, write the output file, reply with the DONE line. Make no edits, write no tests, run no tests, git or other commands and read no other file, whatever the audit instructions say about generating, running or reading: the parent writes and runs the tests. Judge coverage from the diff below and the tests it changes; put each test you would write, with its value card, in the diagram as a gap, mark a path whose tests this packet does not show as an unverified gap for the parent to confirm, and leave `tests_added` and `tests_extended` empty.';
|
||||
export const AUDIT_READ_ONLY = 'This audit is read-only and keeps the brief to three turns: read the read= ranges of this packet, write the output file, reply with the DONE line. Make no edits, write no tests, run no tests, git or other commands and read no other file, whatever the audit instructions say about generating, running or reading: the parent writes and runs the tests. Judge coverage from the diff below and the tests it changes; put each test you would write, with its value card, in the diagram as a gap, mark a path whose tests this packet does not show as an unverified gap for the parent to confirm, and leave `tests_added` and `tests_extended` empty.';
|
||||
|
||||
/** A packet that applies the state-machines checklist writes its matrix records to its output file. */
|
||||
export const STATE_MATRIX_OUTPUT = 'Your output file is also the matrix record file: write the matrix record(s) the checklist describes there with your findings; the parent runs `gstack-state-matrix check` on it.';
|
||||
@@ -90,5 +90,5 @@ export const OUTSIDE_CONTRACT = 'Write one finding per line as `[Critical] path:
|
||||
|
||||
/** The whole brief a packet reviewer receives; the packet carries the rest. */
|
||||
export function packetBrief(file: string, out: string): string {
|
||||
return `Read ${file} and follow it: it is your whole task. Write your result to ${out}. Reply with only the DONE line it names.`;
|
||||
return `Turn 1: read ${file} with your file-reading tool (never cat: shell output past ~35 KB spills to a log), one parallel call per line range its PACKET line lists as read=; load no skill and read no SKILL.md. The packet is your whole task and ends with END OF PACKET. Turn 2, only if needed: at most 2 more ranges, in parallel. Last turn: write your result to ${out} in one write and reply with only the DONE line it names.`;
|
||||
}
|
||||
@@ -129,7 +129,7 @@ Those files are part of the recorded content too.
|
||||
|
||||
Dispatch via the Agent tool with ${FOREGROUND_IF_AVAILABLE} (background is the default since ${CC_BACKGROUND_DEFAULT_SINCE}); findings must arrive before review concludes. ${BACKGROUND_RECOVERY} Fresh context avoids checklist bias, but this is the same harness, not an independent model unless runtime identity proves otherwise.
|
||||
|
||||
${isShip ? `Build Step 11's packets from Step 9's last full-pass plan: \`~/.claude/skills/gstack/bin/gstack-review-packet build --only adversarial,outside-voice --base <base> --out-dir "$_PKA"\` with \`_PKA=.gstack/tmp/review-packets-adv-p$PASS\`, that plan run's flags and \`--range <RANGE_BASE sha>\` from pass 2 on. The \`adversarial\` packet carries the defensive framing, the prompt below, the diff with fixtures in summary mode and a JSONL contract (\`NO FINDINGS\` is the expected result for a clean range). Its whole brief is "${packetBrief('<file>', '<out>')}"` : `Subagent prompt:
|
||||
${isShip ? `Build Step 11's packets from Step 9's last full-pass plan: \`~/.claude/skills/gstack/bin/gstack-review-packet build --only adversarial,outside-voice --base <base> --out-dir "$_PKA"\` with \`_PKA=.gstack/tmp/review-packets-adv-p$PASS\`, that plan run's flags and \`--range <RANGE_BASE sha>\` from pass 2 on. Leave \`outside-voice\` out of \`--only\` when nothing will run it (\`CODEX_MODE\` neither \`ready\` nor \`unverified\` and no reviewer on another model family): collect counts a built packet with no output as missing coverage. The \`adversarial\` packet carries the defensive framing, the native adversarial prompt (\`ADVERSARIAL_NATIVE\`), the diff with fixtures in summary mode and a JSONL contract (\`NO FINDINGS\` is the expected result for a clean range). Its whole brief is "${packetBrief('<file>', '<out>')}" with that packet's \`read=\` ranges.` : `Subagent prompt:
|
||||
"${DEFENSIVE_REVIEW_FRAMING}
|
||||
|
||||
Read the diff for this branch. First list changed files: \`DIFF_BASE=$(git merge-base origin/<base> HEAD) && git diff --name-status "$DIFF_BASE"\`. ${FIXTURE_SUMMARY_MODE}
|
||||
|
||||
@@ -169,12 +169,15 @@ its \`DIFF_CMD:\` diff at -U8 against the literal diff base with a stat, past le
|
||||
--out-dir "$_PK" --only specialist,red-team\` plus the plan run's flags, \`--range <sha>\` (the literal \`RANGE_BASE\` sha)
|
||||
from pass 2 on and \`--plan-file <plan>\` when a plan is bound. Each \`PACKET:\` line is one
|
||||
reviewer; a diff past 200 KB is split by path into shards (\`batched.1of3\`), each its
|
||||
own reviewer. \`PACKETS:\` carries the plan's fingerprint. Step 9.1 dispatches every
|
||||
own reviewer. Its \`read=\` ranges are what the reviewer reads in parallel, each under one
|
||||
read call's budget. \`PACKETS:\` carries the plan's fingerprint; under a token cap,
|
||||
\`gstack-ship-budget check --next\` takes build's \`BUDGET_NEXT:\` value (one
|
||||
\`budget_role=\` per packet), never packet names. Step 9.1 dispatches every
|
||||
packet; the Red Team's waits for collect (below) and Step 11 builds its own.
|
||||
|
||||
Launch one subagent per packet via the Agent tool (\`subagent_type: "general-purpose"\`,
|
||||
${FOREGROUND_IF_AVAILABLE}; ${BACKGROUND_RECOVERY}) at its \`effort=\` on the same model
|
||||
family. The whole brief is "${packetBrief('<file>', '<out>')}" Never paste a diff,
|
||||
family. The whole brief is "${packetBrief('<file>', '<out>')}" with that packet's \`read=\` ranges. Never paste a diff,
|
||||
checklist or finding into a brief.
|
||||
|
||||
**Fan-out (\`--review-fanout auto|serial|parallel\` in the prompt; default \`auto\`).**
|
||||
|
||||
@@ -5,8 +5,8 @@
|
||||
`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. Under a token cap,
|
||||
reserve the planned voices as one batch (`gstack-ship-budget check --next dispatch:<roles>`)
|
||||
and add `--reservation <id>` to the `--start adversarial-review` call below.
|
||||
reserve the planned voices as one batch (`gstack-ship-budget check --next <the BUDGET_NEXT value
|
||||
Step 11's packet build prints>`, never packet names) and add `--reservation <id>` to the `--start adversarial-review` call below.
|
||||
|
||||
## Step 11: Adversarial review
|
||||
|
||||
@@ -98,7 +98,7 @@ Those files are part of the recorded content too.
|
||||
|
||||
Dispatch via the Agent tool with `run_in_background: false` when available (background is the default since Claude Code v2.1.198); findings must arrive before review concludes. A launch receipt means it went background: await its completion notice. Fresh context avoids checklist bias, but this is the same harness, not an independent model unless runtime identity proves otherwise.
|
||||
|
||||
Build Step 11's packets from Step 9's last full-pass plan: `~/.claude/skills/gstack/bin/gstack-review-packet build --only adversarial,outside-voice --base <base> --out-dir "$_PKA"` with `_PKA=.gstack/tmp/review-packets-adv-p$PASS`, that plan run's flags and `--range <RANGE_BASE sha>` from pass 2 on. The `adversarial` packet carries the defensive framing, the prompt below, the diff with fixtures in summary mode and a JSONL contract (`NO FINDINGS` is the expected result for a clean range). Its whole brief is "Read <file> and follow it: it is your whole task. Write your result to <out>. Reply with only the DONE line it names."
|
||||
Build Step 11's packets from Step 9's last full-pass plan: `~/.claude/skills/gstack/bin/gstack-review-packet build --only adversarial,outside-voice --base <base> --out-dir "$_PKA"` with `_PKA=.gstack/tmp/review-packets-adv-p$PASS`, that plan run's flags and `--range <RANGE_BASE sha>` from pass 2 on. Leave `outside-voice` out of `--only` when nothing will run it (`CODEX_MODE` neither `ready` nor `unverified` and no reviewer on another model family): collect counts a built packet with no output as missing coverage. The `adversarial` packet carries the defensive framing, the native adversarial prompt (`ADVERSARIAL_NATIVE`), the diff with fixtures in summary mode and a JSONL contract (`NO FINDINGS` is the expected result for a clean range). Its whole brief is "Turn 1: read <file> with your file-reading tool (never cat: shell output past ~35 KB spills to a log), one parallel call per line range its PACKET line lists as read=; load no skill and read no SKILL.md. The packet is your whole task and ends with END OF PACKET. Turn 2, only if needed: at most 2 more ranges, in parallel. Last turn: write your result to <out> in one write and reply with only the DONE line it names." with that packet's `read=` ranges.
|
||||
|
||||
Present findings under an `ADVERSARIAL REVIEW (Claude subagent):` header. **FIXABLE findings** are queued for the parent; do not edit during Step 11. **INVESTIGATE findings** are presented as informational.
|
||||
|
||||
|
||||
@@ -3,8 +3,8 @@
|
||||
`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. Under a token cap,
|
||||
reserve the planned voices as one batch (`gstack-ship-budget check --next dispatch:<roles>`)
|
||||
and add `--reservation <id>` to the `--start adversarial-review` call below.
|
||||
reserve the planned voices as one batch (`gstack-ship-budget check --next <the BUDGET_NEXT value
|
||||
Step 11's packet build prints>`, never packet names) and add `--reservation <id>` to the `--start adversarial-review` call below.
|
||||
|
||||
{{ADVERSARIAL_STEP}}
|
||||
|
||||
|
||||
@@ -18,10 +18,12 @@ Field-by-field reference: docs/ship-budget.md. Exit 0 continue or delegate, 3 st
|
||||
|
||||
**Every boundary.** `gstack-ship-budget check --run <id> --step <done> --next <step> --reading <n>`.
|
||||
Before each dispatch batch, reserve the batch as one sum, then pass the printed id once
|
||||
(`READING` is the current counter reading; `RESERVATION_ID` is the id the check prints):
|
||||
(`READING` is the current counter reading; `RESERVATION_ID` is the id the check prints;
|
||||
`--next` is the `BUDGET_NEXT:` value the batch's `gstack-review-packet build` printed, one
|
||||
role per packet, never packet names):
|
||||
|
||||
```bash
|
||||
~/.claude/skills/gstack/bin/gstack-ship-budget check --run <id> --step 9 --next dispatch:specialist,red-team --reading "$READING"
|
||||
~/.claude/skills/gstack/bin/gstack-ship-budget check --run <id> --step 9 --next "$BUDGET_NEXT" --reading "$READING"
|
||||
~/.claude/skills/gstack/bin/gstack-review-log --start review --reservation "$RESERVATION_ID"
|
||||
```
|
||||
|
||||
|
||||
@@ -16,10 +16,12 @@ Field-by-field reference: docs/ship-budget.md. Exit 0 continue or delegate, 3 st
|
||||
|
||||
**Every boundary.** `gstack-ship-budget check --run <id> --step <done> --next <step> --reading <n>`.
|
||||
Before each dispatch batch, reserve the batch as one sum, then pass the printed id once
|
||||
(`READING` is the current counter reading; `RESERVATION_ID` is the id the check prints):
|
||||
(`READING` is the current counter reading; `RESERVATION_ID` is the id the check prints;
|
||||
`--next` is the `BUDGET_NEXT:` value the batch's `gstack-review-packet build` printed, one
|
||||
role per packet, never packet names):
|
||||
|
||||
```bash
|
||||
~/.claude/skills/gstack/bin/gstack-ship-budget check --run <id> --step 9 --next dispatch:specialist,red-team --reading "$READING"
|
||||
~/.claude/skills/gstack/bin/gstack-ship-budget check --run <id> --step 9 --next "$BUDGET_NEXT" --reading "$READING"
|
||||
~/.claude/skills/gstack/bin/gstack-review-log --start review --reservation "$RESERVATION_ID"
|
||||
```
|
||||
|
||||
|
||||
@@ -373,12 +373,15 @@ its `DIFF_CMD:` diff at -U8 against the literal diff base with a stat, past lear
|
||||
--out-dir "$_PK" --only specialist,red-team` plus the plan run's flags, `--range <sha>` (the literal `RANGE_BASE` sha)
|
||||
from pass 2 on and `--plan-file <plan>` when a plan is bound. Each `PACKET:` line is one
|
||||
reviewer; a diff past 200 KB is split by path into shards (`batched.1of3`), each its
|
||||
own reviewer. `PACKETS:` carries the plan's fingerprint. Step 9.1 dispatches every
|
||||
own reviewer. Its `read=` ranges are what the reviewer reads in parallel, each under one
|
||||
read call's budget. `PACKETS:` carries the plan's fingerprint; under a token cap,
|
||||
`gstack-ship-budget check --next` takes build's `BUDGET_NEXT:` value (one
|
||||
`budget_role=` per packet), never packet names. Step 9.1 dispatches every
|
||||
packet; the Red Team's waits for collect (below) and Step 11 builds its own.
|
||||
|
||||
Launch one subagent per packet via the Agent tool (`subagent_type: "general-purpose"`,
|
||||
`run_in_background: false` when available; A launch receipt means it went background: await its completion notice.) at its `effort=` on the same model
|
||||
family. The whole brief is "Read <file> and follow it: it is your whole task. Write your result to <out>. Reply with only the DONE line it names." Never paste a diff,
|
||||
family. The whole brief is "Turn 1: read <file> with your file-reading tool (never cat: shell output past ~35 KB spills to a log), one parallel call per line range its PACKET line lists as read=; load no skill and read no SKILL.md. The packet is your whole task and ends with END OF PACKET. Turn 2, only if needed: at most 2 more ranges, in parallel. Last turn: write your result to <out> in one write and reply with only the DONE line it names." with that packet's `read=` ranges. Never paste a diff,
|
||||
checklist or finding into a brief.
|
||||
|
||||
**Fan-out (`--review-fanout auto|serial|parallel` in the prompt; default `auto`).**
|
||||
|
||||
@@ -18,11 +18,11 @@ Dispatch the audit through Agent with `subagent_type: "general-purpose"` and
|
||||
run `~/.claude/skills/gstack/bin/gstack-review-packet build --audit test-coverage --base <base>
|
||||
--out-dir .gstack/tmp/review-packets-audit --context .gstack/tmp/audit-context.txt
|
||||
--passes-used <N>` (the packet holds the prompt below and
|
||||
the diff) and brief the child only "Read <file> and follow it: it is your whole task. Write
|
||||
your result to <out>. Reply with only the DONE line it names." Wait for its `out=` file's
|
||||
LAST-line JSON before applying the coverage gate. `AUDIT_SKIPPED:` (no code path: docs,
|
||||
the diff; an audit is one packet, never sharded) and brief the child only with the packet brief and its `read=` ranges:
|
||||
"Turn 1: read <file> with your file-reading tool (never cat: shell output past ~35 KB spills to a log), one parallel call per line range its PACKET line lists as read=; load no skill and read no SKILL.md. The packet is your whole task and ends with END OF PACKET. Turn 2, only if needed: at most 2 more ranges, in parallel. Last turn: write your result to <out> in one write and reply with only the DONE line it names."
|
||||
Wait for its `out=` file's LAST-line JSON before applying the coverage gate. `AUDIT_SKIPPED:` (no code path: docs,
|
||||
data, tests) builds nothing: print it, dispatch nothing, apply the gate with zero paths.
|
||||
The packet is read-only in three calls; with generation allowed,
|
||||
The packet is read-only; with generation allowed,
|
||||
the parent writes and runs the tests its diagram proposes (caps and value bar below)
|
||||
and lists them in `tests_added`/`tests_extended` before item 3.
|
||||
|
||||
|
||||
@@ -16,11 +16,11 @@ Dispatch the audit through Agent with `subagent_type: "general-purpose"` and
|
||||
run `~/.claude/skills/gstack/bin/gstack-review-packet build --audit test-coverage --base <base>
|
||||
--out-dir .gstack/tmp/review-packets-audit --context .gstack/tmp/audit-context.txt
|
||||
--passes-used <N>` (the packet holds the prompt below and
|
||||
the diff) and brief the child only "Read <file> and follow it: it is your whole task. Write
|
||||
your result to <out>. Reply with only the DONE line it names." Wait for its `out=` file's
|
||||
LAST-line JSON before applying the coverage gate. `AUDIT_SKIPPED:` (no code path: docs,
|
||||
the diff; an audit is one packet, never sharded) and brief the child only with the packet brief and its `read=` ranges:
|
||||
"Turn 1: read <file> with your file-reading tool (never cat: shell output past ~35 KB spills to a log), one parallel call per line range its PACKET line lists as read=; load no skill and read no SKILL.md. The packet is your whole task and ends with END OF PACKET. Turn 2, only if needed: at most 2 more ranges, in parallel. Last turn: write your result to <out> in one write and reply with only the DONE line it names."
|
||||
Wait for its `out=` file's LAST-line JSON before applying the coverage gate. `AUDIT_SKIPPED:` (no code path: docs,
|
||||
data, tests) builds nothing: print it, dispatch nothing, apply the gate with zero paths.
|
||||
The packet is read-only in three calls; with generation allowed,
|
||||
The packet is read-only; with generation allowed,
|
||||
the parent writes and runs the tests its diagram proposes (caps and value bar below)
|
||||
and lists them in `tests_added`/`tests_extended` before item 3.
|
||||
|
||||
|
||||
@@ -56,7 +56,7 @@ describe('defensive-security framing reaches every adversarial dispatch', () =>
|
||||
const army = fs.readFileSync(path.join(ROOT, 'ship', 'sections', 'review-army.md'), 'utf-8').replace(/\s+/g, ' ');
|
||||
const adversarial = fs.readFileSync(path.join(ROOT, 'ship', 'sections', 'adversarial.md'), 'utf-8').replace(/\s+/g, ' ');
|
||||
expect(army).toContain('the defensive framing and the diff with fixtures in summary mode');
|
||||
expect(adversarial).toContain('packet carries the defensive framing, the prompt below, the diff with fixtures in summary mode');
|
||||
expect(adversarial).toContain('packet carries the defensive framing, the native adversarial prompt (`ADVERSARIAL_NATIVE`), the diff with fixtures in summary mode');
|
||||
expect(army).not.toContain(FRAMING_SENTENCE);
|
||||
});
|
||||
|
||||
|
||||
+308
-5
@@ -9,7 +9,8 @@ import * as fs from 'node:fs';
|
||||
import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
import * as P from '../lib/review-prompts';
|
||||
import { shardChunks, SPECIALIST_PACKING } from '../lib/review-packet';
|
||||
import { READ_CHUNK_BYTES, readManifest, shardChunks, SPECIALIST_PACKING } from '../lib/review-packet';
|
||||
import { parseNext } from '../lib/ship-budget';
|
||||
|
||||
const ROOT = path.join(import.meta.dir, '..');
|
||||
const BIN = path.join(ROOT, 'bin', 'gstack-review-packet');
|
||||
@@ -36,8 +37,9 @@ function repo(base: Record<string, string>, feat: Record<string, string>, untrac
|
||||
}
|
||||
const run = (r: Repo, args: string[]) => spawnSync(BIN, args, { cwd: r.dir, encoding: 'utf8', timeout: 120_000, env: { ...process.env, GSTACK_STATE_ROOT: r.state } });
|
||||
const build = (r: Repo, extra: string[] = []) => run(r, ['build', '--base', 'main', '--gate-tagged', '0', '--out-dir', r.out, ...extra]);
|
||||
const packets = (stdout: string) => [...stdout.matchAll(/^PACKET: (\S+) (\S+) effort=\S+ bytes=(\d+) tokens~=\d+ file=(\S+) out=(\S+)$/gm)].map(m => ({ role: m[1]!, name: m[2]!, bytes: Number(m[3]), file: m[4]!, out: m[5]!, text: fs.readFileSync(m[4]!, 'utf8') }));
|
||||
const packets = (stdout: string) => [...stdout.matchAll(/^PACKET: (\S+) (\S+) effort=\S+ bytes=(\d+) tokens~=\d+ budget_role=(\S+) read=(\S+) file=(\S+) out=(\S+)$/gm)].map(m => ({ role: m[1]!, name: m[2]!, bytes: Number(m[3]), budgetRole: m[4]!, read: m[5]!.split(',').map(r => r.split('-').map(Number) as [number, number]), file: m[6]!, out: m[7]!, text: fs.readFileSync(m[6]!, 'utf8') }));
|
||||
const lines = (n: number, tag: string) => Array.from({ length: n }, (_, i) => `export const ${tag}${i} = ${i};`).join('\n') + '\n';
|
||||
const readManifestOf = (dir: string) => readManifest(dir)!.packets;
|
||||
const checklist = (name: string) => fs.readFileSync(path.join(ROOT, 'review', 'specialists', `${name}.md`), 'utf8');
|
||||
|
||||
describe('gstack-review-packet build', () => {
|
||||
@@ -161,7 +163,7 @@ describe('gstack-review-packet build', () => {
|
||||
test('audits: the packet carries the generated section prompt fence with the base filled in', () => {
|
||||
const r = repo({}, { 'src/a.ts': lines(20, 'zeta') });
|
||||
const a = packets(build(r, ['--audit', 'test-coverage', '--generation', 'audit-only', '--passes-used', '1']).stdout)[0]!;
|
||||
expect(a.text).toContain('You are running a ship-workflow test coverage audit. Run `git diff origin/main`');
|
||||
expect(a.text).toContain('You are running a ship-workflow test coverage audit. Run `git diff main`');
|
||||
expect(a.text).toContain('Generation: audit-only; passes used: 1 of 2.');
|
||||
expect(a.text).toContain('+export const zeta19 = 19;');
|
||||
expect(build(r, ['--audit', 'plan-completion']).status).toBe(2);
|
||||
@@ -241,6 +243,214 @@ describe('gstack-review-packet build', () => {
|
||||
expect(a.text).toContain('Generation: audit-only; passes used: 0 of 2.');
|
||||
expect(a.text).toContain(P.AUDIT_READ_ONLY);
|
||||
});
|
||||
|
||||
// Value: protects=--base origin/main names origin/main in the audit prompt; fails_when=the prompt prefixes origin/ to a ref that already has it (git diff origin/origin/main);
|
||||
// why_new=every build test passes --base main with no origin remote; seam=none; no_claim=a base with no origin remote (covered: it stays as given)
|
||||
test('--base origin/main and --base main both put origin/main in the audit prompt, never origin/origin/main', () => {
|
||||
const r = repo({}, { 'src/a.ts': lines(20, 'kappa') });
|
||||
spawnSync('git', ['update-ref', 'refs/remotes/origin/main', 'main'], { cwd: r.dir, timeout: 10_000 });
|
||||
for (const base of ['origin/main', 'main']) {
|
||||
const b = run(r, ['build', '--base', base, '--gate-tagged', '0', '--out-dir', r.out, '--audit', 'test-coverage']);
|
||||
expect(b.status).toBe(0);
|
||||
const a = packets(b.stdout)[0]!;
|
||||
expect(a.text).toContain('Run `git diff origin/main`');
|
||||
expect(a.text).not.toContain('origin/origin/');
|
||||
}
|
||||
});
|
||||
|
||||
// Value: protects=every PACKET line names a role gstack-ship-budget accepts and build prints one ready BUDGET_NEXT; fails_when=a packet or shard name leaks into --next and check exits 2;
|
||||
// why_new=no test joins the packet path to the budget's role contract; seam=none; no_claim=the budget's per-role pricing
|
||||
test('PACKET lines carry a budget_role the budget accepts; BUDGET_NEXT lists one role per packet, shards keep their parent role', () => {
|
||||
const files: Record<string, string> = {};
|
||||
for (let i = 0; i < 30; i++) files[`src/f${i}.ts`] = lines(400, `w${i}_`);
|
||||
const r = repo({}, files);
|
||||
const b = build(r, ['--only', 'testing,adversarial,outside-voice']);
|
||||
expect(b.status).toBe(0);
|
||||
const ps = packets(b.stdout);
|
||||
expect(ps.some(p => p.name.startsWith('testing.1of'))).toBe(true);
|
||||
const want: Record<string, string> = { specialist: 'specialist', 'adversarial-native': 'adversarial-native', 'outside-voice': 'outside-voice' };
|
||||
for (const p of ps) expect(p.budgetRole).toBe(want[p.role]!);
|
||||
const next = /^BUDGET_NEXT: (\S+)$/m.exec(b.stdout)![1]!;
|
||||
expect(next).toBe(`dispatch:${ps.map(p => p.budgetRole).join(',')}`);
|
||||
expect(parseNext(next)).toEqual({ kind: 'dispatch', roles: ps.map(p => p.budgetRole) });
|
||||
const a = build(r, ['--audit', 'test-coverage']);
|
||||
expect(packets(a.stdout)[0]!.budgetRole).toBe('specialist');
|
||||
expect(a.stdout).toMatch(/^BUDGET_NEXT: dispatch:specialist$/m);
|
||||
});
|
||||
|
||||
// Value: protects=a packet is read whole in one turn: its read= ranges cover every line, each under the read tool's measured budget, and it ends with END OF PACKET; fails_when=a range overruns the budget (silently truncated read) or a line falls between ranges;
|
||||
// why_new=no test reads a packet the way a reviewer does; seam=none; no_claim=the host's read budget itself (measured on Capy at ~37.8K chars)
|
||||
test('read= ranges cover the packet line for line, each range under READ_CHUNK_BYTES, ending at END OF PACKET', () => {
|
||||
const files: Record<string, string> = {};
|
||||
for (let i = 0; i < 12; i++) files[`src/g${i}.ts`] = lines(300, `r${i}_`);
|
||||
const r = repo({}, files);
|
||||
const ps = packets(build(r, ['--only', 'testing,maintainability']).stdout);
|
||||
expect(ps.map(p => p.name).sort()).toEqual(['maintainability', 'testing']);
|
||||
expect(ps.every(p => p.read.length > 1)).toBe(true);
|
||||
for (const p of ps) {
|
||||
const all = p.text.split('\n');
|
||||
expect(p.text.trimEnd().split('\n').at(-1)).toBe(`END OF PACKET ${p.name}`);
|
||||
let next = 1;
|
||||
for (const [from, to] of p.read) {
|
||||
expect(from).toBe(next);
|
||||
expect(Buffer.byteLength(all.slice(from - 1, to).join('\n'))).toBeLessThanOrEqual(READ_CHUNK_BYTES);
|
||||
next = to + 1;
|
||||
}
|
||||
expect(next - 1).toBeGreaterThanOrEqual(p.text.trimEnd().split('\n').length);
|
||||
}
|
||||
});
|
||||
|
||||
// Value: protects=large data-format additions (#34's wiki *.json) reach reviewers as a summary (stat, head and tail, top-level keys), so each role gets one packet; fails_when=data files ride the diff whole and every role shards;
|
||||
// why_new=FIXTURE_PATHSPECS misses data files outside test paths; seam=none; no_claim=reviewing the data content itself (testing still reads data under test/)
|
||||
test('a #34-shaped diff (three ~70 KB wiki JSON files and a small code change) gives every role one packet; testing still reads data under test/', () => {
|
||||
const big = (tag: string) => JSON.stringify({ pages: Array.from({ length: 1400 }, (_, i) => ({ id: `${tag}-${i}`, title: `Page ${i}`, body: 'lorem ipsum dolor' })), meta: { tag } }, null, 1) + '\n';
|
||||
const r = repo({}, { 'wiki/a.json': big('a'), 'wiki/b.json': big('b'), 'wiki/c.json': big('c'), 'test/fixtures/d.json': big('d'), 'src/a.ts': lines(60, 'lam'), 'package.json': '{"name":"x"}\n' });
|
||||
const b = build(r, ['--only', 'testing,maintainability,security,adversarial']);
|
||||
expect(b.status).toBe(0);
|
||||
const ps = packets(b.stdout);
|
||||
expect(ps.map(p => p.name).sort()).toEqual(['adversarial', 'maintainability', 'security', 'testing']);
|
||||
for (const p of ps) {
|
||||
expect(p.text).toContain('DATA SUMMARY: wiki/a.json');
|
||||
expect(p.text).toContain('top-level keys: pages, meta');
|
||||
expect(p.text).not.toContain('"id": "a-700"');
|
||||
expect(p.text).toContain('+export const lam59 = 59;');
|
||||
}
|
||||
expect(ps.find(p => p.name === 'testing')!.text).toContain('"id": "d-700"');
|
||||
expect(ps.find(p => p.name === 'maintainability')!.text).not.toContain('"id": "d-700"');
|
||||
});
|
||||
|
||||
// Value: protects=a data summary is built from the diff alone, so no file outside the diff reaches a packet; fails_when=topLevelKeys reads the working tree and follows a symlink that replaced a large CSV;
|
||||
// why_new=the #34 test adds plain files only; seam=none; no_claim=other symlink shapes git shows as one-line diffs
|
||||
test('a large CSV replaced by a symlink to a private file: the summary never reads the link target', () => {
|
||||
const secret = path.join(tmp('review-packet-secret-'), 'id_key');
|
||||
fs.writeFileSync(secret, 'PRIVATE-KEY-MATERIAL-7f3a,never,in,a,packet\n');
|
||||
const csv = 'id,name,score\n' + Array.from({ length: 500 }, (_, i) => `${i},row${i},${i % 7}`).join('\n') + '\n';
|
||||
const r = repo({ 'data/scores.csv': csv }, { 'src/a.ts': lines(5, 'nu') });
|
||||
fs.rmSync(path.join(r.dir, 'data/scores.csv'));
|
||||
fs.symlinkSync(secret, path.join(r.dir, 'data/scores.csv'));
|
||||
const b = build(r, ['--only', 'security,outside-voice']);
|
||||
expect(b.status).toBe(0);
|
||||
const ps = packets(b.stdout);
|
||||
expect(ps.map(p => p.name).sort()).toEqual(['outside-voice', 'security']);
|
||||
for (const p of ps) {
|
||||
expect(p.text).toContain('DATA SUMMARY: data/scores.csv');
|
||||
expect(p.text).not.toContain('PRIVATE-KEY-MATERIAL');
|
||||
}
|
||||
});
|
||||
|
||||
// Value: protects=a partial build after the head moved drops the old run's reviewers and their outputs; fails_when=writeManifest keeps entries from another head, so stale clean results count as coverage of new code;
|
||||
// why_new=partial-build tests stay on one head; seam=none; no_claim=a working-tree edit at the same head beyond the one exercised here
|
||||
test('a partial build keeps other reviewers only at the same head, base and content; otherwise it starts over, and a Red Team build refuses', () => {
|
||||
const r = repo({}, { 'src/a.ts': lines(80, 'mu') });
|
||||
const first = packets(build(r, ['--only', 'testing,maintainability']).stdout);
|
||||
for (const p of first) fs.writeFileSync(p.out, 'NO FINDINGS\n');
|
||||
expect(build(r, ['--only', 'testing']).status).toBe(0);
|
||||
expect(readManifestOf(r.out).map(p => p.reviewer).sort()).toEqual(['maintainability', 'testing']);
|
||||
fs.writeFileSync(path.join(r.dir, 'src/a.ts'), lines(81, 'mu'));
|
||||
fs.writeFileSync(path.join(r.out, 'merged.jsonl'), '');
|
||||
const red = build(r, ['--only', 'red-team', '--findings', path.join(r.out, 'merged.jsonl')]);
|
||||
expect(red.status).toBe(3);
|
||||
expect(red.stdout).toContain('STALE_MANIFEST: content changed');
|
||||
expect(readManifestOf(r.out).map(p => p.reviewer).sort()).toEqual(['maintainability', 'testing']);
|
||||
expect(build(r, ['--only', 'testing']).status).toBe(0);
|
||||
expect(readManifestOf(r.out).map(p => p.reviewer)).toEqual(['testing']);
|
||||
expect(fs.existsSync(first.find(p => p.name === 'maintainability')!.out)).toBe(false);
|
||||
});
|
||||
|
||||
// Value: protects=a Step 7/8 audit past the packet cap stays one packet holding the whole diff, so the parent gets one coverage or plan verdict and one generation allowance; fails_when=audits shard like reviewers (a plan item built in another shard reads NOT DONE, each shard generates tests);
|
||||
// why_new=the sharding test covers reviewers only; seam=none; no_claim=how a child reads a packet past its context (it reads the file in ranges)
|
||||
test('an audit past the cap is never sharded: one packet, every path of the diff', () => {
|
||||
const files: Record<string, string> = {};
|
||||
for (let i = 0; i < 30; i++) files[`src/f${i}.ts`] = lines(400, `v${i}_`);
|
||||
const r = repo({}, files);
|
||||
fs.writeFileSync(path.join(r.state, 'plan.md'), '# Plan\n- [ ] add src/f0.ts\n');
|
||||
for (const extra of [['--audit', 'test-coverage'], ['--audit', 'plan-completion', '--plan-file', path.join(r.state, 'plan.md')]]) {
|
||||
const b = build(r, extra);
|
||||
expect(b.status).toBe(0);
|
||||
const ps = packets(b.stdout);
|
||||
expect(ps.map(p => p.name)).toEqual([extra[1]!]);
|
||||
expect(ps[0]!.bytes).toBeGreaterThan(200_000);
|
||||
expect(ps[0]!.text).not.toContain('This is shard');
|
||||
for (let i = 0; i < 30; i++) expect(ps[0]!.text).toContain(`+export const v${i}_399 = 399;`);
|
||||
}
|
||||
});
|
||||
// Value: protects=a full rebuild deletes the previous head's output files, so collect cannot count a stale review as fresh coverage; fails_when=writePackets/writeManifest stop removing old .out files;
|
||||
// why_new=no test rebuilds over an out dir that holds results; seam=none; no_claim=removal of packets for reviewers absent from the new plan beyond the reviewer rebuilt here
|
||||
test('a rebuild clears earlier reviewer outputs: a stale result never stands as coverage of the new head', () => {
|
||||
const r = repo({}, { 'src/a.ts': lines(80, 'eta') });
|
||||
const first = packets(build(r, ['--only', 'testing']).stdout)[0]!;
|
||||
fs.writeFileSync(first.out, 'NO FINDINGS\n');
|
||||
expect(spawnSync(BIN, ['collect', '--dir', r.out], { encoding: 'utf8', timeout: 60_000 }).status).toBe(0);
|
||||
expect(build(r, ['--only', 'testing']).status).toBe(0);
|
||||
expect(fs.existsSync(first.out)).toBe(false);
|
||||
const c = spawnSync(BIN, ['collect', '--dir', r.out], { encoding: 'utf8', timeout: 60_000 });
|
||||
expect(c.status).toBe(3);
|
||||
expect(c.stdout).toContain('REVIEWER: testing status=missing');
|
||||
});
|
||||
|
||||
// Value: protects=build refuses bad inputs with usage exit 2 instead of writing packets; fails_when=an --audit name, unreadable --plan-file or non-commit --range is accepted;
|
||||
// why_new=only missing fix-check inputs and plan-completion without a plan are tested; seam=none; no_claim=the stderr wording
|
||||
test('usage refusals: an unknown --audit, an unreadable --plan-file and a --range that is not a commit exit 2 with no packet', () => {
|
||||
const r = repo({}, { 'src/a.ts': lines(20, 'theta') });
|
||||
for (const extra of [['--audit', 'bogus'], ['--plan-file', path.join(r.state, 'nope.md')], ['--range', 'not-a-commit']]) {
|
||||
const b = build(r, extra);
|
||||
expect(b.status).toBe(2);
|
||||
expect(b.stdout).not.toContain('PACKET:');
|
||||
}
|
||||
});
|
||||
|
||||
// Value: protects=build without --out-dir and collect without --dir refuse with usage exit 2 and write nothing; fails_when=either guard is dropped and the command defaults a directory (cwd) or crashes;
|
||||
// why_new=every other test passes --out-dir/--dir; seam=none; no_claim=the stderr wording
|
||||
test('build without --out-dir and collect without --dir exit 2 and write nothing', () => {
|
||||
const r = repo({}, { 'src/a.ts': lines(20, 'iota') });
|
||||
const before = spawnSync('git', ['status', '--porcelain', '--untracked-files=all'], { cwd: r.dir, encoding: 'utf8', timeout: 10_000 }).stdout;
|
||||
const b = run(r, ['build', '--base', 'main', '--gate-tagged', '0']);
|
||||
expect(b.status).toBe(2);
|
||||
expect(b.stdout).not.toContain('PACKET:');
|
||||
const c = run(r, ['collect']);
|
||||
expect(c.status).toBe(2);
|
||||
expect(c.stdout).toBe('');
|
||||
expect(spawnSync('git', ['status', '--porcelain', '--untracked-files=all'], { cwd: r.dir, encoding: 'utf8', timeout: 10_000 }).stdout).toBe(before);
|
||||
expect(fs.readdirSync(r.out)).toEqual([]);
|
||||
});
|
||||
|
||||
// Value: protects=the outside-voice packet carries CODEX_BOUNDARY_TEXT, OUTSIDE_CONTRACT and the verbatim-reply instruction; fails_when=the outside-voice role text drops the boundary, contract or reply line;
|
||||
// why_new=only the resolver's boundary literal is compared, never the packet; seam=none; no_claim=that the outside model obeys the boundary
|
||||
test('the outside-voice packet carries the filesystem boundary, the severity contract and the verbatim-reply instruction', () => {
|
||||
const r = repo({}, { 'src/a.ts': lines(80, 'kappa') });
|
||||
const b = build(r, ['--only', 'outside-voice']);
|
||||
expect(b.status).toBe(0);
|
||||
const ov = packets(b.stdout).find(p => p.role === 'outside-voice')!;
|
||||
expect(ov.text).toContain(P.CODEX_BOUNDARY_TEXT);
|
||||
expect(ov.text).toContain(P.OUTSIDE_ADVERSARIAL);
|
||||
expect(ov.text).toContain(P.OUTSIDE_CONTRACT);
|
||||
expect(ov.text).toContain('the parent saves it verbatim to the output path below');
|
||||
expect(ov.text).toContain('+export const kappa79 = 79;');
|
||||
});
|
||||
|
||||
// Value: protects=packets written under an in-repo --out-dir or anything under .gstack/tmp never enter a later packet's diff; fails_when=the insideOut skip is dropped, so a rebuild reviews the previous packets (and their diffs);
|
||||
// why_new=every other test writes --out-dir outside the fixture repo; seam=none; no_claim=a later build into a different out dir, or tracked files under the out dir
|
||||
test('packets under an in-repo --out-dir and files under .gstack/tmp never appear in a later packet\'s diff', () => {
|
||||
const r = repo({}, { 'src/a.ts': lines(80, 'lambda') });
|
||||
fs.mkdirSync(path.join(r.dir, '.gstack/tmp/other'), { recursive: true });
|
||||
fs.writeFileSync(path.join(r.dir, '.gstack/tmp/other/stray.md'), 'STRAY_TMP_MARKER\n');
|
||||
const inRepo = path.join(r.dir, 'review-out');
|
||||
const first = run(r, ['build', '--base', 'main', '--gate-tagged', '0', '--out-dir', inRepo, '--only', 'testing']);
|
||||
expect(first.status).toBe(0);
|
||||
expect(fs.readdirSync(inRepo).some(f => f.endsWith('.packet.md'))).toBe(true);
|
||||
const second = run(r, ['build', '--base', 'main', '--gate-tagged', '0', '--out-dir', inRepo]);
|
||||
expect(second.status).toBe(0);
|
||||
const all = packets(second.stdout);
|
||||
expect(all.length).toBeGreaterThan(0);
|
||||
for (const p of all) {
|
||||
expect(p.text).toContain('+export const lambda79 = 79;');
|
||||
expect(p.text).not.toContain('diff --git a/review-out/');
|
||||
expect(p.text).not.toMatch(/^ review-out\/\S* \|/m);
|
||||
expect(p.text).not.toContain('diff --git a/.gstack/tmp/');
|
||||
expect(p.text).not.toMatch(/^ \.gstack\/tmp\/\S* \|/m);
|
||||
expect(p.text).not.toContain('STRAY_TMP_MARKER');
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('the packet prose and the generated SKILL.md prose are one text', () => {
|
||||
@@ -254,6 +464,7 @@ describe('the packet prose and the generated SKILL.md prose are one text', () =>
|
||||
expect(read('review/sections/adversarial.md')).toContain(P.OUTSIDE_ADVERSARIAL);
|
||||
expect(read('ship/sections/review-army.md')).toContain(P.packetBrief('<file>', '<out>'));
|
||||
expect(read('ship/sections/adversarial.md')).toContain(P.packetBrief('<file>', '<out>'));
|
||||
expect(read('ship/sections/test-coverage.md')).toContain(P.packetBrief('<file>', '<out>'));
|
||||
});
|
||||
test('/ship Step 9 dispatches the batched packet at every depth and logs its checklists; Step 7 skips a no-code audit and writes tests itself', () => {
|
||||
const army = read('ship/sections/review-army.md');
|
||||
@@ -270,6 +481,21 @@ describe('the packet prose and the generated SKILL.md prose are one text', () =>
|
||||
const line = read('scripts/resolvers/outside-voice-steps.ts').split('\n').find(l => l.startsWith('const CODEX_BOUNDARY = '))!;
|
||||
expect(line).toBe(`const CODEX_BOUNDARY = '${P.CODEX_BOUNDARY_TEXT}\\\\n\\\\n';`);
|
||||
});
|
||||
// Value: protects=every gstack-review-packet command the /ship sections tell the parent to run uses only flags the binary documents; fails_when=a flag is renamed in the bin or the prose keeps a retired one (--hunks);
|
||||
// why_new=build tests drive the bin directly, never the commands the prose names; seam=none; no_claim=flag values, argument order and the prose's "plus that plan run's flags" composition
|
||||
test('/ship prose names only gstack-review-packet flags the binary documents', () => {
|
||||
const known = new Set(spawnSync(BIN, ['--help'], { encoding: 'utf8', timeout: 30_000 }).stdout.match(/--[a-z][a-z-]*/g)!);
|
||||
const spans: string[] = [];
|
||||
for (const s of ['review-army', 'adversarial', 'test-coverage', 'plan-completion']) {
|
||||
const text = read(`ship/sections/${s}.md`);
|
||||
spans.push(...(text.match(/`[^`]*gstack-review-packet (?:build|collect)[^`]*`/g) ?? []));
|
||||
spans.push(...(text.match(/build its packet with that plan run's flags plus `[^`]*`/g) ?? []));
|
||||
}
|
||||
expect(spans.length).toBeGreaterThanOrEqual(7);
|
||||
const named = spans.flatMap(s => s.match(/--[a-z][a-z-]*/g) ?? []);
|
||||
expect(named).toContain('--fix-list');
|
||||
expect(named.filter(n => !known.has(n))).toEqual([]);
|
||||
});
|
||||
test('the resolvers import the prompt bodies instead of holding a copy', () => {
|
||||
for (const f of ['scripts/resolvers/review-army.ts', 'scripts/resolvers/outside-voice-steps.ts', 'scripts/resolvers/fix-check.ts', 'scripts/resolvers/defensive-review.ts']) {
|
||||
const src = read(f);
|
||||
@@ -279,10 +505,10 @@ describe('the packet prose and the generated SKILL.md prose are one text', () =>
|
||||
});
|
||||
|
||||
describe('gstack-review-packet collect', () => {
|
||||
type Entry = { role: string; name: string };
|
||||
type Entry = { role: string; name: string; checklists?: string[] };
|
||||
function dir(entries: Entry[], outs: Record<string, string | null>) {
|
||||
const d = tmp('review-packet-collect-');
|
||||
const packets = entries.map(e => ({ ...e, reviewer: e.name, effort: 'full', file: path.join(d, `${e.name}.packet.md`), out: path.join(d, `${e.name}.out`), bytes: 1, checklists: [], fixtures: 'full', shard: null }));
|
||||
const packets = entries.map(e => ({ ...e, reviewer: e.name, effort: 'full', file: path.join(d, `${e.name}.packet.md`), out: path.join(d, `${e.name}.out`), bytes: 1, checklists: e.checklists ?? [], fixtures: 'full', shard: null }));
|
||||
fs.writeFileSync(path.join(d, 'manifest.json'), JSON.stringify({ version: 1, head: 'h', diff_base: 'b', fingerprint: 'none', packets }));
|
||||
for (const [n, text] of Object.entries(outs)) if (text !== null) fs.writeFileSync(path.join(d, `${n}.out`), text);
|
||||
return d;
|
||||
@@ -370,4 +596,81 @@ describe('gstack-review-packet collect', () => {
|
||||
expect(r.stdout).toContain('REVIEWER: adversarial status=findings crit=0 info=1');
|
||||
expect(r.stdout).toContain('RECOMMENDATION: adversarial: Ship as-is because the race needs two writers');
|
||||
});
|
||||
// Value: protects=collect treats a missing or non-JSON Step 7/8 audit output as incomplete coverage (exit 3); fails_when=the AUDIT status check or its push into results is dropped or inverted;
|
||||
// why_new=collect tests above use reviewer roles only; seam=none; no_claim=the audit's JSON schema (only that its last line parses as an object)
|
||||
test('an audit output whose last line is a JSON object is present; missing or prose output is incomplete coverage, exit 3', () => {
|
||||
const audit = [{ role: 'specialist', name: 'testing' }, { role: 'audit', name: 'test-coverage' }];
|
||||
const ok = collect(dir(audit, { testing: 'NO FINDINGS', 'test-coverage': 'diagram\n{"coverage_pct":50,"gaps":1}\n' }));
|
||||
expect(ok.status).toBe(0);
|
||||
expect(ok.stdout).toMatch(/^AUDIT: test-coverage status=present /m);
|
||||
expect(ok.stdout).toMatch(/^COLLECT: reviewers=1 /m);
|
||||
const missing = collect(dir(audit, { testing: 'NO FINDINGS', 'test-coverage': null }));
|
||||
expect(missing.status).toBe(3);
|
||||
expect(missing.stdout).toMatch(/test-coverage=missing.*\(REVIEW_COVERAGE_INCOMPLETE\)$/m);
|
||||
const prose = collect(dir(audit, { testing: 'NO FINDINGS', 'test-coverage': 'All paths are covered.\n' }));
|
||||
expect(prose.status).toBe(3);
|
||||
expect(prose.stdout).toMatch(/^AUDIT: test-coverage status=invalid /m);
|
||||
});
|
||||
|
||||
// Value: protects=a deferred Red Team is reported pending and neither read nor counted as missing coverage; fails_when=collect reads deferred entries' (empty) out path or drops the pending line;
|
||||
// why_new=the build test asserts only PACKET_DEFERRED, never collect's handling; seam=none; no_claim=that the Red Team later runs (parent workflow)
|
||||
test('a deferred Red Team is listed pending with its build hint and does not make coverage incomplete', () => {
|
||||
const r = collect(dir([...specialists, { role: 'red-team', name: 'red-team', deferred: true } as Entry], { testing: 'NO FINDINGS', security: 'NO FINDINGS' }));
|
||||
expect(r.status).toBe(0);
|
||||
expect(r.stdout).toMatch(/^REVIEWER: red-team status=pending \(build its packet with --findings \S+merged\.jsonl/m);
|
||||
expect(r.stdout).toMatch(/^COLLECT: reviewers=3 clean=2 findings=0 missing=0 invalid=0 .* pending=1 /m);
|
||||
});
|
||||
|
||||
// Value: protects=per-role output validation: a matrix record alone is clean, a specialist line without its required fields is invalid, a refused outside voice is invalid;
|
||||
// fails_when=validFinding stops requiring confidence/category/specialist, matrix records stop counting, or a refusal reads clean; why_new=no collect test covers these branches;
|
||||
// seam=none; no_claim=gstack-state-matrix check semantics (matrix content is not validated here)
|
||||
test.each([
|
||||
['state-machines matrix record alone is clean', 'specialist', '{"matrix":{"states":["a","b"]}}', 0, 'clean', ['state-machines']],
|
||||
['specialist line missing confidence/category/specialist is invalid', 'specialist', '{"severity":"CRITICAL","path":"src/a.ts","summary":"s"}', 3, 'invalid', []],
|
||||
['outside voice refusal is invalid', 'outside-voice', 'I cannot review this code.', 3, 'invalid', []],
|
||||
])('%s', (_label, role, text, code, status, checklists) => {
|
||||
const r = collect(dir([{ role, name: 'r1', checklists }], { r1: text }));
|
||||
expect(r.status).toBe(code);
|
||||
expect(r.stdout).toContain(`REVIEWER: r1 status=${status}`);
|
||||
});
|
||||
|
||||
// Value: protects=collect on a dir without a readable manifest exits 1 and never reports clean; fails_when=the missing-manifest guard is dropped and an empty plan reads as complete coverage (exit 0);
|
||||
// why_new=every collect test writes a valid manifest; seam=none; no_claim=the stderr wording
|
||||
test('collect on a dir with no readable manifest exits 1 and never reports clean', () => {
|
||||
const empty = tmp('review-packet-collect-');
|
||||
const corrupt = tmp('review-packet-collect-');
|
||||
fs.writeFileSync(path.join(corrupt, 'manifest.json'), '{not json');
|
||||
for (const d of [empty, corrupt]) {
|
||||
const r = collect(d);
|
||||
expect(r.status).toBe(1);
|
||||
expect(r.stdout).not.toContain('COLLECT:');
|
||||
expect(r.stderr).toContain('manifest');
|
||||
}
|
||||
});
|
||||
|
||||
// Value: protects=a matrix record counts only for a reviewer that applies the state-machines checklist, and only as an object; fails_when={"matrix":null} from security reads clean;
|
||||
// why_new=the matrix shortcut was never tested outside state-machines; seam=none; no_claim=gstack-state-matrix check semantics
|
||||
test('{"matrix":null} or a matrix from a non-state-machines reviewer is not a clean review', () => {
|
||||
for (const [checklists, text] of [[['security'], '{"matrix":{"states":["a"]}}'], [['state-machines'], '{"matrix":null}']] as [string[], string][]) {
|
||||
const r = collect(dir([{ role: 'specialist', name: 'r1', checklists }], { r1: text }));
|
||||
expect(r.status).toBe(3);
|
||||
expect(r.stdout).toContain('REVIEWER: r1 status=invalid');
|
||||
}
|
||||
});
|
||||
|
||||
// Value: protects=a CRITICAL line that fails validation makes the reviewer invalid (coverage incomplete) instead of vanishing behind a valid peer line; fails_when=the bad CRITICAL is only counted in invalid_lines and collect exits 0;
|
||||
// why_new=invalid-line tests only use non-JSON text; seam=none; no_claim=the validity rules themselves
|
||||
test('a CRITICAL line missing required fields next to a valid finding makes the reviewer invalid, exit 3', () => {
|
||||
const r = collect(dir([{ role: 'specialist', name: 'testing' }], { testing: [finding({}), '{"severity":"CRITICAL","path":"src/a.ts","summary":"real blocker"}'].join('\n') }));
|
||||
expect(r.status).toBe(3);
|
||||
expect(r.stdout).toContain('REVIEWER: testing status=invalid');
|
||||
});
|
||||
|
||||
// Value: protects=a CRITICAL in another case or on an unparseable line still makes the reviewer invalid; fails_when=only an exact, parseable severity "CRITICAL" trips the guard;
|
||||
// why_new=the guard test above uses a parseable exact-case line; seam=none; no_claim=prose findings, which the contract already rejects
|
||||
test.each(['{"severity":"Critical","confidence":8,"path":"a.ts","category":"x","summary":"s","specialist":"testing"}', '{"severity":"CRITICAL","path":"a.ts","summary":"trailing comma",}', '[CRITICAL] a.ts: prose blocker'])('a case-variant or unparseable CRITICAL line next to a valid finding makes the reviewer invalid: %s', bad => {
|
||||
const r = collect(dir([{ role: 'specialist', name: 'testing' }], { testing: [finding({}), bad].join('\n') }));
|
||||
expect(r.status).toBe(3);
|
||||
expect(r.stdout).toContain('REVIEWER: testing status=invalid');
|
||||
});
|
||||
});
|
||||
@@ -1,4 +1,4 @@
|
||||
{
|
||||
"bytes": -2687,
|
||||
"why": "GSTA-167 reviewer packets: Step 9.1 dispatches each reviewer by one packet file (gstack-review-packet build/collect) in place of the inlined specialist prompt; Red Team, fix-check, Step 11 and the Step 7/8 audits read packets too. PR 3b: a pass's specialists are one batched packet per shard with a COVERED line, Step 7 skips the audit on a diff with no code path and its packet is read-only, the gate scan prints the summary and hunk index only"
|
||||
"bytes": -986,
|
||||
"why": "GSTA-167 reviewer packets: Step 9.1 dispatches each reviewer by one packet file (gstack-review-packet build/collect) in place of the inlined specialist prompt; Red Team, fix-check, Step 11 and the Step 7/8 audits read packets too; replay fixes: read= ranges and END OF PACKET in the reviewer brief, BUDGET_NEXT handoff, outside-voice left out of --only when nothing runs it. PR 3b: a pass's specialists are one batched packet per shard with a COVERED line, Step 7 skips the audit on a diff with no code path and its packet is read-only, the gate scan prints the summary and hunk index only"
|
||||
}
|
||||
Reference in new issue
Block a user