From f8085aa0e499085c411f712d6d5c00a81d1b104a Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Tue, 3 Nov 2026 12:21:34 -0800 Subject: [PATCH] v1.94.39.0 fix(ship): base-side lines don't count toward the scoped-review section limit (GST-1226) (#3955) * fix(ship-patch): only the branch's own lines count toward the scoped-review section cap (GST-1226) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(ship-patch): a base change past 2,000 lines counts in full; drop the text-matched context predicate (GST-1226 review round 1) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- CHANGELOG.md | 10 +++++ VERSION | 2 +- agents-digest/gstack-AGENTS.md | 2 +- docs/troubleshooting.md | 7 +++- lib/ship-patch.ts | 12 ++++-- package.json | 2 +- test/ship-rereview-triggers.test.ts | 57 +++++++++++++++++++++++++++++ 7 files changed, 84 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 01031fbd0..fd86d6db4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ # Changelog +## [1.94.39.0] - 2026-11-03 + +**A base merge next to a reviewed hunk no longer forces a second full review because of the base's own lines.** On founderstar#515, a base merge put 62 lines of another PR, already merged at base, next to the PR's 76 reviewed and unchanged lines in `tests/ui_shots.gd`. The scoped-review section went past its 200-line limit, so `gstack-ship-release finish` took the behavior route, which a review-loop exception can't accept, and asked for Steps 5 to 11.5 again. #3923 hit the same limit when only one trailing context line changed (GST-1226). + +### Fixed +- **The scoped-review packet counts only the branch's own lines toward its 200-line section limit.** The base's +/- lines, already merged at base, are still shown to the reader but no longer count, in finish and in the receipt carry. A base change over 2,000 lines in one file counts in full again, and a section the branch's own lines push past the limit still takes the full loop. Context-only drift is not carried silently: it still gets the scoped read. + +### Tests +- `test/ship-rereview-triggers.test.ts`: 210 lines another PR merged at base next to the branch's 76 reviewed lines, and a changed trailing context line beside a 221-line base rewrite, both reach the scoped read (both took the behavior route on main). Negative controls: a section the branch's own 230 lines push past the limit, and a 2,100-line base insertion beside the hunk, still take the behavior route. + ## [1.94.38.0] - 2026-11-03 - **A /ship receipt now survives a changelog-fragment conversion (GST-1210).** When a PR moved to a `changelog.d` fragment after review, `VERSION`, `package.json` and `CHANGELOG.md` went back to main's content, and if main had also changed `package.json` beyond its version, `gstack-ship-receipt carry` read that as the branch's change and refused with `RECEIPT_CARRY_PRODUCT_CHANGED` (HARN-1, harness-evals#206). A stamp file the branch changes by no more than a stamp against each head's base now carries as release metadata. A non-stamp `package.json` change, a logic edit, or a stamp file whose entry can't be read still refuses. diff --git a/VERSION b/VERSION index fb73d8cce..8f8262803 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.94.38.0 +1.94.39.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index 66661c8cf..045ce712a 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.94.38.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.94.39.0 — regenerate/re-copy after upgrading gstack Behavioral rules from gstack (https://github.com/garrytan/gstack), compressed for agent hosts without a full skill install. The full skills add workflows, diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 844bf6965..07cf2b847 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -6038,7 +6038,12 @@ hunk). The merge may have spliced the two, as in a JSON entry where the branch 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. +`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. **Fix.** Dispatch one fix-check reviewer ([review-policy.md](review-policy.md)) on the packet. When it comes back clean, diff --git a/lib/ship-patch.ts b/lib/ship-patch.ts index d069dc5cc..447c1a548 100644 --- a/lib/ship-patch.ts +++ b/lib/ship-patch.ts @@ -126,17 +126,20 @@ function hunkTexts(diff: string): string[] { /** The most 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). */ +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. + * 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. */ export function contextPacket(repoRoot: string, s: PatchSides, paths: readonly string[], tests: readonly string[] = []): { text: string; over: string[] } { const over = new Set(); let current = ''; - const fence = (label: string, body: string) => { - if (body.split('\n').length > PACKET_SECTION_CAP) over.add(current); + 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`; }; const parts = [`# Scoped review: base context moved next to the reviewed hunks\n\nreviewed ${s.reviewed.slice(0, 12)} against ${s.reviewedBase.slice(0, 12)}; current ${s.current.slice(0, 12)} against ${s.currentBase.slice(0, 12)}. The branch's +/- lines are unchanged; read whether the merge kept their meaning next to the base's edit. Report clean, or findings at the blocking floor.\n`]; @@ -145,8 +148,9 @@ export function contextPacket(repoRoot: string, s: PatchSides, paths: readonly s current = rel; 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`, 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", hunkTexts(pathDiff(repoRoot, s.reviewedBase, s.currentBase, rel, 3) ?? '').join(''))); + fence("The base's change", b, b.split('\n').length > BASE_SECTION_CEILING ? undefined : l => !/^[+-]/.test(l))); } return { text: parts.join('\n'), over: [...over] }; } diff --git a/package.json b/package.json index e37bb43a9..483c4a15b 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.94.38", + "version": "1.94.39", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module", diff --git a/test/ship-rereview-triggers.test.ts b/test/ship-rereview-triggers.test.ts index 3854c7808..b2ffa01ca 100644 --- a/test/ship-rereview-triggers.test.ts +++ b/test/ship-rereview-triggers.test.ts @@ -869,3 +869,60 @@ describe('finish after a completed fix-check (GST-1130, GEVA-218)', () => { expect(r.status).toBe(3); }); }); + +// Value: protects=a base merge that puts lines already merged at base next to the branch's reviewed, unchanged hunk reaches +// the scoped read even when those base lines push a packet section past PACKET_SECTION_CAP (founderstar#515, #3923); only the +// branch's own lines count toward the cap; fails_when=finish forces the behavior route for a section that is over only +// because of base lines, or lets a section the branch's own lines push over reach the scoped read; seam=contextPacket's count +describe('base lines do not count toward the scoped-review cap (GST-1226, founderstar#515, #3923)', () => { + const block = (name: string, n: number) => Array.from({ length: n }, (_, i) => `export const ${name}${i + 1} = ${i + 1};`); + const join = (rows: string[]) => rows.join('\n') + '\n'; + const at = (n: number, after: number, add: string[]) => { const rows = lines(n).trimEnd().split('\n'); rows.splice(after, 0, ...add); return rows; }; + const scoped = (r: { status: number | null; stdout: string }) => [r.status, lineOf(r.stdout, 'ROUTE').replace(/^.* behavior=/, 'behavior='), r.stdout.includes('STOP: base context moved next to the reviewed hunks in src/shots.ts'), r.stdout.includes('SCOPED_REVIEW: too large')]; + + test('210 lines another PR merged at base next to the branch\'s 76 reviewed lines reach the scoped read', () => { + const pr = block('pr', 76), other = block('other', 210); + const f = fx({ 'src/shots.ts': lines(20) }, { 'src/shots.ts': join(at(20, 10, pr)) }); + const reviewed = wtree(f), reviewedBase = mergeBase(f), post = snapshot(f); + f.advanceBase({ 'src/shots.ts': join(at(20, 11, other)) }); + const merged = at(20, 11, other); merged.splice(10, 0, ...pr); + mergeMain(f, { 'src/shots.ts': join(merged) }); + expect(f.git('show', 'HEAD:src/shots.ts').stdout).toBe(join(merged)); + expect(scoped(finish(f, reviewed, post, ['--reviewed-base', reviewedBase]))).toEqual([3, 'behavior=0 docs=0 metadata=0 tests=0 base=0 context=1', true, false]); + }); + + test('a changed trailing context line with the same +/- lines reaches the scoped read beside a 221-line base rewrite', () => { + const pr = block('pr', 76); + const f = fx({ 'src/shots.ts': lines(300) }, { 'src/shots.ts': join(at(300, 10, pr)) }); + const reviewed = wtree(f), reviewedBase = mergeBase(f), post = snapshot(f); + const edits: Record = { 12: 'export const l12 = 120;' }; + for (let i = 60; i <= 280; i++) edits[i] = `export const l${i} = ${i * 10};`; + f.advanceBase({ 'src/shots.ts': lines(300, edits) }); + const merged = lines(300, edits).trimEnd().split('\n'); merged.splice(10, 0, ...pr); + mergeMain(f, { 'src/shots.ts': join(merged) }); + expect(f.git('show', 'HEAD:src/shots.ts').stdout).toBe(join(merged)); + expect(scoped(finish(f, reviewed, post, ['--reviewed-base', reviewedBase]))).toEqual([3, 'behavior=0 docs=0 metadata=0 tests=0 base=0 context=1', true, false]); + }); + + test('a section the branch\'s own 230 lines push past the cap still takes the behavior route', () => { + const pr = block('pr', 230), other = block('other', 5); + const f = fx({ 'src/shots.ts': lines(20) }, { 'src/shots.ts': join(at(20, 10, pr)) }); + const reviewed = wtree(f), reviewedBase = mergeBase(f), post = snapshot(f); + f.advanceBase({ 'src/shots.ts': join(at(20, 11, other)) }); + const merged = at(20, 11, other); merged.splice(10, 0, ...pr); + mergeMain(f, { 'src/shots.ts': join(merged) }); + const r = finish(f, reviewed, post, ['--reviewed-base', reviewedBase]); + expect(scoped(r)).toEqual([3, 'behavior=1 docs=0 metadata=0 tests=0 base=0 context=0', false, true]); + expect(r.stdout.includes('STOP: behavior, tests or build inputs changed since review (src/shots.ts)')).toBe(true); + }); + + test('a base change past BASE_SECTION_CEILING counts in full again and takes the behavior route', () => { + const pr = block('pr', 76), other = block('other', 2100); + const f = fx({ 'src/shots.ts': lines(20) }, { 'src/shots.ts': join(at(20, 10, pr)) }); + const reviewed = wtree(f), reviewedBase = mergeBase(f), post = snapshot(f); + f.advanceBase({ 'src/shots.ts': join(at(20, 11, other)) }); + const merged = at(20, 11, other); merged.splice(10, 0, ...pr); + mergeMain(f, { 'src/shots.ts': join(merged) }); + 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]); + }); +});