From 3183120cec6241162f0b60f3772bbff6f1835275 Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Mon, 9 Nov 2026 15:41:35 -0800 Subject: [PATCH] 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> --- bin/gstack-review-plan | 14 ++-- ...carry-counts-only-branch-lines-gst-2229.md | 5 ++ docs/review-policy.md | 5 +- docs/ship-policy.md | 4 +- docs/troubleshooting.md | 15 ++-- lib/ship-patch.ts | 14 ++-- lib/ship-receipt-depth.ts | 2 +- test/review-plan-merged-range.test.ts | 77 +++++++++++++++++++ test/ship-rereview-triggers.test.ts | 60 +++++++++++++++ 9 files changed, 173 insertions(+), 23 deletions(-) create mode 100644 changelog.d/receipt-carry-counts-only-branch-lines-gst-2229.md create mode 100644 test/review-plan-merged-range.test.ts diff --git a/bin/gstack-review-plan b/bin/gstack-review-plan index 90c1e8f3d..b90291b38 100755 --- a/bin/gstack-review-plan +++ b/bin/gstack-review-plan @@ -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'), diff --git a/changelog.d/receipt-carry-counts-only-branch-lines-gst-2229.md b/changelog.d/receipt-carry-counts-only-branch-lines-gst-2229.md new file mode 100644 index 000000000..d3b1d4ff6 --- /dev/null +++ b/changelog.d/receipt-carry-counts-only-branch-lines-gst-2229.md @@ -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) diff --git a/docs/review-policy.md b/docs/review-policy.md index 4cfdc181d..92c2e6d56 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -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 diff --git a/docs/ship-policy.md b/docs/ship-policy.md index 436f59ead..0912b6415 100644 --- a/docs/ship-policy.md +++ b/docs/ship-policy.md @@ -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 ()` 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 ()` and stays `RECEIPT_CARRY_PRODUCT_CHANGED`. A changed path alone, or context paths alone, keep the routes above, and their output is unchanged. diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 797191f50..e6591d73a 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -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= packet=`: 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 ()` 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 diff --git a/lib/ship-patch.ts b/lib/ship-patch.ts index 15f0c7d1f..2e48aad90 100644 --- a/lib/ship-patch.ts +++ b/lib/ship-patch.ts @@ -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> = {}): { text: string; over: string[]; sha256: string } { const over = new Set(); 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'); diff --git a/lib/ship-receipt-depth.ts b/lib/ship-receipt-depth.ts index 4772e54aa..58190ae69 100644 --- a/lib/ship-receipt-depth.ts +++ b/lib/ship-receipt-depth.ts @@ -533,7 +533,7 @@ export function bindShippedReceipt(root: string, baseRef: string, body: string, * commit since `from` that origin/ 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; diff --git a/test/review-plan-merged-range.test.ts b/test/review-plan-merged-range.test.ts new file mode 100644 index 000000000..cf103c7e4 --- /dev/null +++ b/test/review-plan-merged-range.test.ts @@ -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) { + 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, 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:'); + }); +}); diff --git a/test/ship-rereview-triggers.test.ts b/test/ship-rereview-triggers.test.ts index 5d9805c51..e3116eb6d 100644 --- a/test/ship-rereview-triggers.test.ts +++ b/test/ship-rereview-triggers.test.ts @@ -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]); + }); +});