mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-01 14:34:42 +01:00
fix: targeted reader reads the merge base on every pass; dirty-tree record test
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
db2b5b00ab
commit
a162c42fcb
3 files changed
+15
-2
No files matched your search
@@ -18,6 +18,7 @@ import { EXIT, exitTableHelp } from '../lib/headless-artifacts';
|
||||
import { BRIEF_ROLES, BriefError, briefFile, promptHash, renderBrief, sectionTexts, type BriefRole, type Slots } from '../lib/review-brief';
|
||||
import { collect, readJsonl, readLedger, readerKey, recordJudgment, recordSent, appendEvent, verify, type JudgedLine, type MergedLine, type VerifyRow } from '../lib/review-ledger';
|
||||
import { TARGETED_RULES, parseClassList, targetedBriefSha, targetedPatchId } from '../lib/review-targeted';
|
||||
import { resolveBaseBranch } from '../lib/ship-policy';
|
||||
|
||||
const HELP = `gstack-review-dispatch — verbatim reviewer briefs and loss-free collection for /ship
|
||||
|
||||
@@ -27,7 +28,8 @@ const HELP = `gstack-review-dispatch — verbatim reviewer briefs and loss-free
|
||||
--learnings-file <f> --checklist <path> (default <root>/review/specialists/<name>.md; red-team.md for red-team)
|
||||
--checklists <a,b> (specialist-batched: the plan's checklists= names, or paths) --findings-file <f> --specialists <n> (red-team)
|
||||
--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)
|
||||
--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
|
||||
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>
|
||||
@@ -61,6 +63,11 @@ function render(): number {
|
||||
const checklist = flag('--checklist') ?? (role === 'red-team' ? path.join(ROOT, 'review/specialists/red-team.md') : role === 'targeted' ? path.join(ROOT, 'review/specialists/targeted.md') : role === 'specialist' && name ? path.join(ROOT, 'review/specialists', `${name}.md`) : undefined);
|
||||
if (checklist && (role === 'specialist' || role === 'red-team' || role === 'targeted') && !fs.existsSync(checklist)) throw new BriefError(`checklist not found: ${checklist}`);
|
||||
const brief = role === 'targeted' ? readFile(checklist, 'targeted brief') : undefined;
|
||||
if (role === 'targeted') {
|
||||
const base = resolveBaseBranch(process.cwd(), flag('--base'));
|
||||
const mb = [`origin/${base}`, base].map(ref => spawnSync('git', ['merge-base', ref, 'HEAD'], { encoding: 'utf8', timeout: 30_000 })).find(r => r.status === 0)?.stdout.trim();
|
||||
if (!diffBase || diffBase !== mb) throw new BriefError(`the targeted reader reads the whole diff on every pass: --diff-base must be the merge base with ${base} (${mb ?? 'unresolved'}), got ${diffBase ?? 'none'}`);
|
||||
}
|
||||
const checklists = flag('--checklists')?.split(',').map(s => s.trim()).filter(Boolean).map(c => (c.includes('/') ? c : path.join(ROOT, 'review/specialists', `${c}.md`)));
|
||||
for (const c of checklists ?? []) if (!fs.existsSync(c)) throw new BriefError(`checklist not found: ${c}`);
|
||||
let diff: string | undefined;
|
||||
|
||||
@@ -313,7 +313,9 @@ full review.
|
||||
|
||||
The plan prints `TARGETED: classes=<money,gate,auth,secret|none> rules=<hash>
|
||||
brief=<sha256 of the brief> targeted_patch_id=<id>` and, when a class hit,
|
||||
`DISPATCH: targeted targeted effort=full`. After a fix, pass
|
||||
`DISPATCH: targeted targeted effort=full`. The targeted reader always reads the whole diff: `render --role targeted`
|
||||
refuses a `--diff-base` other than the merge base, so on pass 2 and later it
|
||||
gets the merge base, not `RANGE_BASE`. After a fix, pass
|
||||
`--targeted-ran <targeted_patch_id>` from the line the reader ran on: the same
|
||||
committed patch skips the reader, and a changed one, uncommitted edits or
|
||||
`none` dispatch it again. `targeted_patch_id` is the patch-id of the whole
|
||||
|
||||
@@ -263,6 +263,7 @@ describe('the targeted role end to end on a fixture repository', () => {
|
||||
expect(brief).toContain(`DIFF_BASE=${base} && git diff "$DIFF_BASE"`);
|
||||
expect(brief).toContain('Read the targeted brief above (');
|
||||
expect(runBin(repo, 'gstack-review-dispatch', ['render', '--role', 'targeted', '--classes', 'cost', '--diff-base', base, '--out', path.join(repo.root, 'bad')]).err).toContain('BRIEF_RENDER_FAILED: slot classes must list');
|
||||
expect(runBin(repo, 'gstack-review-dispatch', ['render', '--role', 'targeted', '--classes', 'money', '--diff-base', head, '--out', path.join(repo.root, 'range')]).err).toContain(`the targeted reader reads the whole diff on every pass: --diff-base must be the merge base with main (${base})`);
|
||||
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' };
|
||||
@@ -281,6 +282,9 @@ describe('the targeted role end to end on a fixture repository', () => {
|
||||
repo.git('commit', '-qam', 'later product change the reader never read');
|
||||
expect(runBin(repo, 'gstack-review-dispatch', ['verify', '--ledger', ledger]).out).toContain('TARGETED_RECORD: classes=money status=failed findings=0');
|
||||
repo.git('reset', '-q', '--hard', head);
|
||||
fs.writeFileSync(path.join(repo.work, 'src', 'charge.ts'), 'export const spend_cap_usd = 7;\n');
|
||||
expect(runBin(repo, 'gstack-review-dispatch', ['verify', '--ledger', ledger]).out).toContain('TARGETED_RECORD: classes=money status=failed findings=0');
|
||||
repo.git('checkout', '--', 'src/charge.ts');
|
||||
expect(runBin(repo, 'gstack-review-dispatch', ['verify', '--ledger', ledger]).out).toContain('TARGETED_RECORD: classes=money status=done findings=1');
|
||||
|
||||
const body = path.join(repo.root, 'body.md');
|
||||
|
||||
Reference in new issue
Block a user