mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-03 15:27:56 +01:00
v1.91.379.0 fix: review readers work in the checkout /ship ships (GST-482) (#3701)
* fix: review readers work in the checkout /ship ships (GST-482) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix: pre-landing review fixes (REVIEWED_TREE parsing, outside-voice findings kept, unreadable checkout refused) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs: outside-voice tree failure wording (fix-check) 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
1 parent
3bfe5667fb
commit
2d1b55d762
13 files changed
+386
-37
No files matched your search
@@ -1,5 +1,18 @@
|
||||
# Changelog
|
||||
|
||||
## [1.91.379.0] - 2026-10-31
|
||||
|
||||
**/ship's review readers now review the checkout /ship ships, even when it runs from a worktree on a shared machine.** Readers start as subagents on `machine: "shared"`, so each one opens in the machine's working directory. Their briefs never named a folder, so a /ship run from a worktree had its readers review whatever branch the primary checkout held. GBrain's crew saw a specialist return four findings in files outside the PR (GBRA-218, GST-482). A clean review of the wrong tree would have looked the same as a clean review of the right one.
|
||||
|
||||
### Fixed
|
||||
- **Every reader's brief ends with a workspace paragraph** naming the checkout under review (the toplevel of the directory the bins run in). It tells the reader to start each command with `cd <checkout> && `, to give every file tool an absolute path under the checkout, and to make the first line of its final reply `REVIEWED_TREE: <HEAD^{tree}> <toplevel>`, measured there. The paragraph comes after the section text, so the rest of the brief is the same bytes as before. It covers the specialists, the batched specialist, the Red Team, the targeted reader, fix-check, Step 11's native and outside voices, and audit readers passed as extras (Steps 7, 8 and 14.5, and Step 9.2.1's QA reader). An audit reader's brief is now written to `briefs/` as its prompt file plus the paragraph, and its prompt file is not changed.
|
||||
- **`gstack-review-dispatch collect` checks that line against the fingerprint recorded when the brief was rendered.** A missing line, or one naming another checkout, prints `COLLECT_WRONG_TREE`; a line from the right checkout at a later commit prints `COLLECT_TREE_MOVED`. Either way it exits 1, merges nothing, and `verify` reports the brief `NOT_COLLECTED`. With `--manifest`, the reader becomes `failed`, so a rerun stops on it instead of collecting the same reply again. An outside voice whose reply fails the check and holds no finding is recorded as unavailable coverage, never clean; one with findings is refused like any other reader, so none is lost. An audit reader gets the same check when the pass marks it collected, and that now needs `--output`.
|
||||
- **Unavailable outside-voice output never counts as a review in `verify`**, even when its text carries a `Recommendation:` line.
|
||||
|
||||
### Tests
|
||||
- `test/review-pass.test.ts`: a primary checkout and a worktree on another branch, with readers starting in the primary checkout. A reader that reviews the primary checkout, or names none, is refused and nothing it said is merged. One that follows its brief passes, and every brief names the worktree. An audit reader from the wrong checkout is refused. A rerun stops on the failed reader with one output event. A worktree at the same commit is told apart by its toplevel. A commit between render and reply is `COLLECT_TREE_MOVED`. A wrong-tree outside voice with no finding is `UNAVAILABLE`. All eight fail on the previous dispatch code.
|
||||
- `test/review-dispatch.test.ts`: every role's brief is the old render byte for byte, followed by the paragraph. A list-item, heading, bold or backticked `REVIEWED_TREE` line counts, the path keeps its own characters, and the first line wins over one quoted later. An outside voice with findings and no line is refused. A render outside a readable checkout refuses, and a ledger rendered before this change collects as before.
|
||||
|
||||
## [1.91.378.0] - 2026-10-31
|
||||
|
||||
**The CSO runtime image workflow passes again after Docker moved its `scout-sbom-indexer:1` tag.** All 10 `validate-native` jobs in Stage CSO Runtime Images were failing with `SOURCE_TAG_MOVED: docker.io/docker/scout-sbom-indexer:1` on every PR that touched CSO paths. Docker pushed release 1.29.0 to the floating `:1` tag on 2026-10-29. The reviewed index `sha256:4b67f29e…` is Docker's 1.24.0 release, and the `1.24.0` tag still points at it. So the SBOM generator source now names `:1.24.0`. The pinned digests and the build-inputs revision are byte-identical, no requalification trigger changes, and the 10 qualified runtimes stay current.
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
# gstack digest v1.91.378.0 — regenerate/re-copy after upgrading gstack
|
||||
# gstack digest v1.91.379.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,
|
||||
|
||||
@@ -19,7 +19,7 @@ import { spawnSync } from 'node:child_process';
|
||||
import * as fs from 'node:fs';
|
||||
import * as path from 'node:path';
|
||||
import { EXIT, exitTableHelp } from '../lib/headless-artifacts';
|
||||
import { BRIEF_ROLES, BriefError, briefFile, promptHash, renderBrief, sectionTexts, type BriefRole, type Slots } from '../lib/review-brief';
|
||||
import { BRIEF_ROLES, BriefError, briefFile, checkReviewedTree, checkoutRoot, checkoutTree, promptHash, renderBrief, sectionTexts, withWorkspace, type BriefRole, type Slots, type TreeCheck } from '../lib/review-brief';
|
||||
import { cleanMarkers, collect, mayBeUnavailable, parseFindings, readJsonl, readLedger, readerKey, recordJudgment, recordSent, appendEvent, verify, type JudgedLine, type MergedLine, type VerifyRow } from '../lib/review-ledger';
|
||||
import { buildManifest, chunkCount, chunkOf, manifestPath, markReader, readManifest, renderStage, writeManifest, STAGES, type Extra, type Manifest, type PassReader, type ReaderState, type Stage } from '../lib/review-pass';
|
||||
import type { Dispatch } from '../lib/review-depth';
|
||||
@@ -37,15 +37,19 @@ const HELP = `gstack-review-dispatch — verbatim reviewer briefs and loss-free
|
||||
--fix-list <f> --hunks <f> --tests <f> [--callers <f>] (fix-check)
|
||||
--classes <money,gate,auth,secret> (targeted: the plan's TARGETED classes; the brief is review/specialists/targeted.md;
|
||||
--diff-base must be the merge base with --base, default origin/HEAD's branch, on every pass)
|
||||
writes <out>/<role>[-<name>].prompt.md, records it in the ledger (default <out>/review-ledger.jsonl) and prints
|
||||
writes <out>/<role>[-<name>].prompt.md, the brief followed by the workspace paragraph naming the toplevel of the
|
||||
checkout it runs in (the one its diff and merge base come from), records it in the ledger (default <out>/review-ledger.jsonl) and prints
|
||||
BRIEF: reader=<role/name> sha256=<h> bytes=<n> file=<path>. Pass that file's content as the reader's prompt, unchanged.
|
||||
sent --ledger <f> --role <role> [--name <n>] --sent <file>
|
||||
the exact prompt a dispatch passed: BRIEF_SENT: … match, or BRIEF_MISMATCH: … (exit 1; the pass is not reviewed)
|
||||
collect --ledger <f> --role <role> [--name <n>] --output <file> --merged <merged.jsonl>
|
||||
collect --ledger <f> --role <role> [--name <n>] --output <file> --merged <merged.jsonl> [--manifest <f>]
|
||||
the reader's raw output: hashes it and appends each finding to merged.jsonl with its label as written. A reply with
|
||||
no finding must carry its brief's clean marker (NO FINDINGS; the Recommendation: line for the voices); without one it is
|
||||
COLLECT_NOT_REVIEWED (exit 1): a refusal, an empty reply or a reader that never saw the diff, and verify reports NOT_COLLECTED
|
||||
until the brief is re-sent and answered with a review, which voids it. --unavailable '<reason>' (the outside voice only; refused when the
|
||||
until the brief is re-sent and answered with a review, which voids it. A reply whose REVIEWED_TREE line is missing or names
|
||||
another checkout or tree than its brief was rendered on is COLLECT_WRONG_TREE (COLLECT_TREE_MOVED when only the tree
|
||||
moved; exit 1, nothing merged, verify reports NOT_COLLECTED, and with --manifest <f> the reader becomes failed); the
|
||||
outside voice's, when it holds no finding, is recorded as unavailable coverage instead --unavailable '<reason>' (the outside voice only; refused when the
|
||||
output holds findings) records its failure text as missing coverage: COLLECT_UNAVAILABLE, and verify prints UNAVAILABLE and unavailable=N, never a review
|
||||
judge --ledger <f> --merged <merged.jsonl> --judged <judged.jsonl>
|
||||
judged lines {"id","severity","reason"?,"duplicate_of"?}: every merged id kept at its severity or above (a
|
||||
@@ -68,6 +72,7 @@ const HELP = `gstack-review-dispatch — verbatim reviewer briefs and loss-free
|
||||
chunk --manifest <f> --reader <r> --index <i>
|
||||
BRIEF_CHUNK: reader=… index=i of=n sha256=…, then one JSON string line of at most 65,536 brief characters
|
||||
mark --manifest <f> --reader <r> --state <intent|created|queued|collected|failed> [--path <p>] [--output <f>] [--error <e>]
|
||||
an audit reader is collected with --output, whose REVIEWED_TREE line must match its render (else failed, COLLECT_WRONG_TREE)
|
||||
mark --manifest <f> --before <tokens>,<credits>
|
||||
mark --manifest <f> --log-start <key> --token <t> records a review-log start token once, so a rerun starts only what is missing
|
||||
reader states move forward only; intent is recorded before the subagent is created, its path right after
|
||||
@@ -113,18 +118,23 @@ function render(): number {
|
||||
classes: flag('--classes'), brief,
|
||||
fix_list: readFile(flag('--fix-list'), 'fix list'), hunks: readFile(flag('--hunks'), 'hunks'), tests: readFile(flag('--tests'), 'tests'), callers: readFile(flag('--callers'), 'callers'),
|
||||
};
|
||||
const text = renderBrief(role, slots, sectionTexts(ROOT));
|
||||
const workdir = checkoutRoot();
|
||||
const tree = checkoutTree(workdir);
|
||||
if (!tree) throw new BriefError(`cannot read HEAD^{tree} and the toplevel of ${workdir}; a brief names the checkout its reply must prove`);
|
||||
const text = withWorkspace(renderBrief(role, slots, sectionTexts(ROOT)), workdir);
|
||||
fs.mkdirSync(out, { recursive: true, mode: 0o700 });
|
||||
const file = path.join(path.resolve(out), briefFile(role, name));
|
||||
fs.writeFileSync(file, text, { mode: 0o600 });
|
||||
const ledger = flag('--ledger') ?? path.join(out, 'review-ledger.jsonl');
|
||||
const sha = promptHash(text);
|
||||
const targeted = brief === undefined ? {} : { classes: parseClassList(flag('--classes')!), rules: TARGETED_RULES, brief_sha: targetedBriefSha(checklist), diff_base: diffBase ?? null, patch_id: diffBase ? targetedPatchId(process.cwd(), diffBase) : 'none' };
|
||||
appendEvent(ledger, { event: 'rendered', reader: readerKey(role, name), role, name: name ?? null, file, sha256: sha, bytes: Buffer.byteLength(text), ...targeted });
|
||||
appendEvent(ledger, { event: 'rendered', reader: readerKey(role, name), role, name: name ?? null, file, sha256: sha, bytes: Buffer.byteLength(text), workdir, tree, ...targeted });
|
||||
process.stdout.write(`BRIEF: reader=${readerKey(role, name)} sha256=${sha} bytes=${Buffer.byteLength(text)} file=${file}\n`);
|
||||
return EXIT.ok;
|
||||
}
|
||||
|
||||
const wrongTree = (reader: string, t: TreeCheck) => `${t.moved ? 'COLLECT_TREE_MOVED' : 'COLLECT_WRONG_TREE'}: reader=${reader} reported=${JSON.stringify(t.reported)} expected=${JSON.stringify(t.expected)} (${t.moved ? 'the checkout moved after this brief was rendered' : 'the reply did not review the checkout its brief named'}; nothing merged; restore it with a fresh pass under a new --pass and --out)\n`;
|
||||
|
||||
const PROGRAM_FILE = path.join(ROOT, 'lib', 'capy-review-pass.js');
|
||||
const PROGRAM_VERSION = 1;
|
||||
const git = (a: string[]) => { const r = spawnSync('git', a, { encoding: 'utf8', timeout: 60_000, maxBuffer: 256 * 1024 * 1024 }); return r.status === 0 ? r.stdout : null; };
|
||||
@@ -215,6 +225,17 @@ function passStep(cmd: string): number {
|
||||
process.stdout.write(`BEFORE: tokens=${m.before.tokens} credits=${m.before.credits}\n`);
|
||||
return EXIT.ok;
|
||||
}
|
||||
const audit = m.readers.find(x => x.reader === flag('--reader'));
|
||||
if (flag('--state') === 'collected' && audit?.prompt_file && audit.tree !== undefined) {
|
||||
if (!flag('--output')) { process.stderr.write(`gstack-review-dispatch mark: ${audit.reader} is collected with --output <its reply>, which must prove the checkout it reviewed\n`); return EXIT.usage; }
|
||||
const tree = checkReviewedTree(readFile(flag('--output'), 'reader output')!, audit.tree);
|
||||
if (!tree.ok) {
|
||||
markReader(m, audit.reader, 'failed', { output: flag('--output'), error: `reviewed another tree: ${tree.reported}` });
|
||||
writeManifest(file, m);
|
||||
process.stdout.write(wrongTree(audit.reader, tree));
|
||||
return EXIT.fail;
|
||||
}
|
||||
}
|
||||
const r = markReader(m, flag('--reader') ?? '', flag('--state') as ReaderState, { path: flag('--path'), output: flag('--output'), error: flag('--error') });
|
||||
writeManifest(file, m);
|
||||
process.stdout.write(readerLine(m, r) + '\n');
|
||||
@@ -276,7 +297,21 @@ function main(): number {
|
||||
if (unavailable !== undefined && !mayBeUnavailable(reader)) { process.stderr.write(`gstack-review-dispatch collect: --unavailable is for the outside voice only; ${reader}'s missing coverage leaves the review incomplete, never clean\n`); return EXIT.usage; }
|
||||
if (unavailable !== undefined && !unavailable.trim()) { process.stderr.write('gstack-review-dispatch collect: --unavailable needs a reason\n'); return EXIT.usage; }
|
||||
if (unavailable !== undefined && parseFindings(reader, output).length) { process.stderr.write(`gstack-review-dispatch collect: ${reader}'s output holds findings; collect it without --unavailable so none is lost\n`); return EXIT.usage; }
|
||||
const r = collect(ledger, reader, output, merged, path.resolve(outFile!), unavailable);
|
||||
const rendered = readLedger(ledger).filter(e => e.event === 'rendered' && e.reader === reader).at(-1);
|
||||
const tree = unavailable === undefined && rendered && 'tree' in rendered ? checkReviewedTree(output, typeof rendered.tree === 'string' ? rendered.tree : null) : undefined;
|
||||
if (tree && !tree.ok && mayBeUnavailable(reader) && !parseFindings(reader, output).length) {
|
||||
const why = `reviewed another tree: reported ${tree.reported}, expected ${tree.expected}`;
|
||||
const r = collect(ledger, reader, output, merged, path.resolve(outFile!), why);
|
||||
process.stdout.write(`COLLECT_UNAVAILABLE: reader=${reader} sha256=${r.sha} reason=${JSON.stringify(why)} (missing outside coverage: unavailable, never clean)\n`);
|
||||
return EXIT.ok;
|
||||
}
|
||||
const r = collect(ledger, reader, output, merged, path.resolve(outFile!), unavailable, tree);
|
||||
if (tree && !tree.ok) {
|
||||
const mf = flag('--manifest');
|
||||
if (mf) { const m = readManifest(mf); markReader(m, reader, 'failed', { output: outFile, error: `reviewed another tree: ${tree.reported}` }); writeManifest(mf, m); }
|
||||
process.stdout.write(wrongTree(reader, tree));
|
||||
return EXIT.fail;
|
||||
}
|
||||
if (unavailable !== undefined) { process.stdout.write(`COLLECT_UNAVAILABLE: reader=${reader} sha256=${r.sha} reason=${JSON.stringify(unavailable)} (missing outside coverage: unavailable, never clean)\n`); return EXIT.ok; }
|
||||
if (!r.review) {
|
||||
process.stdout.write(`COLLECT_NOT_REVIEWED: reader=${reader} findings=0 sha256=${r.sha} (no finding and no ${cleanMarkers(reader).map(([m]) => m).join(' or ')} line: a refusal, an empty reply or a reader that never saw the diff is not a review; verify reports this brief NOT_COLLECTED)\n`);
|
||||
|
||||
@@ -514,6 +514,17 @@ ones, and the arm lost blockers. `bin/gstack-review-dispatch` closes both ends:
|
||||
missing or doubled title stops the pass. On Capy the pass runs as one call to the
|
||||
checked-in program `lib/capy-review-pass.js`, served by `program` and loaded
|
||||
unchanged by the gstack Capy skill.
|
||||
7. Every brief, audit readers' included, ends with a workspace paragraph naming the
|
||||
checkout the bins run in (GST-482): a reader on a shared machine starts in the
|
||||
machine's working directory, so it is told to `cd` there on every command, to give
|
||||
every file tool an absolute path under it, and to start its reply with
|
||||
`REVIEWED_TREE: <HEAD^{tree}> <toplevel>` measured there. The paragraph follows the
|
||||
section's render, which stays byte for byte. `collect` and `mark --state collected`
|
||||
compare that line with the fingerprint recorded at render: a missing or foreign line is
|
||||
`COLLECT_WRONG_TREE`, a later commit in the same checkout `COLLECT_TREE_MOVED`; nothing
|
||||
is merged, the reader becomes `failed`, and `verify` reports `NOT_COLLECTED`. An
|
||||
outside voice's tree failure is recorded `unavailable` only when its reply holds no finding. The line catches a reader that
|
||||
worked elsewhere by accident; it is not proof of reading, since diff text can predict it.
|
||||
|
||||
## Where this is enforced
|
||||
|
||||
|
||||
@@ -129,12 +129,13 @@ try {
|
||||
}
|
||||
const file = m.out + '/out/' + safe(r.reader) + '.txt';
|
||||
await tools.capy.write({ path: file, content: g.summary });
|
||||
const c = await sh(r.prompt_file ? bin + ' mark --manifest ' + q(mf) + ' --reader ' + q(r.reader) + ' --state collected --output ' + q(file)
|
||||
: bin + ' collect --ledger ' + q(m.ledger) + roleArgs(r) + ' --output ' + q(file) + ' --merged ' + q(m.merged) + ' --manifest ' + q(mf));
|
||||
if (c.code !== 0) return stop((r.prompt_file ? 'mark ' : 'collect ') + r.reader + ' exited ' + c.code + ': ' + c.out.slice(-500));
|
||||
if (!r.prompt_file) {
|
||||
const c = await sh(bin + ' collect --ledger ' + q(m.ledger) + roleArgs(r) + ' --output ' + q(file) + ' --merged ' + q(m.merged));
|
||||
if (c.code !== 0) return stop('collect ' + r.reader + ' exited ' + c.code + ': ' + c.out.slice(-500));
|
||||
lines.push(c.out.trim());
|
||||
await mark(r.reader, 'collected', { output: file });
|
||||
}
|
||||
await mark(r.reader, 'collected', { output: file });
|
||||
}
|
||||
m = await load();
|
||||
const stage = active.find((s) => m.readers.some((r) => r.stage === s && r.state !== 'collected'));
|
||||
|
||||
@@ -14,6 +14,7 @@
|
||||
* fix-check fix_list, hunks, tests, callers (scripts/resolvers/fix-check.ts)
|
||||
* targeted classes, brief (review/specialists/targeted.md's text), checklist (its path), diff_base, stack, test_fw, learnings
|
||||
*/
|
||||
import { spawnSync } from 'node:child_process';
|
||||
import { createHash } from 'node:crypto';
|
||||
import * as fs from 'node:fs';
|
||||
import * as path from 'node:path';
|
||||
@@ -134,6 +135,55 @@ function renderRole(role: BriefRole, slots: Slots, t: SectionTexts): string {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The closing paragraph every dispatched brief carries (GST-482): a reader on a shared machine starts in that machine's working
|
||||
* directory, and every command runs in a fresh shell, so the brief names the checkout under review and asks for a fingerprint
|
||||
* measured there. It is appended after the section's render, so the brief before it stays byte for byte what renderBrief returns.
|
||||
*/
|
||||
export function workspaceParagraph(dir: string): string {
|
||||
const q = `'${dir.replace(/'/g, `'\\''`)}'`;
|
||||
return `Workspace: the checkout under review is ${dir}. Your shell and file tools may start in another checkout, and every command runs in a fresh shell: begin each command with \`cd ${q} && \`, and give every file read, search, edit or write an absolute path under ${dir}. Make the first line of your final reply the line \`cd ${q} && echo "REVIEWED_TREE: $(git rev-parse 'HEAD^{tree}') $(git rev-parse --show-toplevel)"\` prints, even where this brief limits your output to findings; a reply without it, or from another checkout, does not count.`;
|
||||
}
|
||||
export const withWorkspace = (text: string, dir: string) => `${text}\n\n${workspaceParagraph(dir)}`;
|
||||
/** The checkout a brief names: the toplevel of the directory the bins run in, where its diff and merge base come from. */
|
||||
export function checkoutRoot(): string {
|
||||
const r = spawnSync('git', ['rev-parse', '--show-toplevel'], { encoding: 'utf8', timeout: 30_000 });
|
||||
return realDir(r.status === 0 ? r.stdout.trim() : process.cwd());
|
||||
}
|
||||
|
||||
/**
|
||||
* One REVIEWED_TREE line as "<tree> <toplevel>", or null. A list marker or heading mark may lead it, and a whole-line `**…**` or
|
||||
* backtick wrapper, a bold label and backticks around the sha or the whole path are removed; the path's own characters are kept.
|
||||
*/
|
||||
function parseTreeLine(line: string): string | null {
|
||||
let t = line.trim().replace(/^(?:[-*>]\s+|\d+[.)]\s+|#+\s+)/, '');
|
||||
for (const w of ['**', '`']) if (t.length > 2 * w.length && t.startsWith(w) && t.endsWith(w)) t = t.slice(w.length, -w.length).trim();
|
||||
const m = /^(?:\*\*REVIEWED_TREE:\*\*|REVIEWED_TREE:)\s+`?([0-9a-f]{40,64})`?\s+(.+)$/.exec(t);
|
||||
if (!m) return null;
|
||||
const p = m[2]!.length > 2 && m[2]!.startsWith('`') && m[2]!.endsWith('`') ? m[2]!.slice(1, -1) : m[2]!;
|
||||
return `${m[1]} ${realDir(p)}`;
|
||||
}
|
||||
export const isReviewedTreeLine = (line: string) => parseTreeLine(line) !== null;
|
||||
const realDir = (p: string) => { try { return fs.realpathSync(p); } catch { return path.resolve(p); } };
|
||||
/** The checkout a reply says it reviewed: its first REVIEWED_TREE line (a later one may be quoted inside a finding); null with none. */
|
||||
const reportedTree = (output: string): string | null => output.replace(/\r\n/g, '\n').split('\n').map(parseTreeLine).find(t => t !== null) ?? null;
|
||||
/** The fingerprint a reply must report, read from the checkout itself when its brief is rendered: "<HEAD^{tree}> <toplevel>". */
|
||||
export function checkoutTree(dir: string): string | null {
|
||||
const git = (a: string[]) => spawnSync('git', ['-C', dir, ...a], { encoding: 'utf8', timeout: 30_000 });
|
||||
const tree = git(['rev-parse', 'HEAD^{tree}']), top = git(['rev-parse', '--show-toplevel']);
|
||||
return tree.status === 0 && top.status === 0 ? `${tree.stdout.trim()} ${realDir(top.stdout.trim())}` : null;
|
||||
}
|
||||
/** moved: the reply reviewed the named checkout, but at another tree than the one its brief was rendered on. */
|
||||
export type TreeCheck = { ok: boolean; moved: boolean; expected: string; reported: string };
|
||||
/** Whether a reply's REVIEWED_TREE line is the fingerprint recorded when its brief was rendered (GST-482). */
|
||||
export function checkReviewedTree(output: string, expected: string | null): TreeCheck {
|
||||
const reported = reportedTree(output);
|
||||
const ok = expected !== null && reported === expected;
|
||||
const top = (t: string) => t.slice(t.indexOf(' ') + 1);
|
||||
return { ok, moved: !ok && !!reported && expected !== null && top(reported) === top(expected),
|
||||
expected: expected ?? 'none (the checkout was unreadable at render)', reported: reported ?? 'none' };
|
||||
}
|
||||
|
||||
/** The file a rendered brief is written to under the run's briefs directory. */
|
||||
export function briefFile(role: BriefRole, name?: string): string {
|
||||
return `${role}${name ? `-${name.replace(/[^a-zA-Z0-9._-]/g, '_')}` : ''}.prompt.md`;
|
||||
|
||||
+13
-6
@@ -16,7 +16,7 @@
|
||||
*/
|
||||
import * as fs from 'node:fs';
|
||||
import * as path from 'node:path';
|
||||
import { promptHash } from './review-brief';
|
||||
import { isReviewedTreeLine, promptHash, type TreeCheck } from './review-brief';
|
||||
|
||||
export interface LedgerEvent { event: 'rendered' | 'sent' | 'output' | 'judgment' | 'judge' | 'program'; [k: string]: unknown }
|
||||
export const readerKey = (role: string, name?: string) => (name ? `${role}/${name}` : role);
|
||||
@@ -83,6 +83,7 @@ export function parseFindings(reader: string, output: string): Array<{ label: st
|
||||
let current: Finding | null = null;
|
||||
let head = '';
|
||||
let gap = false;
|
||||
let treeSeen = false;
|
||||
const role = reader.split('/')[0];
|
||||
const named = (l: string) => (role === 'adversarial-native' && l.toUpperCase() === 'FIXABLE' ? 'CRITICAL' : l);
|
||||
const strongest = (t: string) => [...t.matchAll(ALL_LABELS)].map(m => named(m[1]!)).reduce<string | undefined>((a, b) => (a === undefined ? b : stronger(a, b)), undefined);
|
||||
@@ -90,6 +91,7 @@ export function parseFindings(reader: string, output: string): Array<{ label: st
|
||||
for (const line of output.replace(/\r\n/g, '\n').split('\n')) {
|
||||
const t = line.trim();
|
||||
if (!t) { gap = true; continue; }
|
||||
if (!treeSeen && isReviewedTreeLine(t)) { treeSeen = true; continue; }
|
||||
const afterGap = gap;
|
||||
gap = false;
|
||||
if (/^NO FINDINGS\.?$/.test(t) || /^Recommendation:/i.test(t)) { current = null; continue; }
|
||||
@@ -155,15 +157,18 @@ const hasCleanMarker = (reader: string, output: string) => output.replace(/\r\n/
|
||||
|
||||
/**
|
||||
* collect: hash the raw output, append its findings to merged.jsonl and record both; a reply with no finding and no clean marker records
|
||||
* review: false. With unavailable (the outside voice only) the output is its failure text, recorded as missing coverage.
|
||||
* review: false. With unavailable (the outside voice only) the output is its failure text, recorded as missing coverage. With a
|
||||
* failed tree check (GST-482: the reply reviewed another checkout, or names none) nothing is merged and the output is no review.
|
||||
*/
|
||||
export function collect(ledger: string, reader: string, output: string, mergedFile: string, outputFile?: string, unavailable?: string): { sha: string; lines: MergedLine[]; review: boolean } {
|
||||
export function collect(ledger: string, reader: string, output: string, mergedFile: string, outputFile?: string, unavailable?: string, tree?: TreeCheck): { sha: string; lines: MergedLine[]; review: boolean } {
|
||||
const sha = promptHash(output);
|
||||
const prior = readLedger(ledger).filter(e => e.event === 'output' && e.reader === reader).length;
|
||||
const lines = unavailable ? [] : parseFindings(reader, output).map((f, i) => ({ id: `${reader}#${prior ? `${prior + 1}.` : ''}${i + 1}`, reader, label: f.label, raw: f.raw, output_sha256: sha }));
|
||||
const review = !unavailable && (lines.length > 0 || hasCleanMarker(reader, output));
|
||||
const wrongTree = !unavailable && tree !== undefined && !tree.ok;
|
||||
const lines = unavailable || wrongTree ? [] : parseFindings(reader, output).map((f, i) => ({ id: `${reader}#${prior ? `${prior + 1}.` : ''}${i + 1}`, reader, label: f.label, raw: f.raw, output_sha256: sha }));
|
||||
const review = !unavailable && !wrongTree && (lines.length > 0 || hasCleanMarker(reader, output));
|
||||
fs.appendFileSync(mergedFile, lines.map(l => JSON.stringify(l) + '\n').join(''), { mode: 0o600 });
|
||||
appendEvent(ledger, { event: 'output', reader, sha256: sha, file: outputFile ? path.resolve(outputFile) : null, merged: path.resolve(mergedFile), ids: lines.map(l => l.id), review, ...(unavailable ? { unavailable } : {}) });
|
||||
appendEvent(ledger, { event: 'output', reader, sha256: sha, file: outputFile ? path.resolve(outputFile) : null, merged: path.resolve(mergedFile), ids: lines.map(l => l.id), review, ...(unavailable ? { unavailable } : {}),
|
||||
...(tree && !unavailable ? { tree: tree.expected, reported_tree: tree.reported, tree_ok: tree.ok } : {}) });
|
||||
return { sha, lines, review };
|
||||
}
|
||||
/** The outside voice may fail without blocking (ship/sections/adversarial.md): its failure is recorded as unavailable coverage, never as a review. */
|
||||
@@ -222,6 +227,7 @@ export function recordJudgment(ledger: string, merged: MergedLine[], judged: Jud
|
||||
* a ledger written before collect recorded the field is judged by the same rule; with no readable file, collect's own review field.
|
||||
*/
|
||||
function reviewed(o: LedgerEvent): boolean {
|
||||
if (o.tree_ok === false || unavailableOutput(o)) return false;
|
||||
if (Array.isArray(o.ids) && o.ids.length) return true;
|
||||
let text: string | undefined;
|
||||
try { text = typeof o.file === 'string' ? fs.readFileSync(o.file, 'utf8') : undefined; } catch { /* unreadable */ }
|
||||
@@ -292,6 +298,7 @@ export function verify(ledger: string): { rows: VerifyRow[]; clean: boolean; bri
|
||||
const replaced = review > 0 && later.slice(0, review).some(e => e.event === 'sent' && e.reader === reader && e.sha256 === events[p!.at]!.sha256);
|
||||
if (unavailableOutput(o)) { status = 'UNAVAILABLE'; notes.push(`reason=${JSON.stringify(o.unavailable)}`); }
|
||||
else if (replaced) { status = 'VOIDED'; voided++; notes.push('replaced_by_a_review'); }
|
||||
else if (o.tree_ok === false) { status = 'NOT_REVIEWED'; notes.push(`wrong_tree reported=${JSON.stringify(o.reported_tree)} expected=${JSON.stringify(o.tree)}`); }
|
||||
else { status = 'NOT_REVIEWED'; notes.push('no_finding_no_clean_marker'); }
|
||||
}
|
||||
rows.push({ kind: 'OUTPUT', reader: p?.label ?? reader, status, detail: `sha=${short(o.sha256)} findings=${ids.length}${notes.length ? ' ' + notes.join(' ') : ''}` });
|
||||
|
||||
+13
-6
@@ -13,7 +13,7 @@
|
||||
*/
|
||||
import * as fs from 'node:fs';
|
||||
import * as path from 'node:path';
|
||||
import { BriefError, briefFile, promptHash, renderBrief, type BriefRole, type SectionTexts, type Slots } from './review-brief';
|
||||
import { BriefError, briefFile, checkoutRoot, checkoutTree, promptHash, renderBrief, withWorkspace, type BriefRole, type SectionTexts, type Slots } from './review-brief';
|
||||
import { appendEvent, readerKey } from './review-ledger';
|
||||
import { diffCommand, type Dispatch } from './review-depth';
|
||||
import { TARGETED_RULES, parseClassList, targetedBriefSha, targetedPatchId } from './review-targeted';
|
||||
@@ -31,6 +31,8 @@ export interface PassReader {
|
||||
/** An audit or QA reader: its prompt is a file the parent wrote from its section; its output is returned, never collected. */
|
||||
prompt_file?: string; model?: PassModel;
|
||||
file?: string; sha256?: string; bytes?: number; chunks?: number; findings_sha256?: string;
|
||||
/** The fingerprint the reader's reply must report: the checkout's HEAD^{tree} and toplevel when its brief was rendered (GST-482). */
|
||||
tree?: string | null;
|
||||
path?: string; output?: string; error?: string; state_at?: string;
|
||||
}
|
||||
export interface Manifest {
|
||||
@@ -126,11 +128,16 @@ export function renderStage(m: Manifest, stage: Stage, t: SectionTexts, diffFor:
|
||||
const briefs = path.join(m.out, 'briefs');
|
||||
fs.mkdirSync(briefs, { recursive: true, mode: 0o700 });
|
||||
const done: PassReader[] = [];
|
||||
const workdir = checkoutRoot();
|
||||
const tree = checkoutTree(workdir);
|
||||
if (!tree) throw new BriefError(`cannot read HEAD^{tree} and the toplevel of ${workdir}; a brief names the checkout its reply must prove`);
|
||||
for (const r of m.readers.filter(x => x.stage === stage && x.state === 'planned')) {
|
||||
if (r.prompt_file) {
|
||||
let text: string;
|
||||
try { text = fs.readFileSync(r.prompt_file, 'utf8'); } catch { throw new BriefError(`cannot read ${r.reader}'s prompt file ${r.prompt_file}`); }
|
||||
Object.assign(r, { file: r.prompt_file, sha256: promptHash(text), bytes: Buffer.byteLength(text), chunks: chunkCount(text), state: 'rendered' });
|
||||
try { text = withWorkspace(fs.readFileSync(r.prompt_file, 'utf8'), workdir); } catch { throw new BriefError(`cannot read ${r.reader}'s prompt file ${r.prompt_file}`); }
|
||||
const file = path.join(briefs, `${safe(r.reader)}.prompt.md`);
|
||||
fs.writeFileSync(file, text, { mode: 0o600 });
|
||||
Object.assign(r, { file, sha256: promptHash(text), bytes: Buffer.byteLength(text), chunks: chunkCount(text), state: 'rendered', tree });
|
||||
done.push(r);
|
||||
continue;
|
||||
}
|
||||
@@ -155,13 +162,13 @@ export function renderStage(m: Manifest, stage: Stage, t: SectionTexts, diffFor:
|
||||
Object.assign(slots, { checklist, brief, classes: (r.classes ?? []).join(','), diff_base: r.diff_base ?? m.diff_base });
|
||||
targeted = { classes: parseClassList(slots.classes!), rules: TARGETED_RULES, brief_sha: targetedBriefSha(checklist), diff_base: slots.diff_base, patch_id: targetedPatchId(process.cwd(), slots.diff_base!) };
|
||||
}
|
||||
const text = renderBrief(role, slots, t);
|
||||
const text = withWorkspace(renderBrief(role, slots, t), workdir);
|
||||
const file = path.join(briefs, briefFile(role, r.name));
|
||||
fs.writeFileSync(file, text, { mode: 0o600 });
|
||||
const sha = promptHash(text);
|
||||
const bytes = Buffer.byteLength(text);
|
||||
appendEvent(m.ledger, { event: 'rendered', reader: r.reader, role, name: r.name ?? null, file, sha256: sha, bytes, pass: m.pass, stage, ...(findingsSha ? { findings_sha256: findingsSha } : {}), ...targeted });
|
||||
Object.assign(r, { file, sha256: sha, bytes, chunks: chunkCount(text), state: 'rendered', ...(findingsSha ? { findings_sha256: findingsSha } : {}) });
|
||||
appendEvent(m.ledger, { event: 'rendered', reader: r.reader, role, name: r.name ?? null, file, sha256: sha, bytes, pass: m.pass, stage, workdir, tree, ...(findingsSha ? { findings_sha256: findingsSha } : {}), ...targeted });
|
||||
Object.assign(r, { file, sha256: sha, bytes, chunks: chunkCount(text), state: 'rendered', tree, ...(findingsSha ? { findings_sha256: findingsSha } : {}) });
|
||||
done.push(r);
|
||||
}
|
||||
return done;
|
||||
|
||||
+1
-1
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "gstack",
|
||||
"version": "1.91.378",
|
||||
"version": "1.91.379",
|
||||
"description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.",
|
||||
"license": "MIT",
|
||||
"type": "module",
|
||||
|
||||
@@ -3,7 +3,7 @@ import { spawnSync } from 'node:child_process';
|
||||
import * as fs from 'node:fs';
|
||||
import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
import { BriefError, promptHash, renderBrief, sectionTexts } from '../lib/review-brief';
|
||||
import { BriefError, checkReviewedTree, promptHash, renderBrief, sectionTexts, workspaceParagraph } from '../lib/review-brief';
|
||||
import { collect, judge, parseFindings, type MergedLine } from '../lib/review-ledger';
|
||||
|
||||
const ROOT = path.resolve(import.meta.dir, '..');
|
||||
@@ -11,7 +11,19 @@ const BIN = path.join(ROOT, 'bin', 'gstack-review-dispatch');
|
||||
const dirs: string[] = [];
|
||||
afterEach(() => { while (dirs.length) fs.rmSync(dirs.pop()!, { recursive: true, force: true }); });
|
||||
const tmp = () => { const d = fs.mkdtempSync(path.join(os.tmpdir(), 'review-dispatch-')); dirs.push(d); return d; };
|
||||
const run = (args: string[], cwd = ROOT) => spawnSync(BIN, args, { cwd, encoding: 'utf8', timeout: 60_000 });
|
||||
/** A reader that follows its brief's workspace paragraph starts its reply with the REVIEWED_TREE line of that checkout (GST-482). */
|
||||
function treeLineOf(cwd: string): string {
|
||||
const git = (a: string[]) => spawnSync('git', ['-C', cwd, ...a], { encoding: 'utf8', timeout: 30_000 }).stdout.trim();
|
||||
return `REVIEWED_TREE: ${git(['rev-parse', 'HEAD^{tree}'])} ${git(['rev-parse', '--show-toplevel'])}\n`;
|
||||
}
|
||||
function followBrief(output: string, cwd: string) {
|
||||
const text = fs.readFileSync(output, 'utf8');
|
||||
if (!text.includes('REVIEWED_TREE:')) fs.writeFileSync(output, treeLineOf(cwd) + text);
|
||||
}
|
||||
const run = (args: string[], cwd = ROOT) => {
|
||||
if (args[0] === 'collect' && !args.includes('--unavailable') && args.includes('--output')) followBrief(args[args.indexOf('--output') + 1]!, cwd);
|
||||
return spawnSync(BIN, args, { cwd, encoding: 'utf8', timeout: 60_000 });
|
||||
};
|
||||
const SHA = 'a'.repeat(40);
|
||||
const section = (rel: string) => fs.readFileSync(path.join(ROOT, rel), 'utf8');
|
||||
const field = (out: string, k: string) => new RegExp(`${k}=(\\S+)`).exec(out)?.[1] ?? '';
|
||||
@@ -338,10 +350,10 @@ describe('gstack-review-dispatch sent, collect, judge and verify: nothing change
|
||||
const b = tmp();
|
||||
const batched = field(run(['render', '--role', 'specialist-batched', '--checklists', 'testing,security,state-machines', '--out', b, '--diff-base', SHA]).stdout, 'file');
|
||||
const cases = [
|
||||
{ reader: 'specialist-batched', dir: b, brief: batched, output: refusal, args: ['--role', 'specialist-batched'] },
|
||||
{ reader: 'specialist-batched', dir: b, brief: batched, output: treeLineOf(ROOT) + refusal, args: ['--role', 'specialist-batched'] },
|
||||
...['', 'I could not access the diff: git diff failed with "bad object", so there is nothing here to review.\n'].map(output => {
|
||||
const p = pass();
|
||||
return { reader: 'specialist/testing', dir: p.out, brief: p.file, output, args: ['--role', 'specialist', '--name', 'testing'] };
|
||||
return { reader: 'specialist/testing', dir: p.out, brief: p.file, output: treeLineOf(ROOT) + output, args: ['--role', 'specialist', '--name', 'testing'] };
|
||||
}),
|
||||
];
|
||||
for (const c of cases) {
|
||||
@@ -365,7 +377,7 @@ describe('gstack-review-dispatch sent, collect, judge and verify: nothing change
|
||||
// the non-review rule; fails_when=the rule makes a real NO FINDINGS specialist or a findings reply dirty or exit 1;
|
||||
// why_new=the non-review rule above could over-reach; seam=collect and verify; no_claim=other roles (the matrix below covers them)
|
||||
test('a NO FINDINGS reply and a findings reply still collect with exit 0 and verify clean', () => {
|
||||
for (const [output, findings] of [['NO FINDINGS\n', 0], ['{"severity":"CRITICAL","path":"a.ts","line":3,"category":"logic","summary":"off by one","specialist":"testing"}\n', 1]] as const) {
|
||||
for (const [output, findings] of [[treeLineOf(ROOT) + 'NO FINDINGS\n', 0], [treeLineOf(ROOT) + '{"severity":"CRITICAL","path":"a.ts","line":3,"category":"logic","summary":"off by one","specialist":"testing"}\n', 1]] as const) {
|
||||
const p = pass();
|
||||
run(['sent', '--ledger', p.ledger, '--role', 'specialist', '--name', 'testing', '--sent', p.file]);
|
||||
const col = run(['collect', '--ledger', p.ledger, '--role', 'specialist', '--name', 'testing', '--output', p.w('reply.out', output), '--merged', p.merged]);
|
||||
@@ -476,3 +488,87 @@ describe('gstack-review-dispatch sent, collect, judge and verify: nothing change
|
||||
expect([v.status, v.stdout.trim()]).toEqual([3, "VERIFY: no brief records (this run's ledger kept no prompts; dispatched-vs-rendered cannot be checked)"]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('the workspace paragraph (GST-482)', () => {
|
||||
// Value: protects=GSTA-167's no-loss pin: every role's brief is renderBrief's bytes unchanged, then only the workspace paragraph naming the checkout; fails_when=the paragraph is inserted, a section byte moves, or the CLI names another folder than the checkout it ran in;
|
||||
// why_new=GST-482 appends a paragraph to every brief; seam=gstack-review-dispatch render; no_claim=that the reader obeys it
|
||||
test('every role renders main\'s brief byte for byte, then the paragraph naming the checkout the CLI ran in', () => {
|
||||
const t = sectionTexts(ROOT);
|
||||
const top = fs.realpathSync(spawnSync('git', ['rev-parse', '--show-toplevel'], { cwd: ROOT, encoding: 'utf8', timeout: 30_000 }).stdout.trim());
|
||||
const w = (n: string, s: string) => { const f = path.join(tmp(), n); fs.writeFileSync(f, s); return f; };
|
||||
const cases: Array<[string, string[], Record<string, string>]> = [
|
||||
['specialist', ['--name', 'testing'], { name: 'testing', checklist: path.join(ROOT, 'review/specialists/testing.md') }],
|
||||
['specialist', ['--name', 'security'], { name: 'security', checklist: path.join(ROOT, 'review/specialists/security.md') }],
|
||||
['specialist-batched', ['--checklists', 'testing,security'], { checklists: ['testing', 'security'].map(c => path.join(ROOT, 'review/specialists', c + '.md')).join(',') }],
|
||||
['red-team', ['--findings-file', w('f.jsonl', 'x\n'), '--specialists', '2'], { checklist: path.join(ROOT, 'review/specialists/red-team.md'), findings: 'x\n', specialists: '2' }],
|
||||
['adversarial-native', [], {}],
|
||||
['fix-check', ['--fix-list', w('fl', 'a'), '--hunks', w('h', 'b'), '--tests', w('t', 'c')], { fix_list: 'a', hunks: 'b', tests: 'c' }],
|
||||
];
|
||||
for (const [role, args, slots] of cases) {
|
||||
const out = tmp();
|
||||
const r = run(['render', '--role', role, '--out', out, ...(role === 'fix-check' ? [] : ['--diff-base', SHA]), ...args]);
|
||||
expect(r.status, role + ': ' + r.stderr).toBe(0);
|
||||
const main = renderBrief(role as Parameters<typeof renderBrief>[0], { diff_base: role === 'fix-check' ? undefined : SHA, ...slots }, t);
|
||||
expect(fs.readFileSync(field(r.stdout, 'file'), 'utf8'), role).toBe(main + '\n\n' + workspaceParagraph(top));
|
||||
}
|
||||
expect(workspaceParagraph(top)).toContain(`begin each command with \`cd '${top}' && \``);
|
||||
expect(workspaceParagraph(top)).toContain(`absolute path under ${top}`);
|
||||
});
|
||||
|
||||
test('a list-item or bold REVIEWED_TREE line counts; a ledger rendered before the paragraph existed is collected as before', () => {
|
||||
const out = tmp();
|
||||
const p = { ledger: path.join(out, 'review-ledger.jsonl'), merged: path.join(out, 'merged.jsonl'), file: field(run(['render', '--role', 'specialist', '--name', 'testing', '--out', out, '--diff-base', SHA]).stdout, 'file'),
|
||||
w: (n: string, t: string) => { const f = path.join(out, n); fs.writeFileSync(f, t); return f; } };
|
||||
run(['sent', '--ledger', p.ledger, '--role', 'specialist', '--name', 'testing', '--sent', p.file]);
|
||||
const line = treeLineOf(ROOT).trim();
|
||||
const col = run(['collect', '--ledger', p.ledger, '--role', 'specialist', '--name', 'testing', '--output', p.w('reply.out', '- **' + line + '**\nNO FINDINGS\n'), '--merged', p.merged]);
|
||||
expect(col.stdout).toStartWith('COLLECT: reader=specialist/testing findings=0 ');
|
||||
const legacy = tmp();
|
||||
const ledger = path.join(legacy, 'review-ledger.jsonl');
|
||||
const brief = path.join(legacy, 'b.md');
|
||||
fs.writeFileSync(brief, 'old brief');
|
||||
fs.writeFileSync(ledger, JSON.stringify({ event: 'rendered', reader: 'specialist/testing', role: 'specialist', name: 'testing', file: brief, sha256: promptHash('old brief'), bytes: 9 }) + '\n');
|
||||
run(['sent', '--ledger', ledger, '--role', 'specialist', '--name', 'testing', '--sent', brief]);
|
||||
const reply = path.join(legacy, 'r.out');
|
||||
fs.writeFileSync(reply, 'NO FINDINGS\n');
|
||||
const old = spawnSync(BIN, ['collect', '--ledger', ledger, '--role', 'specialist', '--name', 'testing', '--output', reply, '--merged', path.join(legacy, 'm.jsonl')], { cwd: ROOT, encoding: 'utf8', timeout: 60_000 });
|
||||
expect(old.stdout).toStartWith('COLLECT: reader=specialist/testing findings=0 ');
|
||||
});
|
||||
});
|
||||
|
||||
describe('reading the REVIEWED_TREE line (GST-482 review fixes)', () => {
|
||||
const top = fs.realpathSync(spawnSync('git', ['rev-parse', '--show-toplevel'], { cwd: ROOT, encoding: 'utf8', timeout: 30_000 }).stdout.trim());
|
||||
const tree = spawnSync('git', ['rev-parse', 'HEAD^{tree}'], { cwd: ROOT, encoding: 'utf8', timeout: 30_000 }).stdout.trim();
|
||||
const expected = `${tree} ${top}`;
|
||||
test('Markdown around the line, label, sha or whole path is read; the path keeps its own characters; the first line wins', () => {
|
||||
for (const line of [`REVIEWED_TREE: ${tree} ${top}`, `**REVIEWED_TREE:** ${tree} ${top}`, `REVIEWED_TREE: \`${tree}\` \`${top}\``, `## REVIEWED_TREE: ${tree} ${top}`, `- **REVIEWED_TREE: ${tree} ${top}**`])
|
||||
expect(checkReviewedTree(line + '\nNO FINDINGS', expected).ok, line).toBe(true);
|
||||
expect(checkReviewedTree(`REVIEWED_TREE: ${tree} ${top}*`, expected).ok).toBe(false);
|
||||
expect(checkReviewedTree(`REVIEWED_TREE: ${tree} ${top}\n\`\`\`\nREVIEWED_TREE: ${'b'.repeat(40)} /elsewhere\n\`\`\``, expected).ok).toBe(true);
|
||||
expect(checkReviewedTree(`REVIEWED_TREE: ${'b'.repeat(40)} /elsewhere\nREVIEWED_TREE: ${tree} ${top}`, expected)).toMatchObject({ ok: false, reported: `${'b'.repeat(40)} /elsewhere` });
|
||||
});
|
||||
|
||||
test('an outside voice with findings and no tree line is refused, never turned into unavailable coverage that drops them', () => {
|
||||
const out = tmp();
|
||||
const ledger = path.join(out, 'review-ledger.jsonl');
|
||||
const merged = path.join(out, 'merged.jsonl');
|
||||
const head = spawnSync('git', ['rev-parse', 'HEAD'], { cwd: ROOT, encoding: 'utf8', timeout: 30_000 }).stdout.trim();
|
||||
const file = field(run(['render', '--role', 'outside-voice', '--out', out, '--diff-base', head]).stdout, 'file');
|
||||
run(['sent', '--ledger', ledger, '--role', 'outside-voice', '--sent', file]);
|
||||
const reply = path.join(out, 'r.out');
|
||||
fs.writeFileSync(reply, '1. **CRITICAL**: auth bypass in a.ts:3\nBlocks: yes\n\nRecommendation: request changes\n');
|
||||
const c = spawnSync(BIN, ['collect', '--ledger', ledger, '--role', 'outside-voice', '--output', reply, '--merged', merged], { cwd: ROOT, encoding: 'utf8', timeout: 60_000 });
|
||||
expect([c.status, c.stdout.startsWith('COLLECT_WRONG_TREE: reader=outside-voice reported="none"')]).toEqual([1, true]);
|
||||
expect(run(['verify', '--ledger', ledger]).stdout).not.toContain('VERIFY: clean');
|
||||
fs.writeFileSync(reply, 'Codex timed out before it reviewed anything.\n');
|
||||
const u = spawnSync(BIN, ['collect', '--ledger', ledger, '--role', 'outside-voice', '--output', reply, '--merged', merged], { cwd: ROOT, encoding: 'utf8', timeout: 60_000 });
|
||||
expect([u.status, u.stdout.startsWith('COLLECT_UNAVAILABLE: reader=outside-voice')]).toEqual([0, true]);
|
||||
});
|
||||
|
||||
test('render outside a readable checkout refuses instead of recording a fingerprint no reply can match', () => {
|
||||
const dir = tmp();
|
||||
const r = run(['render', '--role', 'adversarial-native', '--out', path.join(dir, 'o'), '--diff-base', SHA], dir);
|
||||
expect(r.status).not.toBe(0);
|
||||
expect(r.stderr).toContain('BRIEF_RENDER_FAILED: cannot read HEAD^{tree} and the toplevel of');
|
||||
});
|
||||
});
|
||||
+134
-6
@@ -7,7 +7,7 @@ import { spawnSync } from 'node:child_process';
|
||||
import * as fs from 'node:fs';
|
||||
import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
import { promptHash, sectionTexts } from '../lib/review-brief';
|
||||
import { promptHash, sectionTexts, withWorkspace } from '../lib/review-brief';
|
||||
import { CHUNK_CHARS, buildManifest, chunkCount, chunkOf, markReader, renderStage, specialistFindings, type Manifest } from '../lib/review-pass';
|
||||
import type { Dispatch } from '../lib/review-depth';
|
||||
|
||||
@@ -129,19 +129,28 @@ function repo(): { dir: string; env: NodeJS.ProcessEnv } {
|
||||
interface Sub { title: string; prompt: string; status: string; polls: number; model?: unknown; summary?: string; late?: boolean }
|
||||
/** A fake Capy host: the shell is real (the repository's bins), subagents are scripted. */
|
||||
const cleanReply = (title: string) => (title.endsWith('outside-voice') ? 'Recommendation: ship as-is because no finding survived' : 'NO FINDINGS');
|
||||
function host(dir: string, env: NodeJS.ProcessEnv, o: { output?: (title: string) => string; fail?: (title: string) => boolean; reject?: (title: string) => boolean; createThrows?: (title: string) => 'before' | 'after' | undefined; pollsToFinish?: number; lateSummary?: boolean } = {}) {
|
||||
/** The REVIEWED_TREE line a reader prints from dir (its brief's checkout when it follows the workspace paragraph); '' outside a checkout. */
|
||||
function treeLine(dir: string): string {
|
||||
if (!dir) return '';
|
||||
const git = (a: string[]) => spawnSync('git', ['-C', dir, ...a], { encoding: 'utf8', timeout: 30_000 });
|
||||
const tree = git(['rev-parse', 'HEAD^{tree}']), top = git(['rev-parse', '--show-toplevel']);
|
||||
return tree.status === 0 && top.status === 0 ? `REVIEWED_TREE: ${tree.stdout.trim()} ${top.stdout.trim()}\n` : '';
|
||||
}
|
||||
const briefDir = (prompt: string) => /Workspace: the checkout under review is (.+?)\. Your shell and file tools/.exec(prompt)?.[1] ?? '';
|
||||
function host(dir: string, env: NodeJS.ProcessEnv, o: { output?: (title: string) => string; reportFrom?: (title: string) => string; fail?: (title: string) => boolean; reject?: (title: string) => boolean; createThrows?: (title: string) => 'before' | 'after' | undefined; pollsToFinish?: number; lateSummary?: boolean } = {}) {
|
||||
let clock = 0;
|
||||
const subs = new Map<string, Sub>();
|
||||
const creates: Sub[] = [];
|
||||
const starts: number[][] = [];
|
||||
const reply = (s: Sub) => treeLine(o.reportFrom?.(s.title) ?? briefDir(s.prompt)) + (o.output?.(s.title) ?? cleanReply(s.title));
|
||||
const advance = () => {
|
||||
for (const s of subs.values()) {
|
||||
if (s.late && !s.summary) { s.summary = o.output?.(s.title) ?? cleanReply(s.title); continue; }
|
||||
if (s.late && !s.summary) { s.summary = reply(s); continue; }
|
||||
if (s.status !== 'working') continue;
|
||||
if (++s.polls < (o.pollsToFinish ?? 1)) continue;
|
||||
if (o.fail?.(s.title)) s.status = 'failed';
|
||||
else if (o.lateSummary && !s.late) { s.status = 'done'; s.late = true; }
|
||||
else { s.status = 'done'; s.summary = o.output?.(s.title) ?? cleanReply(s.title); }
|
||||
else { s.status = 'done'; s.summary = reply(s); }
|
||||
}
|
||||
};
|
||||
const tools = {
|
||||
@@ -320,7 +329,7 @@ describe('Capy pass program (fake host, real bins)', () => {
|
||||
const h = host(dir, env, { output: t => (t.endsWith('audit/coverage') ? 'Coverage: 80% (one test added)' : 'NO FINDINGS') });
|
||||
const res = await runPass(h, { base: 'main', out, pass: 'x8p1', planFlags: ['--depth-floor', 'full'], extras: path.join(out, 'extras.json') });
|
||||
expect(h.creates[0]!.title).toBe('ship x8p1 audit/coverage');
|
||||
expect(h.creates[0]!.prompt).toBe('Run Step 7 as written.\n');
|
||||
expect(h.creates[0]!.prompt).toBe(withWorkspace('Run Step 7 as written.\n', fs.realpathSync(dir)));
|
||||
expect(res).toContain('Coverage: 80% (one test added)');
|
||||
expect(fs.readFileSync(path.join(out, 'merged.jsonl'), 'utf8')).not.toContain('Coverage');
|
||||
}, 240_000);
|
||||
@@ -422,7 +431,7 @@ describe('Capy pass program (fake host, real bins)', () => {
|
||||
extras: [{ name: 'qa', prompt_file: path.join(out, 'qa.prompt.md'), model: { model_id: 'anthropic/claude-haiku-5-5', reasoning_mode: 'low' } }] });
|
||||
expect(res).not.toContain('STOP:');
|
||||
expect(res).toContain('NEXT: judge (Phase 4)');
|
||||
expect(h.creates.find(c => c.title === 'ship x13p1 audit/qa')!.prompt).toBe('QA brief');
|
||||
expect(h.creates.find(c => c.title === 'ship x13p1 audit/qa')!.prompt).toBe(withWorkspace('QA brief', fs.realpathSync(dir)));
|
||||
}, 180_000);
|
||||
|
||||
test('a plan with no reviewer pass dispatches nothing and passes NEXT through', async () => {
|
||||
@@ -460,3 +469,122 @@ describe('the Capy skill loader', () => {
|
||||
}, 180_000);
|
||||
});
|
||||
|
||||
|
||||
describe('readers on a shared machine review the checkout the pass ships (GST-482)', () => {
|
||||
/** The primary checkout on feat, and a worktree of it on another branch with one more commit: the GBRA-218 layout. */
|
||||
function worktree() {
|
||||
const { dir, env } = repo();
|
||||
const wt = path.join(tmp('rp-wt-'), 'wt');
|
||||
const sh = (c: string, cwd = dir) => { const r = spawnSync('bash', ['-c', c], { cwd, env, encoding: 'utf8', timeout: 60_000 }); if (r.status !== 0) throw new Error(c + ': ' + r.stderr); return r.stdout.trim(); };
|
||||
sh(`git worktree add -q -b allow ${JSON.stringify(wt)} feat`);
|
||||
sh('for i in $(seq 1 20); do echo "export const w$i = $i;" >> src/a.ts; done && git commit -qam allow', wt);
|
||||
return { dir, env, wt: fs.realpathSync(wt) };
|
||||
}
|
||||
// Value: protects=a reader that never left the machine's default checkout cannot hand /ship a review of another tree; fails_when=collect merges a reply whose REVIEWED_TREE names the primary checkout while the pass ships a worktree, or a reply with no such line;
|
||||
// why_new=GBRA-218: pass 1's specialist reviewed /workspace/gbrain while /ship shipped ~/.capy/work/wt-allow; seam=gstack-review-dispatch collect; no_claim=a reader that fingerprints the right checkout but reads another
|
||||
test('a reader that reviews the primary checkout instead of the worktree is refused and nothing it said is merged', async () => {
|
||||
const { dir, env, wt } = worktree();
|
||||
const out = path.join(wt, '.state/run/p1');
|
||||
const h = host(dir, env, { reportFrom: () => dir, output: t => (t.includes('specialist') ? '[CRITICAL] (confidence: 9/10) src/other.ts:3 a file outside this PR' : cleanReply(t)) });
|
||||
const res = await runPass(h, { base: 'main', out, pass: 'w1p1', cwd: wt, planFlags: ['--depth-floor', 'full'] });
|
||||
expect(res).toContain('COLLECT_WRONG_TREE: reader=');
|
||||
expect(res).toContain(`reported="${treeLine(dir).slice(15).trim()}"`);
|
||||
expect(res).toMatch(/STOP: collect \S+ exited 1/);
|
||||
expect(res).not.toContain('NEXT: judge');
|
||||
expect(fs.existsSync(path.join(out, 'merged.jsonl')) ? fs.readFileSync(path.join(out, 'merged.jsonl'), 'utf8') : '').toBe('');
|
||||
const v = bin(wt, env, ['verify', '--ledger', path.join(out, 'review-ledger.jsonl')]).stdout;
|
||||
expect(v).toMatch(/^OUTPUT \S+ NOT_REVIEWED .*wrong_tree/m);
|
||||
expect(v).toContain('VERIFY: dirty');
|
||||
}, 240_000);
|
||||
|
||||
test('a reply that names no checkout is refused the same way', async () => {
|
||||
const { dir, env, wt } = worktree();
|
||||
const out = path.join(wt, '.state/run/p1');
|
||||
const res = await runPass(host(dir, env, { reportFrom: () => '' }), { base: 'main', out, pass: 'w2p1', cwd: wt, planFlags: ['--depth-floor', 'full'] });
|
||||
expect(res).toContain('COLLECT_WRONG_TREE: reader=');
|
||||
expect(res).toContain('reported="none"');
|
||||
expect(res).not.toContain('NEXT: judge');
|
||||
}, 240_000);
|
||||
|
||||
// Value: protects=/ship from a worktree on a shared machine works when readers follow their brief, though they start in another checkout; fails_when=a brief does not name the shipped checkout, or a correct reply is refused;
|
||||
// why_new=GBRA-218 (no brief named a directory); seam=lib/review-pass.ts renderStage and the Capy pass program; no_claim=model compliance with the paragraph
|
||||
test('a reader that works in the checkout its brief names passes, and every brief names the worktree', async () => {
|
||||
const { dir, env, wt } = worktree();
|
||||
const out = path.join(wt, '.state/run/p1');
|
||||
fs.mkdirSync(out, { recursive: true });
|
||||
fs.writeFileSync(path.join(out, 'coverage.prompt.md'), 'Run Step 7 as written.\n');
|
||||
const h = host(dir, env);
|
||||
const res = await runPass(h, { base: 'main', out, pass: 'w3p1', cwd: wt, planFlags: ['--depth-floor', 'full'],
|
||||
extras: [{ name: 'coverage', prompt_file: path.join(out, 'coverage.prompt.md'), first: true }] });
|
||||
expect(res).not.toContain('STOP:');
|
||||
expect(res).toContain('NEXT: judge (Phase 4)');
|
||||
expect(h.creates.length).toBeGreaterThan(2);
|
||||
for (const c of h.creates) expect(c.prompt.endsWith('\n\n' + withWorkspaceTail(wt)), c.title).toBe(true);
|
||||
expect(bin(wt, env, ['verify', '--ledger', path.join(out, 'review-ledger.jsonl')]).stdout).not.toContain('wrong_tree');
|
||||
}, 240_000);
|
||||
|
||||
test('an audit reader that worked in the primary checkout is refused when it is collected', async () => {
|
||||
const { dir, env, wt } = worktree();
|
||||
const out = path.join(wt, '.state/run/p1');
|
||||
fs.mkdirSync(out, { recursive: true });
|
||||
fs.writeFileSync(path.join(out, 'docs.prompt.md'), 'Run the Step 14.5 audit as written.\n');
|
||||
const h = host(dir, env, { reportFrom: t => (t.endsWith('audit/docs') ? dir : briefDirOf(h, t)) });
|
||||
const res = await runPass(h, { base: 'main', out, pass: 'w4p1', cwd: wt, planFlags: ['--depth-floor', 'full'],
|
||||
extras: [{ name: 'docs', prompt_file: path.join(out, 'docs.prompt.md'), first: true }] });
|
||||
expect(res).toContain('COLLECT_WRONG_TREE: reader=audit/docs');
|
||||
expect(res).not.toContain('NEXT: judge');
|
||||
expect(h.creates.map(c => c.title)).toEqual(['ship w4p1 audit/docs']);
|
||||
}, 240_000);
|
||||
|
||||
test('a rerun after a wrong-tree STOP stops on the failed reader and never collects its reply twice', async () => {
|
||||
const { dir, env, wt } = worktree();
|
||||
const out = path.join(wt, '.state/run/p1');
|
||||
const h = host(dir, env, { reportFrom: () => dir });
|
||||
const input = { base: 'main', out, pass: 'w5p1', cwd: wt, planFlags: ['--depth-floor', 'full'] };
|
||||
expect(await runPass(h, input)).toContain('COLLECT_WRONG_TREE: reader=');
|
||||
const again = await runPass(h, input);
|
||||
expect(again).toMatch(/STOP: \S+ \(subagent \d+\) reviewed another tree: [0-9a-f]{40} /);
|
||||
const outputs = fs.readFileSync(path.join(out, 'review-ledger.jsonl'), 'utf8').trim().split('\n').map(l => JSON.parse(l)).filter(e => e.event === 'output');
|
||||
expect(outputs.length).toBe(1);
|
||||
}, 240_000);
|
||||
|
||||
test('a worktree at the same commit as the primary checkout is still told apart by its toplevel', async () => {
|
||||
const { dir, env } = repo();
|
||||
const wt = path.join(tmp('rp-wt-'), 'same');
|
||||
expect(spawnSync('git', ['worktree', 'add', '-q', '--detach', wt, 'feat'], { cwd: dir, env, encoding: 'utf8', timeout: 60_000 }).status).toBe(0);
|
||||
const out = path.join(wt, '.state/run/p1');
|
||||
const res = await runPass(host(dir, env, { reportFrom: () => dir }), { base: 'main', out, pass: 'w6p1', cwd: fs.realpathSync(wt), planFlags: ['--depth-floor', 'full'] });
|
||||
expect(res).toContain('COLLECT_WRONG_TREE: reader=');
|
||||
expect(res).not.toContain('COLLECT_TREE_MOVED');
|
||||
}, 240_000);
|
||||
|
||||
test('a commit between render and reply is COLLECT_TREE_MOVED, not a pass', async () => {
|
||||
const { dir, env, wt } = worktree();
|
||||
const out = path.join(wt, '.state/run/p1');
|
||||
let moved = false;
|
||||
const h = host(dir, env, { reportFrom: t => {
|
||||
if (!moved) { moved = true; spawnSync('bash', ['-c', 'echo "export const late = 1;" >> src/a.ts && git commit -qam late'], { cwd: wt, env, encoding: 'utf8', timeout: 60_000 }); }
|
||||
return briefDirOf(h, t);
|
||||
} });
|
||||
const res = await runPass(h, { base: 'main', out, pass: 'w7p1', cwd: wt, planFlags: ['--depth-floor', 'full'] });
|
||||
expect(res).toContain('COLLECT_TREE_MOVED: reader=');
|
||||
expect(res).not.toContain('NEXT: judge');
|
||||
}, 240_000);
|
||||
|
||||
test('an outside voice that reviewed another tree is recorded as unavailable coverage, never clean, and the voices still finish', async () => {
|
||||
const { dir, env, wt } = worktree();
|
||||
const out = path.join(wt, '.state/run/p1');
|
||||
const h = host(dir, env, { reportFrom: t => (t.endsWith('outside-voice') ? dir : briefDirOf(h, t)) });
|
||||
const base = { base: 'main', out, pass: 'w8p1', cwd: wt, planFlags: ['--depth-floor', 'full'] };
|
||||
expect(await runPass(h, base)).toContain('NEXT: judge (Phase 4)');
|
||||
const models = { 'outside-voice': { model_id: 'openai/gpt-6.1-sol' }, 'adversarial-native': { model_id: 'anthropic/claude-opus-5-5', reasoning_mode: 'high' } };
|
||||
const res = await runPass(h, { ...base, stage: 'voices', models });
|
||||
expect(res).toContain('COLLECT_UNAVAILABLE: reader=outside-voice');
|
||||
expect(res).toContain('reviewed another tree');
|
||||
expect(res).toContain('NEXT: judge the voices');
|
||||
const v = bin(wt, env, ['verify', '--ledger', path.join(out, 'review-ledger.jsonl')]).stdout;
|
||||
expect(v).toMatch(/^BRIEF outside-voice UNAVAILABLE /m);
|
||||
}, 240_000);
|
||||
});
|
||||
const withWorkspaceTail = (dir: string) => withWorkspace('', dir).slice(2);
|
||||
const briefDirOf = (h: ReturnType<typeof host>, title: string) => briefDir(h.creates.find(c => c.title === title)?.prompt ?? '');
|
||||
@@ -18,6 +18,7 @@ import { carryReceipt, extractReceipt, renderReceiptBlock } from '../lib/ship-re
|
||||
import { bindShippedReceipt, parseReviewTargeted, receiptTargetedLine } from '../lib/ship-receipt-depth';
|
||||
import type { ShipReceipt } from '../lib/headless-artifacts';
|
||||
import { cleanup, makeFixtureRepo, runBin, type FixtureRepo } from './helpers/restamp-fixture';
|
||||
import { checkoutTree } from '../lib/review-brief';
|
||||
|
||||
const ROOT = path.resolve(import.meta.dir, '..');
|
||||
const PLAN = path.join(ROOT, 'bin', 'gstack-review-plan');
|
||||
@@ -268,7 +269,7 @@ describe('the targeted role end to end on a fixture repository', () => {
|
||||
expect(runBin(repo, 'gstack-review-dispatch', ['sent', '--ledger', ledger, '--role', 'targeted', '--sent', file]).out).toContain('BRIEF_SENT: reader=targeted');
|
||||
const output = path.join(repo.root, 'targeted.out');
|
||||
const finding = { severity: 'CRITICAL', confidence: 8, path: 'src/charge.ts', line: 1, category: 'money', summary: 'cap is never enforced', fingerprint: 'src/charge.ts:1:money', specialist: 'targeted' };
|
||||
fs.writeFileSync(output, JSON.stringify(finding) + '\n');
|
||||
fs.writeFileSync(output, `REVIEWED_TREE: ${checkoutTree(repo.work)}\n` + JSON.stringify(finding) + '\n');
|
||||
const merged = path.join(repo.root, 'merged.jsonl');
|
||||
expect(runBin(repo, 'gstack-review-dispatch', ['collect', '--ledger', ledger, '--role', 'targeted', '--output', output, '--merged', merged]).out).toContain('COLLECT: reader=targeted findings=1');
|
||||
const judged = path.join(repo.root, 'judged.jsonl');
|
||||
|
||||
Reference in new issue
Block a user