mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-10 10:40:32 +01:00
Stop a main merge from counting as the PR's own change, so a retake no longer needs a second full review (GST-2229) (#4259)
The receipt carry and finish scoped-review packet counted context lines, hunk headers and the base's change toward the 200-line section cap (founderstar#802). gstack-review-plan measured the fix range as --range-base..HEAD, so files a main merge brought counted as the PR's fix (harness-evals#354). Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
1d91652764
commit
3183120cec
9 files changed
+173
-23
No files matched your search
@@ -41,7 +41,7 @@ import { effectiveReaders, READERS_POLICY_DEFAULT } from '../lib/ship-policy-rea
|
||||
import { classifyDiff, diffCommand, keepCommand, planReview, reviewDepthLine, reviewPlanLine, scanNumbers, ROLE_EFFORT, SCOPED_RANGE_LINES, type Depth, type History } from '../lib/review-depth';
|
||||
import { classList, targetedBriefSha, targetedClasses, targetedPatchId } from '../lib/review-targeted';
|
||||
import { targetedDelta } from '../lib/review-targeted-delta';
|
||||
import { docsChangeSince } from '../lib/ship-receipt-depth';
|
||||
import { docsChangeSince, ownChangeSince } from '../lib/ship-receipt-depth';
|
||||
import { RISK_CLASSES, type RiskClass } from '../lib/review-risk';
|
||||
import { renderFixCheckPrompt } from '../scripts/resolvers/fix-check';
|
||||
|
||||
@@ -226,17 +226,19 @@ function main(): number {
|
||||
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; touched_gate?: true } | undefined;
|
||||
let own: string | null = null;
|
||||
if (rangeBase) {
|
||||
const unread = (error: string) => ({ base: rangeBase, runtime_lines: null, error });
|
||||
const resolved = git(['rev-parse', '--verify', '--quiet', `${rangeBase}^{commit}`])?.trim();
|
||||
if (!resolved) fixRange = unread('not a commit in this repository');
|
||||
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 if (!(own = ownChangeSince(root, ref, resolved, 'HEAD'))) fixRange = unread('git could not build the PR\'s own change since the range base');
|
||||
else {
|
||||
const commit = collectCommit(root, resolved, 'HEAD');
|
||||
const range = classifyDiff({ ...commit, registry, scope: scopeOf([resolved, '--head', 'HEAD']), gateTagged: gateTagsOf([resolved]), ...riskPaths, ...(active ? { excluded: active.excluded } : {}) });
|
||||
const commit = collectCommit(root, resolved, own);
|
||||
const range = classifyDiff({ ...commit, registry, scope: scopeOf([resolved, '--head', own]), gateTagged: gateTagsOf(['--commit', own]), ...riskPaths, ...(active ? { excluded: active.excluded } : {}) });
|
||||
fixRange = range.errors.length ? unread([...range.errors.slice(0, 5).map(e => `${e.code} ${e.input}`), ...(range.errors.length > 5 ? [`+${range.errors.length - 5} more`] : [])].join('; '))
|
||||
: !commit.paths.length ? unread('the fix range is empty: commit the fix after the range base')
|
||||
: !commit.paths.length ? unread('the fix range is empty: commit the fix after the range base (a base merge alone is not the PR\'s own change)')
|
||||
: { base: rangeBase, runtime_lines: range.runtimeLines, ...(range.class === 'gate' ? { touched_gate: true as const } : {}) };
|
||||
}
|
||||
}
|
||||
@@ -247,8 +249,8 @@ function main(): number {
|
||||
const postReview = has('--post-review');
|
||||
const fixCheckAt = postReview && pass > 0 && last === 'fix-check' && fixRange?.runtime_lines != null ? git(['rev-parse', '--verify', '--quiet', `${rangeBase}^{commit}`])?.trim() : undefined;
|
||||
const afterFixCheck = fixCheckAt ? docsChangeSince(root, fixCheckAt, head, ref) : null;
|
||||
const rawSince = fixCheckAt ? git(['diff', '--raw', '--no-renames', fixCheckAt, 'HEAD']) : null;
|
||||
const carried = afterFixCheck && afterFixCheck.verdict !== 'code' && rawSince !== null && !/^:\d+ (100755|120000|160000) /m.test(rawSince) && gateTagsOf([fixCheckAt!]) === 0 ? afterFixCheck : null;
|
||||
const rawSince = fixCheckAt && own ? git(['diff', '--raw', '--no-renames', fixCheckAt, own]) : null;
|
||||
const carried = afterFixCheck && afterFixCheck.verdict !== 'code' && rawSince !== null && !/^:\d+ (100755|120000|160000) /m.test(rawSince) && gateTagsOf(['--commit', own!]) === 0 ? afterFixCheck : null;
|
||||
const plan = planReview({
|
||||
...collected, ...exclusionInput, ...riskPaths, declared: declared as RiskClass[], shape: { panel: review.panel, readerModel: review.reader_model, platform: review.platform, head, platformReview: platform?.ok ? platform.review : null }, scope, gateTagged, targeted: { classes: targeted.classes, ran }, numbers: scanNumbers(kept(added)), force, gated: list('--gated'), floor, tiers: auditEffort !== 'host',
|
||||
criticalFound: has('--critical-found'), adversarialRan: has('--adversarial-ran'), reviewedNumbers: flag('--reviewed-numbers'),
|
||||
|
||||
@@ -0,0 +1,5 @@
|
||||
---
|
||||
important: blocker
|
||||
---
|
||||
- **A base merge that only moves the lines around a reviewed change no longer forces a second full /ship.** On founderstar#802, a base merge changed the context lines around the PR's 22 reviewed and unchanged hunks in `scripts/core/game.gd` (+66/-11 at both the old receipt head and the new one). The scoped-review packet counted those context lines and the hunk headers toward its 200-line section limit, so the section came to about 205 lines and `gstack-ship-receipt carry` refused with `RECEIPT_CARRY_PRODUCT_CHANGED`, which meant a full /ship on every mechanical retake. 1.94.39.0 (GST-1226) had stopped counting the base's +/- lines, but not the context lines inside the branch's hunks or in the base's change. Now only the branch's own +/- lines count, in carry and in `gstack-ship-release finish`; context lines, hunk headers and the base's change are still shown to the reader. A section whose branch +/- lines pass 200 still takes the full loop, and a base change over 2,000 lines in one file still counts in full. (GST-2229)
|
||||
- **A main merge inside the review fix range no longer makes main's files count as the PR's fix.** On harness-evals#354, the PR's own change since the last targeted read was a changelog fragment, one docs line and one manifest line, but `gstack-review-plan` measured the fix range as `--range-base` to HEAD, so the 68-line bootstrap script and the `.gstack/pregate.json` edit a main merge brought in made the range gate class. With the owner's grant it printed `next=voices` and re-reviewed code main had already shipped, instead of the one targeted-only read. The fix range, and the check for gate-class edits after the last fix-check, now read only the PR's own change since the range base, the same change the receipt already reads (GST-1593). A gate-class edit in the PR's own change still escalates. (GST-2229)
|
||||
@@ -172,7 +172,10 @@ A full pass 1 always gets a full pass 2: `--pass1-depth` carries pass 1's
|
||||
`REVIEW_DEPTH`, and an absent flag counts as full. The fix range
|
||||
`RANGE_BASE..HEAD` must have at most `SCOPED_RANGE_LINES` (150) runtime lines, counted
|
||||
as the depth counts them (`gstack-review-plan --range-base $PASS_HEAD` prints
|
||||
`FIX_RANGE:`). It dispatches one fix-check on the range, the Red Team on the
|
||||
`FIX_RANGE:`). The range holds only the PR's own change since `RANGE_BASE`: when
|
||||
a base merge lands inside it, what the base brought (a file only main changed,
|
||||
or main's lines in a file both changed) is not counted, and neither is its gate
|
||||
class (GST-2229, harness-evals #354). It dispatches one fix-check on the range, the Red Team on the
|
||||
range, the native adversarial pass when the range changes runtime lines, every
|
||||
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
|
||||
|
||||
+2
-2
@@ -2129,8 +2129,8 @@ changed paths' diff, then the scoped-review sections of the context paths. The
|
||||
400-line and 64 KB caps count the whole packet, and `range_diff_sha256` binds
|
||||
all of it. The record also names `range_base`, the commit the reviewed range
|
||||
starts from (the fork point, or a landed stack parent), and the carry reason
|
||||
logs the context paths and that `range_base`. A context section over 200 lines,
|
||||
or a packet over the caps, prints `RECHECK: not eligible (<why>)` and stays
|
||||
logs the context paths and that `range_base`. A context section with over 200
|
||||
of the branch's own +/- lines, or a packet over the caps, prints `RECHECK: not eligible (<why>)` and stays
|
||||
`RECEIPT_CARRY_PRODUCT_CHANGED`. A changed path alone, or context paths alone,
|
||||
keep the routes above, and their output is unchanged.
|
||||
|
||||
|
||||
@@ -6845,12 +6845,13 @@ took `SAVE_VERSION := 83` under the branch's `:= 84`). The merge may have splice
|
||||
changed one key and the base the next, so the receipt does not carry silently.
|
||||
The refusal names the paths and a packet file with the hunks that differ and
|
||||
the base's own change. The same lines moved somewhere else are
|
||||
`RECEIPT_CARRY_PRODUCT_CHANGED`, not this code. A packet section over 200
|
||||
lines takes the full loop instead, but only the branch's own lines count toward
|
||||
that limit: the base's +/- lines, already merged at base, are shown and not
|
||||
counted (GST-1226), so a large base edit next to the branch's hunk still gets
|
||||
the scoped read. A base change over 2,000 lines in one file counts in full
|
||||
again and takes the full loop.
|
||||
`RECEIPT_CARRY_PRODUCT_CHANGED`, not this code. A packet section with over
|
||||
200 of the branch's own +/- lines takes the full loop instead. Nothing else
|
||||
counts toward that limit: the context lines and headers of the branch's hunks,
|
||||
which a base merge can move (GST-2229), and the base's own change, already
|
||||
merged at base (GST-1226), are shown and not counted, so a large base edit next
|
||||
to the branch's hunks still gets the scoped read. A base change over 2,000 lines
|
||||
in one file counts in full again and takes the full loop.
|
||||
|
||||
**Fix.** Dispatch one fix-check reviewer
|
||||
([review-policy.md](review-policy.md)) on the packet. When it comes back clean,
|
||||
@@ -6884,7 +6885,7 @@ range_diff_lines=N range_diff_sha256=<sha> packet=<file>`: one reader on that
|
||||
range-diff can clear it instead of a full second /ship. A rebased branch
|
||||
(`RECEIPT_CARRY_TREE_MISMATCH`), a retargeted stack, a receipt with no
|
||||
`review_depth`, more than 20 changed product paths, a binary path, a moved
|
||||
base context section over 200 lines, an empty or unreadable range-diff and a
|
||||
base context section with over 200 of the branch's own +/- lines, an empty or unreadable range-diff and a
|
||||
packet over 400 lines or 64 KB print `RECHECK: not eligible (<why>)` instead.
|
||||
When base context also moved next to reviewed hunks in other paths, their
|
||||
scoped-review sections join the same packet (GST-1561): the caps count the
|
||||
|
||||
+8
-6
@@ -176,20 +176,22 @@ function hunkTexts(diff: string): string[] {
|
||||
return diff.split(/^(?=@@ )/m).slice(1).map(h => h.replace(/^@@ [^@]*@@/, '@@'));
|
||||
}
|
||||
|
||||
/** The most lines one packet section may hold; a path with a longer section takes the full loop instead. */
|
||||
/** The most branch +/- lines one packet section may hold; a path with a longer section takes the full loop instead. */
|
||||
export const PACKET_SECTION_CAP = 200;
|
||||
/** The most lines the base's change section may hold before its uncounted base lines count again (GST-1226). */
|
||||
/** The most lines the base's change section may hold before its uncounted lines count again (GST-1226). */
|
||||
export const BASE_SECTION_CEILING = 2000;
|
||||
|
||||
/**
|
||||
* The scoped-review packet: per context path, the hunks that differ between the two branch patches and the base's own
|
||||
* change; per test path, its test hunks. `over` names every path with a section past PACKET_SECTION_CAP: one reviewer
|
||||
* cannot be shown all of it, so the caller sends it to the full loop. The base's +/- lines, already merged at base, are
|
||||
* shown but do not count toward the cap (GST-1226) until that section passes BASE_SECTION_CEILING lines.
|
||||
* cannot be shown all of it, so the caller sends it to the full loop. Only the branch's own +/- lines count toward the cap:
|
||||
* the context lines and headers of its hunks, which a base merge can move (GST-2229, founderstar#802), and the base's change,
|
||||
* already merged at base (GST-1226), are shown but not counted until the base's change passes BASE_SECTION_CEILING lines.
|
||||
*/
|
||||
export function contextPacket(repoRoot: string, s: PatchSides, paths: readonly string[], tests: readonly string[] = [], bounded: Readonly<Record<string, BoundedKind>> = {}): { text: string; over: string[]; sha256: string } {
|
||||
const over = new Set<string>();
|
||||
let current = '';
|
||||
const own = (line: string) => /^[+-]/.test(line);
|
||||
const fence = (label: string, body: string, counts: (line: string) => boolean = () => true) => {
|
||||
if (body.split('\n').filter(counts).length > PACKET_SECTION_CAP) over.add(current);
|
||||
return `${label}\n\`\`\`diff\n${body.trimEnd()}\n\`\`\`\n`;
|
||||
@@ -206,8 +208,8 @@ export function contextPacket(repoRoot: string, s: PatchSides, paths: readonly s
|
||||
const r = hunkTexts(pathDiff(repoRoot, s.reviewedBase, s.reviewed, rel, 3) ?? '');
|
||||
const c = hunkTexts(pathDiff(repoRoot, s.currentBase, s.current, rel, 3) ?? '');
|
||||
const b = hunkTexts(pathDiff(repoRoot, s.reviewedBase, s.currentBase, rel, 3) ?? '').join('');
|
||||
parts.push(`## ${rel}\n`, ...(rel in bounded ? [fence('Changed since review', pathDiff(repoRoot, s.reviewed, s.current, rel, 3) ?? '')] : []), fence('Reviewed hunks', r.filter(h => !c.includes(h)).join('')), fence('Current hunks', c.filter(h => !r.includes(h)).join('')),
|
||||
fence("The base's change", b, b.split('\n').length > BASE_SECTION_CEILING ? undefined : l => !/^[+-]/.test(l)));
|
||||
parts.push(`## ${rel}\n`, ...(rel in bounded ? [fence('Changed since review', pathDiff(repoRoot, s.reviewed, s.current, rel, 3) ?? '')] : []), fence('Reviewed hunks', r.filter(h => !c.includes(h)).join(''), own), fence('Current hunks', c.filter(h => !r.includes(h)).join(''), own),
|
||||
fence("The base's change", b, b.split('\n').length > BASE_SECTION_CEILING ? undefined : () => false));
|
||||
}
|
||||
if (late.length) parts.push(`## Targeted brief (${TARGETED_CLASSES.join(', ')})\n\n${packetBrief()}\n`);
|
||||
const text = parts.join('\n');
|
||||
|
||||
@@ -533,7 +533,7 @@ export function bindShippedReceipt(root: string, baseRef: string, body: string,
|
||||
* commit since `from` that origin/<base> does not hold, side-branch merges included (a CI skip marker is gate class). A path in
|
||||
* `keep` keeps `from`'s content. Null when git cannot build it.
|
||||
*/
|
||||
function ownChangeSince(root: string, base: string, from: string, head: string, keep: readonly string[] = []): string | null {
|
||||
export function ownChangeSince(root: string, base: string, from: string, head: string, keep: readonly string[] = []): string | null {
|
||||
const run = (args: string[], env?: NodeJS.ProcessEnv, input?: string) => {
|
||||
const r = spawnSync('git', args, { cwd: root, encoding: 'utf8', env, input, timeout: 60_000, maxBuffer: 256 * 1024 * 1024 });
|
||||
return r.status === 0 ? r.stdout : null;
|
||||
|
||||
@@ -0,0 +1,77 @@
|
||||
import { afterEach, describe, expect, setDefaultTimeout, test } 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';
|
||||
|
||||
setDefaultTimeout(60_000);
|
||||
const BIN = path.join(import.meta.dir, '..', 'bin');
|
||||
const dirs: string[] = [];
|
||||
afterEach(() => { while (dirs.length) fs.rmSync(dirs.pop()!, { recursive: true, force: true }); });
|
||||
|
||||
const money = (cap: number) => `export const spendCapUsd = ${cap};\nexport function charge(amountUsd: number) { if (amountUsd > spendCapUsd) throw new Error("over cap"); return amountUsd; }\n`;
|
||||
const bootstrap = '#!/usr/bin/env bash\nset -euo pipefail\n' + Array.from({ length: 66 }, (_, i) => `echo "step ${i + 1}"\n`).join('');
|
||||
const pregate = (stages: string[]) => JSON.stringify({ stages: stages.map(name => ({ name, run: `bun test ${name}` })) }, null, 2) + '\n';
|
||||
|
||||
/**
|
||||
* The harness-evals #354 shape: the targeted reader read R, the loop stopped at the cap on S (a fix after R), the owner granted one
|
||||
* scoped fix-check naming S, and then a plain main merge brought gate-class content (an executable bootstrap script and
|
||||
* .gstack/pregate.json) next to the PR's own small change since R: a changelog fragment, a docs line and a manifest line.
|
||||
*/
|
||||
function merged(own: Record<string, string>) {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'merged-range-'));
|
||||
const state = fs.mkdtempSync(path.join(os.tmpdir(), 'merged-range-state-'));
|
||||
dirs.push(dir, state);
|
||||
const env: NodeJS.ProcessEnv = { ...process.env, GSTACK_STATE_ROOT: state, GSTACK_APPROVALS_MIRROR: '0' };
|
||||
delete env.GSTACK_APPROVALS_DIR;
|
||||
const git = (...a: string[]) => spawnSync('git', a, { cwd: dir, encoding: 'utf8', timeout: 10_000 }).stdout.trim();
|
||||
const commit = (files: Record<string, string>, msg: string) => {
|
||||
for (const [p, text] of Object.entries(files)) { fs.mkdirSync(path.dirname(path.join(dir, p)), { recursive: true }); fs.writeFileSync(path.join(dir, p), text); }
|
||||
git('add', '.'); git('commit', '-qm', msg);
|
||||
return git('rev-parse', 'HEAD');
|
||||
};
|
||||
git('init', '-q', '-b', 'main'); git('config', 'user.email', 't@t'); git('config', 'user.name', 't'); git('config', 'commit.gpgsign', 'false');
|
||||
commit({ 'README.md': '# x\n', 'docs/README.md': '# Docs\n\nIntro.\n', 'manifest.json': '{\n "name": "x",\n "files": ["src"]\n}\n', '.gstack/pregate.json': pregate(['unit']) }, 'base');
|
||||
git('checkout', '-qb', 'feat');
|
||||
const read = commit({ 'src/billing.ts': money(25) }, 'feature');
|
||||
const plan = (args: string[]) => spawnSync(path.join(BIN, 'gstack-review-plan'), ['--base', 'main', '--gate-tagged', '0', ...args], { cwd: dir, encoding: 'utf8', timeout: 60_000, env });
|
||||
const ran = JSON.parse(plan(['--json']).stdout).targeted.targeted_patch_id as string;
|
||||
const stop = commit({ 'src/billing.ts': money(30) }, 'round 2 fix');
|
||||
const add = spawnSync(path.join(BIN, 'gstack-approvals'), ['add', '--ask', `Owner grant: one scoped fix-check after round 2 (review_loop_grant ${stop.slice(0, 12)})`, '--words', 'do it', '--by', 'GStack Minder', '--scope', 'this /ship invocation', '--json'], { cwd: dir, encoding: 'utf8', timeout: 60_000, env });
|
||||
expect(add.status, add.stderr).toBe(0);
|
||||
const grant = JSON.parse(add.stdout).added[0] as string;
|
||||
git('checkout', '-q', 'main');
|
||||
commit({ 'scripts/ubicloud/bootstrap.sh': bootstrap, '.gstack/pregate.json': pregate(['unit', 'e2e', 'smoke']) }, 'main: ubicloud bootstrap');
|
||||
fs.chmodSync(path.join(dir, 'scripts/ubicloud/bootstrap.sh'), 0o755); git('add', '.'); git('commit', '-qm', 'main: executable bootstrap');
|
||||
git('checkout', '-q', 'feat');
|
||||
expect(spawnSync('git', ['merge', '-q', '--no-edit', 'main'], { cwd: dir, encoding: 'utf8', timeout: 10_000 }).status).toBe(0);
|
||||
commit(own, 'own change after the merge');
|
||||
return plan(['--pass', '2', '--last', 'fix-check', '--fix-checks', '1', '--range-base', read, '--targeted-ran', ran, '--grant', grant]);
|
||||
}
|
||||
const lineOf = (stdout: string, key: string) => stdout.split('\n').find(l => l.startsWith(`${key}: `)) ?? '';
|
||||
const small = { 'changelog.d/billing-cap.md': '- **Billing cap documented.**\n', 'docs/README.md': '# Docs\n\nIntro, with the billing cap.\n', 'manifest.json': '{\n "name": "x",\n "files": ["src", "docs"]\n}\n' };
|
||||
|
||||
describe('the fix range counts only the PR\'s own change after a main merge (GST-2229, harness-evals #354)', () => {
|
||||
// Value: protects=a plain main merge since --range-base does not make gate-class content main already shipped count as the PR's
|
||||
// own fix range: with the owner's grant and a small own change, the plan prints the one targeted-only read;
|
||||
// fails_when=the fix range diffs --range-base against HEAD, so main's bootstrap script and pregate.json mark it touched_gate and the
|
||||
// plan stops or asks for Step 11's voices (harness-evals #354: next=voices); seam=bin/gstack-review-plan's fix range;
|
||||
// no_claim=what the targeted reader finds
|
||||
test('main\'s merged gate-class files leave the fix range, and the grant gets its targeted-only read', () => {
|
||||
const p = merged(small);
|
||||
expect(p.status, p.stderr).toBe(0);
|
||||
expect(Number(/^FIX_RANGE: runtime_lines=(\d+) /.exec(lineOf(p.stdout, 'FIX_RANGE'))?.[1])).toBeLessThan(10);
|
||||
expect(lineOf(p.stdout, 'NEXT')).toStartWith('NEXT: targeted (the loop is done');
|
||||
expect(p.stdout).toContain('DISPATCH: targeted targeted effort=full classes=money');
|
||||
});
|
||||
|
||||
// Value: protects=hold: a gate-class edit in the PR's own change since --range-base still escalates after the same merge;
|
||||
// fails_when=measuring the PR's own change drops its own pregate.json edit and the plan prints the targeted-only read;
|
||||
// seam=bin/gstack-review-plan's fix range; no_claim=none
|
||||
test('the PR\'s own pregate.json edit after the merge still marks the fix range gate class', () => {
|
||||
const p = merged({ ...small, '.gstack/pregate.json': pregate(['unit', 'e2e', 'smoke', 'billing']) });
|
||||
expect(p.status, p.stderr).toBe(0);
|
||||
expect(lineOf(p.stdout, 'NEXT')).toMatch(/^NEXT: (stop|voices) .*the fix-check edited a gate-class path/);
|
||||
expect(p.stdout).not.toContain('TARGETED_DELTA:');
|
||||
});
|
||||
});
|
||||
@@ -981,3 +981,63 @@ describe('base lines do not count toward the scoped-review cap (GST-1226, founde
|
||||
expect(scoped(finish(f, reviewed, post, ['--reviewed-base', reviewedBase]))).toEqual([3, 'behavior=1 docs=0 metadata=0 tests=0 base=0 context=0', false, true]);
|
||||
});
|
||||
});
|
||||
|
||||
// Value: protects=a base merge that only changes context lines inside the branch's own unchanged hunks reaches the carry's
|
||||
// scoped read even when those context lines and hunk headers push a section past PACKET_SECTION_CAP (founderstar#802: 22
|
||||
// hunks, +66/-11, a section of about 205 lines); a branch whose own +/- lines pass the cap is still refused;
|
||||
// fails_when=carry counts base context lines (in the branch's hunks or in the base's change) toward the cap and refuses
|
||||
// with RECEIPT_CARRY_PRODUCT_CHANGED, or lets a section the branch's own lines push over reach the scoped read;
|
||||
// seam=contextPacket's count; no_claim=the scoped reviewer's judgment
|
||||
describe('only the branch\'s own +/- lines count toward the scoped-review cap in carry (GST-2229, founderstar#802)', () => {
|
||||
const row = (at: number, v: string | number = at) => `exports.v${at} = ${v};`;
|
||||
/** `hunks` branch hunks 15 lines apart, alternating a 1-to-3 replacement and a 3-line insertion; the base edits the line two below each of the first `edits` spots, inside a branch hunk's context where there is one. */
|
||||
const file = (hunks: number, edits: number, pr: boolean, base: boolean) => Array.from({ length: 15 * Math.max(hunks, edits) + 10 }, (_, i) => {
|
||||
const at = i + 1, k = (at - 10) / 15, e = (at - 12) / 15;
|
||||
const line = base && Number.isInteger(e) && e >= 0 && e < edits ? row(at, `"base ${at}"`) : row(at);
|
||||
if (!pr || !Number.isInteger(k) || k < 0 || k >= hunks) return [line];
|
||||
const added = [0, 1, 2].map(j => `exports.pr${at}_${j} = ${j};`);
|
||||
return k % 2 ? [line, ...added] : added;
|
||||
}).flat().join('\n') + '\n';
|
||||
const carry = (hunks: number, edits = hunks) => {
|
||||
const repo = makeFixtureRepo({ prefix: 'ship-rereview-own-' }); repos.push(repo);
|
||||
repo.git('checkout', '-q', 'main');
|
||||
write(repo.work, 'lib/game.js', file(hunks, edits, false, false)); repo.git('add', '-A'); repo.git('commit', '-qm', 'game'); repo.git('push', '-q', 'origin', 'main');
|
||||
repo.git('checkout', '-q', 'feature'); repo.git('merge', '-q', '--no-edit', 'main');
|
||||
write(repo.work, 'lib/game.js', file(hunks, edits, true, false)); repo.git('commit', '-qam', 'feat: game');
|
||||
const body = path.join(repo.root, 'body.md');
|
||||
fs.writeFileSync(body, placeReceipt('## Summary\n\nProse.\n', { schema_version: 1, pr: 9, head: repo.git('rev-parse', 'HEAD'), base: 'main', queue_mode: 'stamp-at-merge' } as any));
|
||||
repo.git('checkout', '-q', 'main');
|
||||
write(repo.work, 'lib/game.js', file(hunks, edits, false, true)); repo.git('commit', '-qam', 'main: context'); repo.git('push', '-q', 'origin', 'main');
|
||||
repo.git('checkout', '-q', 'feature');
|
||||
spawnMerge(repo, { 'lib/game.js': file(hunks, edits, true, true) });
|
||||
expect(fs.readFileSync(path.join(repo.work, 'lib/game.js'), 'utf8')).toBe(file(hunks, edits, true, true));
|
||||
return runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body, '--base', 'main']);
|
||||
};
|
||||
const section = (text: string, label: string) => (new RegExp(`^${label}\\n\`\`\`diff\\n([\\s\\S]*?)\\n\`\`\`$`, 'm').exec(text)?.[1] ?? '').split('\n');
|
||||
|
||||
const packet = (r: { status: number | null; out: string; err: string }) => {
|
||||
expect([r.status, r.out.trim().split('\n')[0]], r.err).toEqual([EXIT.refused, 'CARRY: refused (RECEIPT_CARRY_SCOPED_REVIEW)']);
|
||||
const at = /reads (\S+), then/.exec(r.err)?.[1];
|
||||
expect(at).toBeDefined();
|
||||
return fs.readFileSync(at!, 'utf8');
|
||||
};
|
||||
const own = (rows: string[]) => rows.filter(l => /^[+-]/.test(l)).length;
|
||||
|
||||
test('22 unchanged branch hunks whose base context moved reach the scoped read with their sections over 200 lines', () => {
|
||||
const text = packet(carry(22));
|
||||
const current = section(text, 'Current hunks'), reviewed = section(text, 'Reviewed hunks');
|
||||
expect([current.length > 200, own(current), reviewed.length > 200, own(reviewed)]).toEqual([true, 77, true, 77]);
|
||||
expect(text).toContain('+exports.pr10_0 = 0;');
|
||||
expect(text).toContain(' exports.v12 = "base 12";');
|
||||
});
|
||||
|
||||
test('30 scattered base edits whose context lines pass 200 next to 4 unchanged branch hunks reach the scoped read', () => {
|
||||
const base = section(packet(carry(4, 30)), "The base's change");
|
||||
expect([base.length - own(base) > 200, own(base)]).toEqual([true, 60]);
|
||||
});
|
||||
|
||||
test('60 branch hunks whose own +/- lines pass 200 are still too large for the scoped review', () => {
|
||||
const r = carry(60);
|
||||
expect([r.status, r.out.trim().split('\n')[0], r.err.includes('lib/game.js (too large for the scoped review: over 200 lines in a section)')]).toEqual([EXIT.refused, 'CARRY: refused (RECEIPT_CARRY_PRODUCT_CHANGED)', true]);
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user