mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-09 02:08:19 +01:00
fix(review-log): fix-range base is the commit the fixing pass started from; follow Step 9.4's real order (#3339 review)
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
4e54d98c00
commit
a0b7ca7c6c
3 files changed
+113
-71
No files matched your search
+4
-4
@@ -5,18 +5,18 @@
|
||||
**/ship's Step 9 can re-review just the fixes after a fixing pass, and a narrowed pass can never be the review that ships (#3339).** Before, every fix cycle reran every specialist and the Red Team over the whole branch. On a large branch each full pass tends to find something new in code earlier passes already read, so the loop often ran to the three-cycle stop without converging. Now the pass after a fixing pass can review only what changed since that pass. One full pass still has to come last: a fix-range record is never CURRENT, and Step 11.5 can't bind it. Garry approved this on 2026-10-24 ("only with a mandatory final full pass").
|
||||
|
||||
### Added
|
||||
- **`bin/gstack-review-log --start review --fix-range`** prints the start token, then `FIX_BASE=<sha>`. The base comes from the review log, never from the caller or the working tree, so `gstack-wtree`'s fingerprint is untouched. It is the commit of the latest pass of the same skill. The start refuses (exit 1, "run a full pass") in each of these cases: there is no earlier pass, that pass didn't complete (restoring coverage always takes a full pass), it made no fixes, its content changed while it ran, it ran on another branch, a Step 11 review followed it, or its commit no longer resolves or is no longer an ancestor of HEAD (after a rebase).
|
||||
- **`lib/review-evidence.ts`**: the start token carries `scope: "fix-range"` and `fix_base`. The logged row takes `scope` and `fix_base` only from the token (a caller's values are dropped either way). On unchanged content its `review_binding` is `{state: "fix-range", range_start_wtree, range_end_wtree, …}`, with no `start_wtree`/`end_wtree` and no root `wtree`. `reviewFreshness` returns UNVERIFIED for any fix-range row ("fix-range pass; only a later full pass can clear review"), even one with a verified-looking binding.
|
||||
- **`bin/gstack-review-log --start review --fix-range`** prints the start token, then `FIX_BASE=<sha>`. The base comes from the review log, never from the caller or the working tree, so `gstack-wtree`'s fingerprint is untouched. It is the commit of the latest pass of the same skill, and that commit's tree must be exactly the content the pass started from, so `git diff <FIX_BASE>` holds every fix, committed or not. The start refuses (exit 1, "run a full pass") in each of these cases: there is no earlier pass, that pass didn't complete (restoring coverage always takes a full pass), it made no fixes, it has no captured start, it ran on another branch, a Step 11 review followed it, it started from uncommitted or untracked content, or its commit no longer resolves or is no longer an ancestor of HEAD (after a rebase). On the start-work-then-ship flow, where Step 9 first reviews uncommitted work, the first repeat therefore stays full.
|
||||
- **`lib/review-evidence.ts`**: the start token carries `scope: "fix-range"` and `fix_base`. The logged row takes `scope` and `fix_base` only from the token (a caller's values are dropped either way), and a token that names any fix base stays narrowed even if its scope is rewritten. On unchanged content its `review_binding` is `{state: "fix-range", range_start_wtree, range_end_wtree, …}`, with no `start_wtree`/`end_wtree` and no root `wtree`. `reviewFreshness` returns UNVERIFIED for any fix-range row ("fix-range pass; only a later full pass can clear review"), even one with a verified-looking binding.
|
||||
- **`bin/gstack-specialist-stats`** skips fix-range rows, so narrowed dispatches don't push specialists toward `[GATE_CANDIDATE]`.
|
||||
|
||||
### Changed
|
||||
- **`ship/sections/review-army.md.tmpl`**: the core checklist explains the fix-range start, and that `FIX_BASE` stands in for `origin/<base>` and `<base>` for that pass. Decision 3 allows the repeat to be a fix-range pass. Decision 4 continues to Step 10 only from a full pass. Nearby prose was tightened to pay for the growth, so ship's skeleton plus sections is 313,246 bytes against its 313,281 cap (35 bytes below main).
|
||||
|
||||
### Tests
|
||||
- `test/review-fix-range.test.ts` (15 cases) covers the base, chained fix-range passes, the seven refusals, a caller claiming `scope: "full"` on a fix-range token (it is still recorded as fix-range) or `"fix-range"` on a full one (it is dropped), Step 11.5's missing binding fields, UNVERIFIED freshness followed by CURRENT after a full pass, shared-libs reuse refusing a fix-range row, the stats exclusion, and the rendered Step 9 rules.
|
||||
- `test/review-fix-range.test.ts` (20 cases) follows Step 9.4's real order (fixes applied before the row is logged). It covers the base with committed and uncommitted fixes, chained fix-range passes, the eight refusals, a rewritten start token, a caller claiming `scope: "full"` on a fix-range token (it is still recorded as fix-range) or `"fix-range"` on a full one (it is dropped), Step 11.5's missing binding fields, UNVERIFIED freshness followed by CURRENT after a full pass, shared-libs reuse refusing a fix-range row, the stats exclusion, and the rendered Step 9 rules.
|
||||
|
||||
### Gate integrity
|
||||
- One gate change to review: Step 9 may narrow a repeat pass. Verdict: not a weakening of what ships, because the narrowed pass can't be the final review. Three things enforce that: its row is never CURRENT, Step 11.5 has no `start_wtree`/`end_wtree` to bind, and decision 4 requires a full pass before Step 10. Mutation checks, each caught by the suite: dropping the freshness check, giving a narrowed row the normal binding, allowing a fix-range start after a clean, an incomplete or a Step 11-followed pass, accepting a non-ancestor base, and counting fix-range rows in stats. No threshold, budget, timeout, trial count or case kind changed.
|
||||
- One gate change to review: Step 9 may narrow a repeat pass. Verdict: not a weakening of what ships, because the narrowed pass can't be the final review. Three things enforce that: its row is never CURRENT, Step 11.5 has no `start_wtree`/`end_wtree` to bind, and decision 4 requires a full pass before Step 10. Mutation checks, each caught by the suite: dropping the freshness check, giving a narrowed row the normal binding, allowing a fix-range start after a clean, an incomplete or a Step 11-followed pass, accepting a non-ancestor base, accepting a start from uncommitted content, trusting the token's scope alone, and counting fix-range rows in stats. No threshold, budget, timeout, trial count or case kind changed.
|
||||
|
||||
## [1.91.191.0] - 2026-10-25
|
||||
|
||||
|
||||
+14
-13
@@ -7,6 +7,7 @@ import { resultLine, type ResultCodeName } from './result-codes';
|
||||
|
||||
const DIFF_REVIEWS = new Set(['review', 'adversarial-review', 'codex-review', 'design-review-lite', 'ship']);
|
||||
const SHARED_LIBS_COVERAGE_VERSION = 1;
|
||||
const HASH = /^(?:[0-9a-f]{40}|[0-9a-f]{64})$/;
|
||||
|
||||
function record(value: unknown): value is Record<string, any> {
|
||||
return value !== null && typeof value === 'object' && !Array.isArray(value);
|
||||
@@ -165,10 +166,11 @@ export function checkSharedLibsReuse(finding: unknown, token: string, env = proc
|
||||
|
||||
/**
|
||||
* #3339: a fix-range repeat reviews only what changed since the latest pass of
|
||||
* the same skill, and only when that pass completed, made fixes, reviewed
|
||||
* unchanged content on this branch, and no Step 11 review followed it. The
|
||||
* base is that pass's commit, held in the review log rather than the working
|
||||
* tree, and it must still be an ancestor of HEAD. Anything else needs a full pass.
|
||||
* the same skill, and only when that pass completed and made fixes on this
|
||||
* branch, no Step 11 review followed it, and the content it started from is
|
||||
* exactly its commit's tree. That commit, held in the review log rather than
|
||||
* the working tree, is the base; it must still be an ancestor of HEAD, so
|
||||
* `git diff <base>` holds every fix. Anything else needs a full pass.
|
||||
*/
|
||||
function fixRangeBase(skill: string, env: NodeJS.ProcessEnv): string {
|
||||
const refuse = (reason: string): never => {
|
||||
@@ -186,14 +188,13 @@ function fixRangeBase(skill: string, env: NodeJS.ProcessEnv): string {
|
||||
const prior = rows[at]!, binding = prior.review_binding;
|
||||
if (prior.completed !== true) refuse('the latest pass did not complete');
|
||||
if (prior.converged !== false) refuse('the latest pass made no fixes');
|
||||
const reviewed = binding?.state === 'incomplete' ? [binding.start_wtree, binding.end_wtree]
|
||||
: binding?.state === 'fix-range' ? [binding.range_start_wtree, binding.range_end_wtree] : [];
|
||||
if (!reviewed[0] || reviewed[0] !== reviewed[1]) refuse('the latest pass has no unchanged captured content');
|
||||
if (!record(binding) || (binding.state !== 'changed' && binding.state !== 'incomplete') || typeof binding.start_wtree !== 'string') refuse('the latest pass has no captured start');
|
||||
if (binding.branch_id !== sha256(env.GSTACK_REVIEW_BRANCH ?? '')) refuse('the latest pass is from another branch');
|
||||
const base = prior.commit_full;
|
||||
const git = (...args: string[]) => spawnSync('git', args, { cwd: env.GSTACK_REVIEW_REPO, timeout: 10_000 }).status === 0;
|
||||
if (typeof base !== 'string' || !/^(?:[0-9a-f]{40}|[0-9a-f]{64})$/.test(base) || !git('cat-file', '-e', `${base}^{commit}`)) refuse('the latest pass has no resolvable commit');
|
||||
if (!git('merge-base', '--is-ancestor', base, 'HEAD')) refuse(`${base} is not an ancestor of HEAD`);
|
||||
const git = (...args: string[]) => spawnSync('git', args, { cwd: env.GSTACK_REVIEW_REPO, encoding: 'utf8', timeout: 10_000 });
|
||||
if (typeof base !== 'string' || !HASH.test(base) || git('cat-file', '-e', `${base}^{commit}`).status !== 0) refuse('the latest pass has no resolvable commit');
|
||||
if (git('rev-parse', `${base}^{tree}`).stdout.trim() !== binding.start_wtree) refuse('the latest pass started from uncommitted content, so no commit marks what it read');
|
||||
if (git('merge-base', '--is-ancestor', base, 'HEAD').status !== 0) refuse(`${base} is not an ancestor of HEAD`);
|
||||
return base;
|
||||
}
|
||||
|
||||
@@ -293,15 +294,15 @@ export function bindReview(rec: Record<string, any>, token: string, env = proces
|
||||
: start.wtree !== end ? 'changed'
|
||||
: rec.completed !== true || rec.converged !== true ? 'incomplete' : 'verified';
|
||||
const branchId = typeof start?.branch === 'string' && start.branch.length > 0 ? { branch_id: sha256(start.branch) } : {};
|
||||
const narrowed = start?.scope === 'fix-range';
|
||||
const narrowed = start?.scope === 'fix-range' || start?.fix_base !== undefined;
|
||||
if (narrowed) {
|
||||
rec.scope = 'fix-range';
|
||||
rec.fix_base = start.fix_base;
|
||||
if (typeof start.fix_base === 'string' && HASH.test(start.fix_base)) rec.fix_base = start.fix_base;
|
||||
}
|
||||
// A narrowed pass on unchanged content never carries start_wtree/end_wtree,
|
||||
// so Step 11.5 cannot bind it and it never becomes CURRENT; only a later full pass can.
|
||||
rec.review_binding = narrowed && (state === 'incomplete' || state === 'verified')
|
||||
? { state: 'fix-range', fix_base: start.fix_base, range_start_wtree: start.wtree, range_end_wtree: end, started_at: start.started_at, ...branchId }
|
||||
? { state: 'fix-range', fix_base: rec.fix_base, range_start_wtree: start.wtree, range_end_wtree: end, started_at: start.started_at, ...branchId }
|
||||
: { state, start_wtree: start?.wtree, end_wtree: end, started_at: start?.started_at, ...branchId };
|
||||
const verified = rec.review_binding.state === 'verified';
|
||||
if (verified) rec.wtree = end;
|
||||
|
||||
@@ -1,16 +1,17 @@
|
||||
/**
|
||||
* #3339: /ship Step 9 may re-review only the fix range, but a fix-range pass
|
||||
* can never be the last review. The fix base comes from the review log (never
|
||||
* the working tree or the caller), a narrowed row is never CURRENT and carries
|
||||
* no start_wtree/end_wtree for Step 11.5 to bind, and narrowed dispatches stay
|
||||
* out of specialist hit rates.
|
||||
* the working tree or the caller) and is the commit whose tree the fixing pass
|
||||
* started from, a narrowed row is never CURRENT and carries no
|
||||
* start_wtree/end_wtree for Step 11.5 to bind, and narrowed dispatches stay out
|
||||
* of specialist hit rates.
|
||||
*/
|
||||
import { describe, test, expect, beforeEach, afterEach } from 'bun:test';
|
||||
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 { canReuseSharedLibsAdvisory, reviewFreshness } from '../lib/review-evidence';
|
||||
import { canReuseSharedLibsAdvisory, reviewFreshness, sharedLibsFingerprint } from '../lib/review-evidence';
|
||||
import { gitIn } from './helpers/scratch-repo';
|
||||
|
||||
const ROOT = path.resolve(import.meta.dir, '..');
|
||||
@@ -36,61 +37,80 @@ function bin(name: string, args: string[]) {
|
||||
return { code: r.status, out: r.stdout.trim(), err: r.stderr };
|
||||
}
|
||||
const head = () => gitIn(repo, 'rev-parse HEAD').trim();
|
||||
function fix(text: string) {
|
||||
fs.writeFileSync(path.join(repo, 'a.ts'), text);
|
||||
gitIn(repo, 'commit -q -am fix');
|
||||
}
|
||||
function pass(row: Record<string, unknown>, start: string[] = ['--start', 'review']) {
|
||||
const edit = (text: string) => fs.writeFileSync(path.join(repo, 'a.ts'), text);
|
||||
const commit = () => gitIn(repo, 'commit -q -am fix');
|
||||
const row = (fields: Record<string, unknown>) => JSON.stringify({ skill: 'review', via: 'ship', status: 'clean', issues_found: 0, critical: 0, informational: 0, ...fields });
|
||||
const fixing = { status: 'issues_found', issues_found: 1, completed: true, converged: false };
|
||||
const clean = { completed: true, converged: true };
|
||||
|
||||
// Step 9.4's order: start, review, apply fixes, log with the original token, then commit the fixes.
|
||||
function pass(fields: Record<string, unknown>, start: string[] = ['--start', 'review'], fixes?: string, commitFixes = true) {
|
||||
const started = bin('gstack-review-log', start);
|
||||
expect(started.code).toBe(0);
|
||||
const [token, base] = started.out.split('\n');
|
||||
const logged = bin('gstack-review-log', [JSON.stringify({ skill: 'review', via: 'ship', status: 'clean', issues_found: 0, critical: 0, informational: 0, ...row }), '--finish', token!]);
|
||||
expect(logged.code).toBe(0);
|
||||
if (fixes) edit(fixes);
|
||||
expect(bin('gstack-review-log', [row(fields), '--finish', token!]).code).toBe(0);
|
||||
if (fixes && commitFixes) commit();
|
||||
return base;
|
||||
}
|
||||
const fixing = { status: 'issues_found', completed: true, converged: false };
|
||||
const clean = { completed: true, converged: true };
|
||||
function rows(): any[] {
|
||||
return bin('gstack-review-read', []).out.split('\n---CONFIG---')[0]!.split('\n').filter(line => line.startsWith('{')).map(line => JSON.parse(line));
|
||||
}
|
||||
const refused = () => bin('gstack-review-log', ['--start', 'review', '--fix-range']);
|
||||
const narrowStart = () => bin('gstack-review-log', ['--start', 'review', '--fix-range']);
|
||||
|
||||
describe('fix-range start', () => {
|
||||
test('after a completed fixing pass it prints the token and that pass commit as FIX_BASE', () => {
|
||||
test('after a completed fixing pass it prints the token and the reviewed commit as FIX_BASE', () => {
|
||||
const reviewed = head();
|
||||
pass(fixing);
|
||||
fix('export const a = 2;\n');
|
||||
pass(fixing, undefined, 'export const a = 2;\n');
|
||||
expect(pass(clean, ['--start', 'review', '--fix-range'])).toBe(`FIX_BASE=${reviewed}`);
|
||||
expect(gitIn(repo, `diff --name-only ${reviewed}`).trim()).toBe('a.ts');
|
||||
});
|
||||
|
||||
test('fixes still uncommitted at the repeat are inside the range', () => {
|
||||
const reviewed = head();
|
||||
pass(fixing, undefined, 'export const a = 2;\n', false);
|
||||
expect(pass(clean, ['--start', 'review', '--fix-range'])).toBe(`FIX_BASE=${reviewed}`);
|
||||
expect(gitIn(repo, `diff --name-only ${reviewed}`).trim()).toBe('a.ts');
|
||||
});
|
||||
|
||||
test('chained fix-range passes take the latest fixing pass as base', () => {
|
||||
pass(fixing);
|
||||
fix('export const a = 2;\n');
|
||||
pass(fixing, undefined, 'export const a = 2;\n');
|
||||
const second = head();
|
||||
pass(fixing, ['--start', 'review', '--fix-range']);
|
||||
fix('export const a = 3;\n');
|
||||
pass(fixing, ['--start', 'review', '--fix-range'], 'export const a = 3;\n');
|
||||
expect(pass(clean, ['--start', 'review', '--fix-range'])).toBe(`FIX_BASE=${second}`);
|
||||
});
|
||||
|
||||
test.each([
|
||||
['no earlier pass', () => {}, 'no earlier pass on this branch'],
|
||||
['a clean latest pass', () => pass(clean), 'the latest pass made no fixes'],
|
||||
['an incomplete latest pass (coverage restoration stays full)', () => pass({ ...fixing, status: 'unavailable', completed: false }), 'the latest pass did not complete'],
|
||||
['a Step 11 review after the latest pass', () => { pass(fixing); pass({ skill: 'adversarial-review', ...clean }, ['--start', 'adversarial-review']); }, 'a Step 11 review followed the latest pass'],
|
||||
['content that changed during the latest pass', () => {
|
||||
const token = bin('gstack-review-log', ['--start', 'review']).out;
|
||||
fs.writeFileSync(path.join(repo, 'a.ts'), 'export const a = 9;\n');
|
||||
bin('gstack-review-log', [JSON.stringify({ skill: 'review', ...fixing }), '--finish', token]);
|
||||
}, 'the latest pass has no unchanged captured content'],
|
||||
['an incomplete latest pass (coverage restoration stays full)', () => pass({ ...fixing, status: 'unavailable', completed: false }, undefined, 'export const a = 2;\n'), 'the latest pass did not complete'],
|
||||
['a Step 11 review after the latest pass', () => {
|
||||
pass(fixing, undefined, 'export const a = 2;\n');
|
||||
pass({ skill: 'adversarial-review', ...clean }, ['--start', 'adversarial-review']);
|
||||
}, 'a Step 11 review followed the latest pass'],
|
||||
['a latest pass logged without a start capture', () => {
|
||||
edit('export const a = 2;\n');
|
||||
expect(bin('gstack-review-log', [row(fixing)]).code).toBe(0);
|
||||
}, 'the latest pass has no captured start'],
|
||||
['a pass that started from uncommitted content (a reverted dirty hunk would fall outside the range)', () => {
|
||||
edit('export const a = 1; // dirty\n');
|
||||
pass(fixing, undefined, 'export const a = 1;\n', false);
|
||||
}, 'started from uncommitted content'],
|
||||
['a pass that started with an untracked file', () => {
|
||||
fs.writeFileSync(path.join(repo, 'b.ts'), 'export const b = 1;\n');
|
||||
pass(fixing, undefined, 'export const a = 2;\n');
|
||||
}, 'started from uncommitted content'],
|
||||
['a base that is no longer an ancestor of HEAD', () => {
|
||||
fix('export const a = 2;\n');
|
||||
pass(fixing);
|
||||
gitIn(repo, 'reset -q --hard HEAD~1');
|
||||
fix('export const a = 3;\n');
|
||||
edit('export const a = 2;\n');
|
||||
commit();
|
||||
pass(fixing, undefined, 'export const a = 3;\n');
|
||||
gitIn(repo, 'reset -q --hard HEAD~2');
|
||||
edit('export const a = 4;\n');
|
||||
commit();
|
||||
}, 'is not an ancestor of HEAD'],
|
||||
])('refuses %s', (_, setup, reason) => {
|
||||
setup();
|
||||
const r = refused();
|
||||
const r = narrowStart();
|
||||
expect(r.code).toBe(1);
|
||||
expect(r.out).toBe('');
|
||||
expect(r.err).toContain('fix-range refused: ');
|
||||
@@ -106,30 +126,50 @@ describe('fix-range start', () => {
|
||||
describe('a fix-range pass is never the last review', () => {
|
||||
test('its row is fix-range scoped whatever the caller claims, unbindable at Step 11.5 and never CURRENT', () => {
|
||||
const reviewed = head();
|
||||
pass(fixing);
|
||||
fix('export const a = 2;\n');
|
||||
pass(fixing, undefined, 'export const a = 2;\n');
|
||||
pass({ ...clean, scope: 'full', fix_base: 'f'.repeat(40), review_binding: { state: 'verified', start_wtree: W, end_wtree: W }, wtree: W },
|
||||
['--start', 'review', '--fix-range']);
|
||||
const row = rows().at(-1);
|
||||
expect(row.scope).toBe('fix-range');
|
||||
expect(row.fix_base).toBe(reviewed);
|
||||
expect(row.review_binding.state).toBe('fix-range');
|
||||
expect(row.review_binding.range_start_wtree).toBe(row.review_binding.range_end_wtree);
|
||||
expect(row.review_binding).not.toHaveProperty('start_wtree');
|
||||
expect(row.review_binding).not.toHaveProperty('end_wtree');
|
||||
expect(row).not.toHaveProperty('wtree');
|
||||
expect(row.review_freshness).toEqual({ status: 'UNVERIFIED', reason: 'fix-range pass; only a later full pass can clear review' });
|
||||
expect(refused().err).toContain('the latest pass made no fixes');
|
||||
const last = rows().at(-1);
|
||||
expect(last.scope).toBe('fix-range');
|
||||
expect(last.fix_base).toBe(reviewed);
|
||||
expect(last.review_binding.state).toBe('fix-range');
|
||||
expect(last.review_binding.range_start_wtree).toBe(last.review_binding.range_end_wtree);
|
||||
expect(last.review_binding).not.toHaveProperty('start_wtree');
|
||||
expect(last.review_binding).not.toHaveProperty('end_wtree');
|
||||
expect(last).not.toHaveProperty('wtree');
|
||||
expect(last.review_freshness).toEqual({ status: 'UNVERIFIED', reason: 'fix-range pass; only a later full pass can clear review' });
|
||||
expect(narrowStart().err).toContain('the latest pass made no fixes');
|
||||
pass(clean);
|
||||
expect(rows().at(-1).review_freshness.status).toBe('CURRENT');
|
||||
});
|
||||
|
||||
test('a fixing fix-range pass is not CURRENT either', () => {
|
||||
pass(fixing, undefined, 'export const a = 2;\n');
|
||||
pass(fixing, ['--start', 'review', '--fix-range'], 'export const a = 3;\n');
|
||||
const last = rows().at(-1);
|
||||
expect(last.scope).toBe('fix-range');
|
||||
expect(last.review_freshness.status).not.toBe('CURRENT');
|
||||
});
|
||||
|
||||
test('a caller cannot narrow a full pass by claiming fix-range', () => {
|
||||
pass({ ...clean, scope: 'fix-range', fix_base: W });
|
||||
const row = rows().at(-1);
|
||||
expect(row).not.toHaveProperty('scope');
|
||||
expect(row).not.toHaveProperty('fix_base');
|
||||
expect(row.review_freshness.status).toBe('CURRENT');
|
||||
const last = rows().at(-1);
|
||||
expect(last).not.toHaveProperty('scope');
|
||||
expect(last).not.toHaveProperty('fix_base');
|
||||
expect(last.review_freshness.status).toBe('CURRENT');
|
||||
});
|
||||
|
||||
test('a start token that still names a fix base stays narrowed even if its scope is rewritten', () => {
|
||||
pass(fixing, undefined, 'export const a = 2;\n');
|
||||
const [token] = narrowStart().out.split('\n');
|
||||
const slug = bin('gstack-slug', []).out.match(/^SLUG=(.+)$/m)![1]!;
|
||||
const file = path.join(root, 'state', 'projects', slug, '.review-starts', `${token}.json`);
|
||||
fs.writeFileSync(file, JSON.stringify({ ...JSON.parse(fs.readFileSync(file, 'utf8')), scope: 'full', fix_base: 'not-a-commit' }));
|
||||
expect(bin('gstack-review-log', [row(clean), '--finish', token!]).code).toBe(0);
|
||||
const last = rows().at(-1);
|
||||
expect(last.scope).toBe('fix-range');
|
||||
expect(last).not.toHaveProperty('fix_base');
|
||||
expect(last.review_freshness.status).toBe('UNVERIFIED');
|
||||
});
|
||||
|
||||
test('freshness rejects a fix-range row even with a verified-looking binding', () => {
|
||||
@@ -141,11 +181,12 @@ describe('a fix-range pass is never the last review', () => {
|
||||
|
||||
test('shared-libs advisory reuse never trusts a fix-range row', () => {
|
||||
const finding = { severity: 'INFORMATIONAL', advisory: true, action: 'skipped', evidence_paths: ['a.ts'], helper_target: { path: 'b.ts', symbol: 'b' } };
|
||||
const fp = 'shared-libs:' + 'c'.repeat(64);
|
||||
const prior = { skill: 'review', completed: true, converged: true, wtree: W, scope: 'fix-range',
|
||||
review_binding: { state: 'fix-range', range_start_wtree: W, range_end_wtree: W, branch_id: 'd'.repeat(64) } };
|
||||
expect(canReuseSharedLibsAdvisory({ ...finding, fingerprint: fp, snapshot_covered_paths: ['a.ts'] }, finding, prior,
|
||||
{ wtree: W, branch_id: 'd'.repeat(64), covered_paths: ['a.ts'] })).toBe(false);
|
||||
const prior = { ...finding, fingerprint: sharedLibsFingerprint(finding), snapshot_covered_paths: ['a.ts'] };
|
||||
const snapshot = { wtree: W, branch_id: 'd'.repeat(64), covered_paths: ['a.ts'] };
|
||||
const full = { skill: 'review', completed: true, converged: true, wtree: W, review_binding: { state: 'verified', start_wtree: W, end_wtree: W, branch_id: 'd'.repeat(64) } };
|
||||
expect(canReuseSharedLibsAdvisory(prior, finding, full, snapshot)).toBe(true);
|
||||
const narrowed = { ...full, scope: 'fix-range', review_binding: { state: 'fix-range', range_start_wtree: W, range_end_wtree: W, branch_id: 'd'.repeat(64) } };
|
||||
expect(canReuseSharedLibsAdvisory(prior, finding, narrowed, snapshot)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
Reference in new issue
Block a user