diff --git a/bin/gstack-review-packet b/bin/gstack-review-packet index b80d7aae1..93e60ccf2 100755 --- a/bin/gstack-review-packet +++ b/bin/gstack-review-packet @@ -47,8 +47,8 @@ ${PLAN_FLAGS_HELP} build prints PACKET: effort= bytes= tokens~= budget_role= read= file= out= per packet (a reviewer past ${PACKET_CAP_BYTES} bytes is sharded by path as .1ofK, never truncated; read= are the line ranges, each under ${READ_CHUNK_BYTES} bytes, a reviewer reads in parallel), PACKED: specialist batched checklists= shards= -per batched reviewer, PACKETS: n= total_bytes= fingerprint= and BUDGET_NEXT: dispatch: for gstack-ship-budget check --next. collect exits 3 with REVIEW_COVERAGE_INCOMPLETE when any packet's output is +per batched reviewer, PACKETS: n= total_bytes= fingerprint= and BUDGET_NEXT: dispatch: for gstack-ship-budget check --next (the bytes price each reader from its packet). 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()}`; @@ -139,7 +139,7 @@ function build(): number { 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(',')}`); + if (built.length) out.push(`BUDGET_NEXT: dispatch:${built.map(e => `${budgetRole(e.role)}@${e.bytes}`).join(',')}`); process.stdout.write(out.join('\n') + '\n'); return EXIT.ok; } diff --git a/docs/review-policy.md b/docs/review-policy.md index 97a0956d5..cb1fc5f29 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -363,8 +363,8 @@ 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:` is the value for -`gstack-ship-budget check --next`. A partial build (`--only`) keeps other reviewers' +and finishes in about three calls. `BUDGET_NEXT: dispatch:` is the value for +`gstack-ship-budget check --next`; the bytes price each reader from its packet. 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. diff --git a/lib/review-collect.ts b/lib/review-collect.ts index 07f2031ac..22079af9d 100644 --- a/lib/review-collect.ts +++ b/lib/review-collect.ts @@ -32,6 +32,12 @@ function git(root: string, args: string[]): { ok: boolean; out: string; err: str return { ok: r.status === 0, out: r.stdout ?? '', err: (r.stderr ?? '').trim() }; } const scratch = (p: string) => p.startsWith('.gstack/tmp/'); +/** + * gstack's own run artifacts in the worktree: any untracked file under a .gstack/ + * subdirectory (QA reports in .gstack/qa-reports/, review scratch in .gstack/tmp/, + * run directories). The committed .gstack/*.json configuration stays reviewable. + */ +export const isRunArtifact = (p: string) => p.startsWith('.gstack/') && p.indexOf('/', '.gstack/'.length) > 0; const oneLine = (s: string) => s.replace(/[\0-\x1f]+/g, ' ').trim().slice(0, 200); const unavailable = (input: string, detail: string): RegistryInfo => ({ sha: null, outputs: [], error: { code: 'registry_unavailable', detail: oneLine(detail), input, repair: 'fetch the base branch (git fetch origin ) so its commit and registry resolve' } }); @@ -128,7 +134,7 @@ export function collectLive(root: string, base: string): Collected { const diff = readDiff(root, [mergeBase], errors); const others = git(root, ['ls-files', '--others', '--exclude-standard', '-z']); if (!others.ok) errors.push({ code: 'numstat_failed', detail: oneLine(others.err || 'git ls-files failed'), input: 'git ls-files --others --exclude-standard', repair: 'run the command by hand and fix what git reports' }); - const untracked = others.out.split('\0').filter(p => p && !scratch(p)).sort(); + const untracked = others.out.split('\0').filter(p => p && !isRunArtifact(p)).sort(); for (const p of untracked) { let buf: Buffer; try { buf = fs.readFileSync(path.join(root, p)); } catch { diff.perPath.push({ path: p, lines: 0, binary: true }); continue; } diff --git a/lib/review-packet.ts b/lib/review-packet.ts index 5d1137fa7..c0363cbb1 100644 --- a/lib/review-packet.ts +++ b/lib/review-packet.ts @@ -14,6 +14,7 @@ import { spawnSync } from 'node:child_process'; import { createHash } from 'node:crypto'; import * as fs from 'node:fs'; import * as path from 'node:path'; +import { isRunArtifact } from './review-collect'; import { classifyPath, type Dispatch, type Role } from './review-depth'; import * as P from './review-prompts'; @@ -273,7 +274,7 @@ const diffCmd = (base: string, exclude: readonly string[], summary: boolean) => 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); +const insideOut = (ctx: BuildContext) => (p: string) => isRunArtifact(p) || 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[] { return [`## Context\n\nStack: ${stack}. Test framework: ${testFw}. Diff base: \`${ctx.diffBase}\`; head \`${ctx.head}\`.${learnings === null ? '' : ` Past learnings: ${learnings || 'none'}`}`, '', diff --git a/test/review-packet.test.ts b/test/review-packet.test.ts index 5b41c5df0..538547a78 100644 --- a/test/review-packet.test.ts +++ b/test/review-packet.test.ts @@ -271,11 +271,11 @@ describe('gstack-review-packet build', () => { const want: Record = { 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) }); + expect(next).toBe(`dispatch:${ps.map(p => `${p.budgetRole}@${p.bytes}`).join(',')}`); + expect(parseNext(next)).toEqual({ kind: 'dispatch', roles: ps.map(p => p.budgetRole), bytes: ps.map(p => p.bytes) }); const a = build(r, ['--audit', 'test-coverage']); expect(packets(a.stdout)[0]!.budgetRole).toBe('specialist'); - expect(a.stdout).toMatch(/^BUDGET_NEXT: dispatch:specialist$/m); + expect(a.stdout).toMatch(new RegExp(`^BUDGET_NEXT: dispatch:specialist@${packets(a.stdout)[0]!.bytes}$`, '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; @@ -357,6 +357,30 @@ describe('gstack-review-packet build', () => { expect(fs.existsSync(first.find(p => p.name === 'maintainability')!.out)).toBe(false); }); + // Value: protects=gstack's own run artifacts (a QA report under .gstack/qa-reports/, a run directory under .gstack/) never enter a packet or the content key, so the Red Team build after QA is not refused; fails_when=the untracked filter drops only .gstack/tmp/ (GSTA-167 replay 2 on gstack-evals #36: QA wrote its report into the worktree and the Red Team build hit STALE_MANIFEST); + // why_new=the stale test edits source, never a run artifact; seam=none; no_claim=artifacts gstack writes outside .gstack/ + test('a QA report or run directory written under .gstack/ after the specialists build neither enters a packet nor makes the Red Team build stale', () => { + const r = repo({}, { 'src/a.ts': lines(80, 'nu') }); + const first = packets(build(r, ['--only', 'testing']).stdout); + for (const p of first) fs.writeFileSync(p.out, 'NO FINDINGS\n'); + for (const [rel, text] of [['.gstack/qa-reports/qa-report-app-2026-10-29.md', 'QA_REPORT_MARKER\n'], ['.gstack/ship-runs/run-1/notes.md', 'RUN_DIR_MARKER\n']]) { + fs.mkdirSync(path.dirname(path.join(r.dir, rel)), { recursive: true }); + fs.writeFileSync(path.join(r.dir, rel), text); + } + fs.writeFileSync(path.join(r.out, 'merged.jsonl'), ''); + const red = build(r, ['--only', 'red-team', '--findings', path.join(r.out, 'merged.jsonl')]); + expect(red.stdout).not.toContain('STALE_MANIFEST'); + expect(red.status, red.stdout + red.stderr).toBe(0); + for (const p of packets(red.stdout)) { + expect(p.text).not.toContain('QA_REPORT_MARKER'); + expect(p.text).not.toContain('RUN_DIR_MARKER'); + } + fs.writeFileSync(path.join(r.dir, 'src/new.ts'), 'export const UNTRACKED_SOURCE = 1;\n'); + const stale = build(r, ['--only', 'red-team', '--findings', path.join(r.out, 'merged.jsonl')]); + expect(stale.status).toBe(3); + expect(stale.stdout).toContain('STALE_MANIFEST: content changed'); + }); + // 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', () => {