v1.91.330.0 fix(review): packet reviewers read callers, callees and tests beyond the hunks (GSTA-167 hotfix) (#3636)

* fix(ship): reviewers confirm suspected defects outside the hunks: callers, callees, tests, contracts; at most 8 reads in 2 turns (GSTA-167)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* chore: CHANGELOG entry for the reviewer read rule (GSTA-167)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

---------

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
Garry Tanandcapy-ai[bot] authored and GitHub committed 2026-10-30 05:31:51 +00:00
1 parent de53aef03f
commit ce3a093a64
13 files changed
+57 -13

No files matched your search

+12
View File
@@ -1,5 +1,17 @@
# Changelog
## [1.91.330.0] - 2026-10-30
**/ship reviewers now check a suspected defect against the code it depends on before they report it or let it go.** A reviewer packet used to allow at most two more ranges of up to 200 lines, from files in the diff, in one turn. In GSTA-167's no-loss replay, 7 recorded blockers needed context outside those ranges, and no reviewer found them. Each reviewer now confirms or rules out a suspected defect by reading the callers and callees of changed functions, the tests that cover them, and the config and contracts they read. It uses grep and the read tool, at most 8 reads in at most 2 turns of parallel calls. The coverage audit keeps its own budget.
### Changed
- **Reviewer read rule** (`lib/review-prompts.ts`): `READ_BUDGET`, which every specialist, Red Team, adversarial, outside-voice and fix-check packet carries, and the packet brief (`packetBrief`), which now allows two read turns before the write.
- `docs/review-policy.md` (reviewer packets), `capy/skill/gstack/SKILL.md` (/ship reviewers on Capy) and the generated ship sections (review-army, adversarial, test-coverage) say the same.
### Tests
- `test/review-packet.test.ts`: the batched specialist, Red Team, adversarial and fix-check packets carry the rule; the coverage audit keeps its own budget; the rule names callers, callees, tests and contracts with its read and turn bound; the Capy skill and the policy match it.
- `test/size-ledger/ship/gsta-167-review-reads.json`: 108 bytes for the longer brief, rendered three times in the ship sections.
## [1.91.329.0] - 2026-10-30
**A selected pregate now runs the whole-tree tripwires and ratchets on every code branch, so they fail before review instead of in CI.** #3592 and #3593 passed their selected gate and then failed CI on exactly these scanners, because no touched file maps to a test that reads the whole tree.
+1 -1
View File
@@ -1 +1 @@
1.91.329.0
1.91.330.0
+1 -1
View File
@@ -1,4 +1,4 @@
# gstack digest v1.91.329.0 — regenerate/re-copy after upgrading gstack
# gstack digest v1.91.330.0 — regenerate/re-copy after upgrading gstack
Behavioral rules from gstack (https://github.com/garrytan/gstack), compressed
for agent hosts without a full skill install. The full skills add workflows,
+1 -1
View File
@@ -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 (`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.
/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 at most four turns: the parallel packet reads, up to two turns that confirm or rule out each suspected defect from what it depends on outside the hunks (callers, callees, covering tests, the config and contracts they read; grep and the read tool, at most 8 reads, batched in parallel), 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
+8 -3
View File
@@ -349,8 +349,13 @@ writes one packet per `DISPATCH:` line holding:
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 2 more named ranges (200 lines at most) of files in the diff,
in one turn, no repository search, no other packets, no transcript;
- a read rule: before reporting, the reviewer confirms or rules out each suspected defect
from what it depends on outside the hunks (the callers and callees of changed functions,
the tests that cover them, the config and contracts they read), with grep and the read
tool, at most 8 reads in at most 2 turns of parallel calls; no other packets, no
transcript (GSTA-167: under a budget of 2 ranges around the hunks, 7 recorded blockers
that needed context outside them were found by no reviewer); an audit keeps its own
budget (files in the diff, their tests and the files the plan names);
- 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:
@@ -358,7 +363,7 @@ audits reserve as `specialist`, shards as their parent role) and `read=`, line r
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
and finishes in at most four turns. `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
+2 -2
View File
@@ -74,7 +74,7 @@ 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 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.';
export const READ_BUDGET = 'The packet is your starting context: the diff, checklists and contract. Before reporting, confirm or rule out each suspected defect by reading what it depends on outside the hunks: the callers and callees of changed functions, the tests that cover them, and the config/contracts they read. Use grep and the read tool freely for that, batching reads into parallel calls; at most 8 reads, in at most 2 turns. Never stop at the hunk when the defect would show elsewhere. Do not rerun git diff (the diff is below); no other packets, no transcript, no web.';
/** The native adversarial pass writes JSONL like the specialists, plus a classification and its Recommendation line. */
export const ADVERSARIAL_CONTRACT = 'Write each finding as one JSON object per line with the specialist fields (`"specialist":"adversarial"`) plus `"classification":"FIXABLE|INVESTIGATE"`, rated against the same floor (CRITICAL only for a test that would fail, a production break, or a security or data-loss defect); `NO FINDINGS` when nothing would fail. End with the one `Recommendation:` line either way.';
@@ -84,5 +84,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 `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.`;
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. Turns 2 and 3, only if needed: the reads its Read budget allows, each turn's calls in parallel. Last turn: write your result to ${out} in one write and reply with only the DONE line it names.`;
}
+1 -1
View File
@@ -1,6 +1,6 @@
{
"name": "gstack",
"version": "1.91.329",
"version": "1.91.330",
"description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.",
"license": "MIT",
"type": "module",
+1 -1
View File
@@ -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. 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.
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. Turns 2 and 3, only if needed: the reads its Read budget allows, each turn's calls 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.
+1 -1
View File
@@ -379,7 +379,7 @@ 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 "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,
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. Turns 2 and 3, only if needed: the reads its Read budget allows, each turn's calls 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`).**
+1 -1
View File
@@ -19,7 +19,7 @@ run `~/.claude/skills/gstack/bin/gstack-review-packet build --audit test-coverag
--out-dir .gstack/tmp/review-packets-audit --context .gstack/tmp/audit-context.txt
--generation <allowed|audit-only> --passes-used <N>` (the packet holds the prompt below and
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."
"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. Turns 2 and 3, only if needed: the reads its Read budget allows, each turn's calls 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.
**Generation allowance:** Maximum 2 generation passes total per invocation.
+1 -1
View File
@@ -17,7 +17,7 @@ run `~/.claude/skills/gstack/bin/gstack-review-packet build --audit test-coverag
--out-dir .gstack/tmp/review-packets-audit --context .gstack/tmp/audit-context.txt
--generation <allowed|audit-only> --passes-used <N>` (the packet holds the prompt below and
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."
"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. Turns 2 and 3, only if needed: the reads its Read budget allows, each turn's calls 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.
**Generation allowance:** Maximum 2 generation passes total per invocation.
+23
View File
@@ -59,6 +59,7 @@ describe('gstack-review-packet build', () => {
for (const p of all) expect(p.text).not.toContain('UNRELATED_MARKER');
expect(b.stdout).toMatch(/^PACKETS: n=\d+ total_bytes=\d+ fingerprint=\S+$/m);
expect(all.find(p => p.name === 'adversarial')!.text).toContain(P.ADVERSARIAL_NATIVE);
expect(all.find(p => p.name === 'adversarial')!.text).toContain(P.READ_BUDGET);
});
test('the defensive framing reaches the security and adversarial packets with fixtures as a stat; testing reads fixtures in full', () => {
@@ -87,6 +88,7 @@ describe('gstack-review-packet build', () => {
expect(batched.text).toContain(checklist(c));
}
expect(batched.text).toContain('Tag each finding with the checklist that found it');
expect(batched.text).toContain(P.READ_BUDGET);
});
test('exclusions are honored: maintainability leaves out docs and tests; testing keeps tests; untracked files are in', () => {
@@ -139,6 +141,7 @@ describe('gstack-review-packet build', () => {
expect(red.text).toContain(P.DEFENSIVE_REVIEW_FRAMING);
expect(red.text).toContain(P.RED_TEAM_OUTPUT);
expect(red.text).toContain(checklist('red-team'));
expect(red.text).toContain(P.READ_BUDGET);
expect(red.text).not.toContain('+PAYLOAD_MARKER');
expect(red.text).toContain('test/fixture.test.ts');
const manifest = JSON.parse(fs.readFileSync(path.join(r.out, 'manifest.json'), 'utf8'));
@@ -155,6 +158,7 @@ describe('gstack-review-packet build', () => {
const b = build(r, [...flags, '--fix-list', w('fix.txt', 'FIX_LIST_MARKER'), '--tests', w('tests.txt', '12 pass')]);
const fc = packets(b.stdout).find(p => p.role === 'fix-check')!;
expect(fc.text).toContain(P.FIX_CHECK_INSTRUCTIONS);
expect(fc.text).toContain(P.READ_BUDGET);
expect(fc.text).toContain('FIX_LIST_MARKER');
expect(fc.text).toContain('+export const FIXED = 1;');
expect(fc.text).not.toContain('+export const eps0 = 0;');
@@ -166,6 +170,8 @@ describe('gstack-review-packet build', () => {
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(a.text).not.toContain(P.READ_BUDGET);
expect(a.text).toContain('Read only files in the diff, their tests and the files the plan names');
expect(build(r, ['--audit', 'plan-completion']).status).toBe(2);
});
@@ -391,6 +397,23 @@ describe('the packet prose and the generated SKILL.md prose are one text', () =>
expect(read('ship/sections/adversarial.md')).toContain(P.packetBrief('<file>', '<out>'));
expect(read('ship/sections/test-coverage.md')).toContain(P.packetBrief('<file>', '<out>'));
});
// Value: protects=every reviewer confirms or rules out a suspected defect from what it depends on outside the hunks (callers, callees, covering tests, config and contracts), in bounded batched reads, and the Capy reviewer paragraph and policy say the same; fails_when=the rule shrinks back to a fixed count of ranges around the hunks (GSTA-167 replay: 7 blockers needing outside context found by no arm) or loses its read and turn bound;
// why_new=the build tests only hold the packet equal to READ_BUDGET, never what it allows; seam=none; no_claim=that a model follows the rule
test('the read rule sends reviewers past the hunks to callers, callees, tests and contracts, bounded and batched', () => {
for (const clause of ['confirm or rule out each suspected defect', 'the callers and callees of changed functions, the tests that cover them, and the config/contracts they read',
'Use grep and the read tool freely', 'batching reads into parallel calls; at most 8 reads, in at most 2 turns', 'Never stop at the hunk when the defect would show elsewhere']) expect(P.READ_BUDGET).toContain(clause);
expect(P.READ_BUDGET).not.toContain('at most 2 more');
const brief = P.packetBrief('<file>', '<out>');
expect(brief).not.toContain('2 more ranges');
expect(brief).toContain('Turns 2 and 3, only if needed: the reads its Read budget allows, each turn\'s calls in parallel.');
const capy = read('capy/skill/gstack/SKILL.md').replace(/\s+/g, ' ');
expect(capy).not.toContain('one turn of extra ranges');
expect(capy).toContain('up to two turns that confirm or rule out each suspected defect from what it depends on outside the hunks');
expect(capy).toContain('at most 8 reads');
const policy = read('docs/review-policy.md').replace(/\s+/g, ' ');
expect(policy).not.toContain('at most 2 more named ranges');
expect(policy).toContain('at most 8 reads in at most 2 turns');
});
test('the outside voice boundary the safety-rule registry pins in the resolver is the packet\'s boundary', () => {
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';`);
@@ -0,0 +1,4 @@
{
"bytes": 108,
"why": "GSTA-167: the reviewer packet brief allows up to two batched read turns past the hunks (rendered three times in ship sections)"
}