From fc5ef06e13c9c09334720086831a4983e32f63e2 Mon Sep 17 00:00:00 2001 From: garrytan Date: Thu, 29 Oct 2026 08:53:42 +0000 Subject: [PATCH] fix: classify the fix range and the diff at the range base with their own scope and gate scan (GSTA-152 PR B pass 2) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- CHANGELOG.md | 2 +- bin/gstack-review-plan | 30 ++++++++++++++++++------------ docs/review-policy.md | 5 +++-- test/review-plan-cli.test.ts | 20 +++++++++++++++++++- 4 files changed, 41 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f6253ddb..e5af2dcce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ - **Docs**: docs/ship-budget.md (linked from docs/unattended.md), and a paragraph each in the Capy skill's SKILL.md and captain.md. ### Changed -- **`gstack-review-plan --range-base `** (`lib/review-depth.ts`): counts the committed fix range `..HEAD` the way the depth counts runtime lines, prints `FIX_RANGE: runtime_lines=|unread () base=` (`fix_range` in `--json`), and `NEXT: scoped` when the scoped pass applies. `nextStep` takes the depth and `History.rangeRuntimeLines`; `SCOPED_RANGE_LINES` (150) joins `DEPTH_RULES`, so `rules=` changes. `REVIEW_PLAN:` ends with the step's `reason="…"`. Uncommitted or untracked edits, a range base that is not an ancestor of HEAD, an empty range, and a range the classifier cannot count all plan the full pass. A range that touches a gate-class path (a revert of a gate edit included) plans the full pass without `--touched-gate` and sets `fix_range.touched_gate`. +- **`gstack-review-plan --range-base `** (`lib/review-depth.ts`): counts the committed fix range `..HEAD` the way the depth counts runtime lines, prints `FIX_RANGE: runtime_lines=|unread () base=` (`fix_range` in `--json`), and `NEXT: scoped` when the scoped pass applies. `nextStep` takes the depth and `History.rangeRuntimeLines`; `SCOPED_RANGE_LINES` (150) joins `DEPTH_RULES`, so `rules=` changes. `REVIEW_PLAN:` ends with the step's `reason="…"`. Uncommitted or untracked edits, a range base that is not an ancestor of HEAD, an empty range, and a range the classifier cannot count all plan the full pass. The range and the diff at the range base are classified with their own `gstack-diff-scope` and `gstack-gate-diff` input; when either is gate class (a revert of a gate, auth or migration edit included) the plan is the full pass without `--touched-gate` and sets `fix_range.touched_gate`. - **Scoped dispatches**: the fix-check, every force-included specialist, the Red Team, the native adversarial pass when the range changes runtime lines, and the outside voice when published numbers changed. Step 11's voices that have not run yet stay listed. A light diff's raise to standard applies to the scoped set only. - **Step 9** (`ship/sections/review-army.md`): the repeat decision passes `--range-base $PASS_HEAD` and maps `NEXT: scoped` to decision 3; a scoped pass logs a full-pass `review` row with `"scope":"scoped"`, so it counts toward the cap and its roles land in `review_loop.dispatches` with no receipt schema change. - **`raised_by` on classified findings** (`gstack-review-scope classify`, `lib/review-scope.ts`): the reviewer that raised each finding (`core`, `qa`, a specialist, `red-team`, `adversarial-native`, `outside-voice`, `fix-check`), from the finding's `raised_by` or `specialist` tag, else `unknown`. Step 9 copies it into the review-log row. Older rows without it still summarize. diff --git a/bin/gstack-review-plan b/bin/gstack-review-plan index 59df799af..03c8bc048 100755 --- a/bin/gstack-review-plan +++ b/bin/gstack-review-plan @@ -91,17 +91,20 @@ function main(): number { } const head = git(['rev-parse', 'HEAD'])?.trim() ?? 'unknown'; const binDir = path.dirname(Bun.fileURLToPath(import.meta.url)); - const scopeRun = runScriptSync(path.join(binDir, 'gstack-diff-scope'), [base], { encoding: 'utf8', timeout: 60_000 }); - const scope: Record = {}; - for (const l of (scopeRun.stdout ?? '').split('\n')) { const m = /^(SCOPE_[A-Z_]+)=(.*)$/.exec(l); if (m) scope[m[1]!] = m[2]!; } - if (scopeRun.status !== 0 && !scope.SCOPE_ERROR) scope.SCOPE_ERROR = `exit ${scopeRun.status}`; - let gateTagged: number | null = null; - if (flag('--gate-tagged') !== undefined) gateTagged = Number(flag('--gate-tagged')); - else { - const g = runScriptSync(path.join(binDir, 'gstack-gate-diff'), [base], { encoding: 'utf8', timeout: 120_000, maxBuffer: 16 * 1024 * 1024 }); + const scopeOf = (args: string[]): Record => { + const run = runScriptSync(path.join(binDir, 'gstack-diff-scope'), args, { encoding: 'utf8', timeout: 60_000 }); + const s: Record = {}; + for (const l of (run.stdout ?? '').split('\n')) { const m = /^(SCOPE_[A-Z_]+)=(.*)$/.exec(l); if (m) s[m[1]!] = m[2]!; } + if (run.status !== 0 && !s.SCOPE_ERROR) s.SCOPE_ERROR = `exit ${run.status}`; + return s; + }; + const gateTagsOf = (args: string[]): number | null => { + const g = runScriptSync(path.join(binDir, 'gstack-gate-diff'), args, { encoding: 'utf8', timeout: 120_000, maxBuffer: 16 * 1024 * 1024 }); const m = /GATE_SUMMARY: .*?tagged=\{([^}]*)\}/.exec(g.stdout ?? ''); - gateTagged = g.status === 0 && m ? m[1]!.split(',').reduce((n, kv) => n + (Number(kv.split(':')[1]) || 0), 0) : null; - } + return g.status === 0 && m ? m[1]!.split(',').reduce((n, kv) => n + (Number(kv.split(':')[1]) || 0), 0) : null; + }; + const scope = scopeOf([base]); + let gateTagged = flag('--gate-tagged') !== undefined ? Number(flag('--gate-tagged')) : gateTagsOf([base]); if (gateTagged !== null && !Number.isFinite(gateTagged)) gateTagged = null; const force = flag('--force') === 'all' ? 'all' as const : list('--force'); const tiersSetting = runScriptSync(path.join(binDir, 'gstack-config'), ['get', 'review_effort_tiers'], { encoding: 'utf8', timeout: 15_000 }); @@ -116,10 +119,13 @@ function main(): number { else if (git(['diff', '--quiet', 'HEAD', '--']) === null || collected.untracked.length) fixRange = unread('uncommitted or untracked edits: commit the fixes first'); else { const commit = collectCommit(root, resolved, 'HEAD'); - const range = classifyDiff({ ...commit, registry, gateTagged: 0 }); + const range = classifyDiff({ ...commit, registry, scope: scopeOf([resolved, '--head', 'HEAD']), gateTagged: gateTagsOf([resolved]) }); + const baseSha = git(['merge-base', base, resolved])?.trim(); + const atBase = baseSha ? classifyDiff({ ...collectCommit(root, baseSha, resolved), registry, scope: scopeOf([base, '--head', resolved]), gateTagged: gateTagsOf([base, '--commit', resolved]) }) : undefined; fixRange = range.errors.length ? unread(range.errors.map(e => `${e.code} ${e.input}`).join('; ')) : !commit.paths.length ? unread('the fix range is empty: commit the fix after the range base') - : { base: rangeBase, runtime_lines: range.runtimeLines, ...(range.class === 'gate' ? { touched_gate: true as const } : {}) }; + : !atBase || atBase.errors.length ? unread('the diff at the range base could not be classified') + : { base: rangeBase, runtime_lines: range.runtimeLines, ...(range.class === 'gate' || atBase.class === 'gate' ? { touched_gate: true as const } : {}) }; } } const plan = planReview({ diff --git a/docs/review-policy.md b/docs/review-policy.md index a51cc458e..40a9b7c51 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -134,8 +134,9 @@ force-included specialist and the outside voice when published numbers changed. Step 11's voices that have not run yet stay listed, as on a full pass 2. It writes a full-pass `review` row with `"scope":"scoped"`, so it counts toward the cap, and a blocker it leaves open stops for the owner as before. A fix that -touched a gate-class path (read from the range itself, so a revert of a gate -edit counts without `--touched-gate`), a larger range, an empty range, or a +touched a gate-class path (read from the range and from the diff at the range +base, each with its own scope and gate scan, so a revert of a gate, auth or +migration edit counts without `--touched-gate`), a larger range, an empty range, or a range, classifier or depth it cannot read keeps the full pass 2. The 150-line figure is a starting value from a 24-PR backtest (GSTA-152 D6), to revisit once classified findings carry `raised_by`. diff --git a/test/review-plan-cli.test.ts b/test/review-plan-cli.test.ts index 346e59431..0d1c4c0e2 100644 --- a/test/review-plan-cli.test.ts +++ b/test/review-plan-cli.test.ts @@ -206,7 +206,25 @@ describe('gstack-review-plan: the scoped second pass (GSTA-152 D6)', () => { expect(out).toMatch(/^REVIEW_DEPTH: standard /m); expect(out).toMatch(/^NEXT: full \(a blocking finding was fixed and a fix touched a gate-class path/m); expect(JSON.parse(run(r, [...pass2, '--range-base', rangeBase, '--json']).stdout)).toMatchObject({ next: { step: 'full' }, fix_range: { touched_gate: true } }); - }); + }, 30_000); + test.each([ + ['an auth revert', 'src/auth/login.ts', 'export const login = (u: string) => u.length > 0;\n'], + ['a migration revert', 'db/migrate/001_a.sql', 'ALTER TABLE users ADD COLUMN email text;\n'], + ])('%s keeps the full pass 2: the range and the diff at the range base carry scope and gate-scan input', (_n, file, text) => { + const r = repo({ 'src/a.ts': code(80) }); + const git = (...a: string[]) => spawnSync('git', a, { cwd: r.dir, encoding: 'utf8', timeout: 10_000 }); + const write = (body: string) => { fs.mkdirSync(path.dirname(path.join(r.dir, file)), { recursive: true }); fs.writeFileSync(path.join(r.dir, file), body); }; + git('checkout', '-q', 'main'); write(text); git('add', '.'); git('commit', '-qm', 'base file'); + git('checkout', '-q', 'feat'); git('merge', '-q', '--no-edit', 'main'); + write(text + '-- edited\n'); git('add', '.'); git('commit', '-qm', 'feat edit'); + const rangeBase = git('rev-parse', 'HEAD').stdout.trim(); + write(text); + fs.writeFileSync(path.join(r.dir, 'src', 'b.ts'), code(2, 'b')); + git('add', '.'); git('commit', '-qm', 'fix: revert the edit'); + const out = run(r, [...pass2, '--range-base', rangeBase]).stdout; + expect(out).not.toMatch(/^REVIEW_CLASS: gate/m); + expect(out).toMatch(/^NEXT: full \(a blocking finding was fixed and a fix touched a gate-class path/m); + }, 30_000); test('--depth-floor full opts out of the scoped pass', () => { const { r, rangeBase } = fixed(10); expect(run(r, [...pass2, '--range-base', rangeBase, '--depth-floor', 'full']).stdout).toMatch(/^NEXT: full \(a blocking finding was fixed and the diff is at full depth/m);