mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-09 18:28:15 +01:00
fix: plan the full pass 2 when the fix range touches a gate path or is empty (GSTA-152 PR B review)
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
0be7c3d443
commit
2e8ed2a918
6 files changed
+33
-10
No files matched your search
+1
-1
@@ -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 <sha>`** (`lib/review-depth.ts`): counts the committed fix range `<sha>..HEAD` the way the depth counts runtime lines, prints `FIX_RANGE: runtime_lines=<n>|unread (<why>) base=<sha>` (`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, and a range the classifier cannot count all plan the full pass.
|
||||
- **`gstack-review-plan --range-base <sha>`** (`lib/review-depth.ts`): counts the committed fix range `<sha>..HEAD` the way the depth counts runtime lines, prints `FIX_RANGE: runtime_lines=<n>|unread (<why>) base=<sha>` (`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`.
|
||||
- **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.
|
||||
|
||||
@@ -107,7 +107,7 @@ function main(): number {
|
||||
const tiersSetting = runScriptSync(path.join(binDir, 'gstack-config'), ['get', 'review_effort_tiers'], { encoding: 'utf8', timeout: 15_000 });
|
||||
const auditEffort = has('--no-tiers') || (tiersSetting.stdout ?? '').trim() === 'off' ? 'host' : 'light';
|
||||
const { added, diffLines, paths, deleted, stampOnly, perPath, registry } = collected;
|
||||
let fixRange: { base: string; runtime_lines: number | null; error?: string } | undefined;
|
||||
let fixRange: { base: string; runtime_lines: number | null; error?: string; touched_gate?: true } | undefined;
|
||||
if (rangeBase) {
|
||||
const unread = (error: string) => ({ base: rangeBase, runtime_lines: null, error });
|
||||
const resolved = git(['rev-parse', '--verify', '--quiet', `${rangeBase}^{commit}`])?.trim();
|
||||
@@ -115,14 +115,17 @@ function main(): number {
|
||||
else if (git(['merge-base', '--is-ancestor', resolved, 'HEAD']) === null) fixRange = unread('not an ancestor of HEAD');
|
||||
else if (git(['diff', '--quiet', 'HEAD', '--']) === null || collected.untracked.length) fixRange = unread('uncommitted or untracked edits: commit the fixes first');
|
||||
else {
|
||||
const range = classifyDiff({ ...collectCommit(root, resolved, 'HEAD'), registry, gateTagged: 0 });
|
||||
fixRange = range.errors.length ? unread(range.errors.map(e => `${e.code} ${e.input}`).join('; ')) : { base: rangeBase, runtime_lines: range.runtimeLines };
|
||||
const commit = collectCommit(root, resolved, 'HEAD');
|
||||
const range = classifyDiff({ ...commit, registry, gateTagged: 0 });
|
||||
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 } : {}) };
|
||||
}
|
||||
}
|
||||
const plan = planReview({
|
||||
...collected, scope, gateTagged, numbers: scanNumbers(added), force, gated: list('--gated'), floor,
|
||||
criticalFound: has('--critical-found'), adversarialRan: has('--adversarial-ran'), reviewedNumbers: flag('--reviewed-numbers'),
|
||||
history: { fullPasses: pass, last, fixedBlocking: has('--fixed-blocking'), fixedBelow: has('--fixed-below'), blockingOpen: has('--blocking-open'), touchedGate: has('--touched-gate'), rangeRuntimeLines: fixRange?.runtime_lines },
|
||||
history: { fullPasses: pass, last, fixedBlocking: has('--fixed-blocking'), fixedBelow: has('--fixed-below'), blockingOpen: has('--blocking-open'), touchedGate: has('--touched-gate') || !!fixRange?.touched_gate, rangeRuntimeLines: fixRange?.runtime_lines },
|
||||
});
|
||||
const gateScan = { tagged: gateTagged, head };
|
||||
if (has('--json')) {
|
||||
|
||||
@@ -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, a larger range, or a range, classifier or depth it
|
||||
cannot read keeps the full pass 2. The 150-line figure is a starting value from
|
||||
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
|
||||
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`.
|
||||
|
||||
|
||||
+4
-2
@@ -354,14 +354,16 @@ export function planReview(i: PlanInput): ReviewPlan {
|
||||
}
|
||||
} else if (next.step === 'scoped') {
|
||||
add('fix-check', 'fix-check', 'full');
|
||||
for (const n of SPECIALISTS.filter(forcedBy(i))) add('specialist', n, 'full');
|
||||
const forced = SPECIALISTS.filter(forcedBy(i));
|
||||
for (const n of forced) add('specialist', n, 'full');
|
||||
skipped.push(forced.length ? 'specialists (scoped pass; forced ones dispatched)' : 'specialists (scoped pass)');
|
||||
add('red-team', 'red-team', 'full');
|
||||
const stepElevenPending = !i.adversarialRan && cls === 'code';
|
||||
if ((h.rangeRuntimeLines ?? 0) > 0 || stepElevenPending) add('adversarial-native', 'adversarial', 'full');
|
||||
else skipped.push('adversarial-native (scoped pass: the fix range changes no runtime lines)');
|
||||
if (stepElevenPending) add('outside-voice', 'outside-voice', 'host');
|
||||
else if (numbersChanged) add('outside-voice', 'outside-voice (numbers changed)', 'host');
|
||||
else skipped.push('specialists and outside-voice (scoped pass)');
|
||||
else skipped.push('outside-voice (scoped pass)');
|
||||
} else if (next.step === 'fix-check') {
|
||||
add('fix-check', 'fix-check', 'full');
|
||||
if (numbersChanged) add('outside-voice', 'outside-voice (numbers changed)', 'host');
|
||||
|
||||
@@ -307,7 +307,11 @@ describe('D6: the scoped second pass after a blocking fix (GSTA-152 PR B)', () =
|
||||
expect(p.dispatches.every(d => d.effort === 'full' && d.exclude.length === 0)).toBe(true);
|
||||
});
|
||||
test('Step 11 voices that have not run yet stay listed, as on a full pass 2', () => {
|
||||
expect(r(planReview(n19({ adversarialRan: false })))).toEqual(['fix-check:fix-check', 'red-team:red-team', 'adversarial-native:adversarial', 'outside-voice:outside-voice']);
|
||||
const p = planReview(n19({ adversarialRan: false }));
|
||||
expect(r(p)).toEqual(['fix-check:fix-check', 'red-team:red-team', 'adversarial-native:adversarial', 'outside-voice:outside-voice']);
|
||||
expect(p.skipped).toContain('specialists (scoped pass)');
|
||||
expect(planReview(n19()).skipped).toEqual(expect.arrayContaining(['specialists (scoped pass)', 'outside-voice (scoped pass)']));
|
||||
expect(planReview(n19({ force: ['security'] })).skipped).toContain('specialists (scoped pass; forced ones dispatched)');
|
||||
});
|
||||
test.each([
|
||||
['full depth (a gate tag)', { gateTagged: 1 }],
|
||||
|
||||
@@ -194,6 +194,19 @@ describe('gstack-review-plan: the scoped second pass (GSTA-152 D6)', () => {
|
||||
expect(dirty).toMatch(/^NEXT: full /m);
|
||||
expect(run(r, [...pass2, '--range-base']).status).toBe(2);
|
||||
});
|
||||
test('a fix range that touches a gate path keeps the full pass 2 without --touched-gate, and an empty range is unread', () => {
|
||||
const r = repo({ 'src/a.ts': code(80), '.github/workflows/ci.yml': 'on: push\n' });
|
||||
const git = (...a: string[]) => spawnSync('git', a, { cwd: r.dir, encoding: 'utf8', timeout: 10_000 });
|
||||
const rangeBase = git('rev-parse', 'HEAD').stdout.trim();
|
||||
expect(run(r, [...pass2, '--range-base', rangeBase]).stdout).toMatch(/^FIX_RANGE: unread \(the fix range is empty: .*\) base=/m);
|
||||
git('rm', '-q', '.github/workflows/ci.yml');
|
||||
fs.writeFileSync(path.join(r.dir, 'src', 'b.ts'), code(2, 'b'));
|
||||
git('add', '.'); git('commit', '-qm', 'fix: revert the workflow edit');
|
||||
const out = run(r, [...pass2, '--range-base', rangeBase]).stdout;
|
||||
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 } });
|
||||
});
|
||||
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);
|
||||
|
||||
Reference in new issue
Block a user