diff --git a/lib/review-depth.ts b/lib/review-depth.ts index bce1fffef..6914a39c3 100644 --- a/lib/review-depth.ts +++ b/lib/review-depth.ts @@ -273,6 +273,7 @@ const forcedBy = (i: PlanInput) => (n: string) => i.force === 'all' || (Array.is /** The specialists a full pass selects for a class: class sets the eligible set, hit-rate gating applies inside it, gate class ignores gating. */ export function specialistsFor(cls: DiffClass, i: PlanInput, skipped: string[] = []): Specialist[] { const s = i.scope ?? {}; + const unknown = !!s.SCOPE_ERROR; const forced = forcedBy(i); if (cls === 'docs') { skipped.push('specialists (docs class)'); return SPECIALISTS.filter(forced); } if (cls === 'tests') { skipped.push('specialists except testing (tests class)'); return SPECIALISTS.filter(n => n === 'testing' || forced(n)); } @@ -282,9 +283,9 @@ export function specialistsFor(cls: DiffClass, i: PlanInput, skipped: string[] = if (i.diffLines < SMALL_DIFF_LINES) return false; switch (n) { case 'testing': case 'maintainability': return true; - case 'security': return s.SCOPE_AUTH === 'true' || (s.SCOPE_BACKEND === 'true' && i.diffLines > LARGE_DIFF_LINES); + case 'security': return unknown || s.SCOPE_AUTH === 'true' || (s.SCOPE_BACKEND === 'true' && i.diffLines > LARGE_DIFF_LINES); case 'simplification': return i.diffLines > LARGE_DIFF_LINES; - default: return scopeWants[n]!(s); + default: return unknown || scopeWants[n]!(s); } }; return SPECIALISTS.filter(n => { diff --git a/lib/review-plan-live.ts b/lib/review-plan-live.ts index 712beee09..3d04c072b 100644 --- a/lib/review-plan-live.ts +++ b/lib/review-plan-live.ts @@ -8,7 +8,7 @@ import * as path from 'node:path'; import { spawnSync } from 'node:child_process'; import { runScriptSync } from './script-invocation'; -import { collectCommit, collectLive, type Collected } from './review-collect'; +import { collectCommit, collectLive, resolveBaseRef, type Collected } from './review-collect'; import { classifyDiff, planReview, scanNumbers, SCOPED_RANGE_LINES, type Depth, type History, type ReviewPlan } from './review-depth'; export interface PlanFlags { @@ -90,7 +90,7 @@ export function livePlan(f: PlanFlags, binDir: string, bin: string): LivePlan | const m = /GATE_SUMMARY: .*?tagged=\{([^}]*)\}/.exec(g.stdout ?? ''); return g.status === 0 && m ? m[1]!.split(',').reduce((n, kv) => n + (Number(kv.split(':')[1]) || 0), 0) : null; }; - const scope = scopeOf([f.base]); + const scope = scopeOf([resolveBaseRef(root, f.base)]); let gateTagged = f.gateTagged !== undefined ? f.gateTagged : gateTagsOf([f.base]); if (gateTagged !== null && !Number.isFinite(gateTagged)) gateTagged = null; const tiersSetting = runScriptSync(path.join(binDir, 'gstack-config'), ['get', 'review_effort_tiers'], { encoding: 'utf8', timeout: 15_000 }); diff --git a/test/review-depth.test.ts b/test/review-depth.test.ts index d7f10c1ab..323175d21 100644 --- a/test/review-depth.test.ts +++ b/test/review-depth.test.ts @@ -11,6 +11,15 @@ const none = scanNumbers({}); const base = (o: Partial): PlanInput => ({ paths: ['src/app.ts'], scope: { SCOPE_BACKEND: 'true', SCOPE_STATE_MACHINE: 'false' }, gateTagged: 0, diffLines: 120, history: { fullPasses: 0, last: 'none' }, numbers: none, ...o }); const roles = (p: ReturnType) => p.dispatches.map(d => `${d.role}:${d.name}:${d.effort}`); +describe('a scope error fails the specialist selection closed', () => { + // Value: protects=an unreadable diff scope selects every scope-gated specialist instead of none; fails_when=SCOPE_ERROR leaves all SCOPE_* false and security/performance drop silently; + // why_new=GSTA-167 dogfood on #3633 (no local main); seam=none; no_claim=the scope tool's own errors + test('SCOPE_ERROR=no_base keeps security, performance, api-contract, data-migration and design', () => { + const names = planReview(base({ scope: { SCOPE_ERROR: 'no_base' }, diffLines: 1000 })).dispatches.filter(d => d.role === 'specialist').map(d => d.name); + for (const n of ['security', 'performance', 'api-contract', 'data-migration', 'design']) expect(names).toContain(n); + }); +}); + describe('classifyPath: agent-read markdown is code', () => { test.each([ ['README.md', 'docs'], ['CHANGELOG.md', 'docs'], ['docs/howto-x.md', 'docs'], ['TODOS.md', 'docs'], diff --git a/test/review-plan-cli.test.ts b/test/review-plan-cli.test.ts index 1f7bc9128..ed804d350 100644 --- a/test/review-plan-cli.test.ts +++ b/test/review-plan-cli.test.ts @@ -41,6 +41,16 @@ describe('gstack-review-plan', () => { expect(plan.audit_effort).toBe('light'); expect(JSON.parse(run(r, ['--json', '--no-tiers']).stdout).audit_effort).toBe('host'); }); + // Value: protects=the plan reads diff scope from origin/ like collectLive, so a clone with no local base branch keeps its scope-gated specialists; fails_when=gstack-diff-scope fails on a missing local ref and security/performance silently drop; + // why_new=GSTA-167 dogfood on #3633: a Capy checkout has no local main; seam=gstack-diff-scope; no_claim=which specialists a scope selects + test('no local base branch: scope comes from origin/ and the scope-gated specialists stay', () => { + const r = repo({ 'src/a.ts': Array.from({ length: 80 }, (_, i) => `export const v${i} = ${i};`).join('\n') + '\n' }); + const git = (...a: string[]) => spawnSync('git', a, { cwd: r.dir, encoding: 'utf8', timeout: 10_000 }); + git('update-ref', 'refs/remotes/origin/main', 'main'); git('branch', '-D', 'main'); + const out = run(r, []).stdout; + expect(out).not.toContain('scope_error'); + expect(out).toContain('DISPATCH: specialist performance'); + }); test('a below-floor fix after pass 1 plans one fix-check', () => { const r = run(repo({ 'src/a.ts': 'export const a = 1;\n' }), ['--pass', '1', '--last', 'full', '--fixed-below']); expect(r.stdout).toContain('next=fix-check dispatches=1');