ship-receipt carry: changed and moved-context paths compose into one bounded re-check (GST-1561) (#4054)

* test: failing-first repro for the changed+context carry deadlock (GST-1561)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* ship-receipt carry: a changed path plus moved base context compose into one bounded re-check, and a base-side removed line joins the scoped review (GST-1561)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: pre-landing review fixes

baseEditOnly refuses a hunk whose range holds a line the base inserted without replacing any, so a merge that drops base code stays changed. Curates the new test out of the Windows scoped shard like ship-mechanical-retake.

Review-Finding: p1/specialist-batched#1 reader=testing
Review-Finding: p1/red-team#1 reader=red-team
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* fix: pre-landing review fixes

baseEditOnly refuses any base hunk at the reviewed range that adds more lines than it removes, so a merge that drops a line the base added beside its own edit stays changed.

Review-Finding: p2/red-team#1 reader=red-team
Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

* docs: ship-policy names the base-added-line case that stays a changed path (GST-1561)

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>
This commit is contained in:
Garry Tanandcapy-ai[bot] authored and GitHub committed 2026-11-06 07:02:54 +00:00
1 parent 12d18a2e10
commit 4b1a115284
9 files changed
+323 -38

No files matched your search

+9 -6
View File
@@ -50,7 +50,7 @@ import {
} from '../lib/ship-receipt';
import { checkDocsCaps, docsCapLine, docsExceptionLine, parseReceiptDocs } from '../lib/ship-receipt-docs';
import { comparePath, contextPacket, PACKET_SECTION_CAP, readScopedReview, touchedPaths } from '../lib/ship-patch';
import { planRecheck, readRecheck, recheckPacket } from '../lib/ship-recheck';
import { planRecheck, readRecheck, recheckPacket, recheckRecordTemplate } from '../lib/ship-recheck';
import { carryExcludeBinds, deeper, depthBelow, exceptionHead, exceptionLoopProblem, exceptionProblems, parseReviewLoopException, parseReviewTargeted, plainDocsSince, platformReceiptDepth, receiptExclude, receiptReviewAtHead, receiptTargetedAtHead, reviewLoopExceptionLine, targetedClassesSince, writeExcludeBinds } from '../lib/ship-receipt-depth';
import { RISK_CLASSES, type RiskClass } from '../lib/review-risk';
import { checkReceiptProse, proseLine } from '../lib/ship-receipt-prose';
@@ -843,21 +843,24 @@ function cmdCarry(rest: string[], json: boolean): number {
const fullRange = rangeRaw.status === 0 && inter?.status === 0 ? `${rangeRaw.stdout.trim()}\n${inter.stdout.trim()}` : null;
const numstat = named.length ? spawnSync('git', ['diff', '--numstat', from, head, '--', ...named.map(p => `:(literal)${p}`)], { cwd: repoRoot, encoding: 'utf8', timeout: 30_000, maxBuffer: 16 * 1024 * 1024 }) : null;
const binary = numstat?.status !== 0 || (numstat?.stdout ?? '').split('\n').some(l => l.startsWith('-\t-\t'));
const r = planRecheck({ code: 'RECEIPT_CARRY_PRODUCT_CHANGED', ancestor, retarget, rangeDiff: fullRange, unnamedPaths: named.length !== changedProduct.length, legacy: !receipt.review_depth, walkCut, contextPaths: contextPaths.length, binary });
// Moved base context composes into the same packet, bound to range_base (GST-1561).
const context = contextPaths.length ? contextPacket(repoRoot, { reviewedBase: rangeBase, reviewed: from, currentBase: headBase, current: head }, contextPaths) : undefined;
const bound = context ? { rangeBase, context: context.text } : {};
const r = planRecheck({ code: 'RECEIPT_CARRY_PRODUCT_CHANGED', ancestor, retarget, rangeDiff: fullRange, unnamedPaths: named.length !== changedProduct.length, legacy: !receipt.review_depth, walkCut, contextPaths: contextPaths.length, context, binary });
const record = flag(rest, '--recheck');
if (!r.ok) process.stderr.write(`RECHECK: not eligible (${r.reason}); run /ship on ${head.slice(0, 12)}\n`);
else if (record) {
const read = readRecheck(record, { from, head, sha256: r.sha256 });
const read = readRecheck(record, { from, head, sha256: r.sha256, ...bound });
if (!read.ok) return refused('RECEIPT_CARRY_REFUSED', `--recheck: ${read.message}`);
recheck = `scoped re-check by ${read.record.reader} (range-diff ${r.lines} lines)`;
recheck = `scoped re-check by ${read.record.reader} (range-diff ${r.lines} lines${context ? `; base context moved in ${contextPaths.join(', ')}; range_base ${rangeBase.slice(0, 12)}` : ''})`;
} else {
let where = 'not written (dry run)';
if (!rest.includes('--dry-run')) {
const packet = path.resolve(repoRoot, git(['rev-parse', '--git-path', `gstack/recheck-${head.slice(0, 12)}.md`], repoRoot) ?? `.git/gstack/recheck-${head.slice(0, 12)}.md`);
try { fs.mkdirSync(path.dirname(packet), { recursive: true }); fs.writeFileSync(packet, recheckPacket({ from, head, plan: r, rangeDiff: fullRange! })); where = packet; }
try { fs.mkdirSync(path.dirname(packet), { recursive: true }); fs.writeFileSync(packet, recheckPacket({ from, head, plan: r, rangeDiff: fullRange!, ...bound })); where = packet; }
catch (e) { where = `unwritable: ${(e as Error).message.split('\n')[0]}`; }
}
process.stderr.write(`RECHECK: eligible range_diff_lines=${r.lines} range_diff_sha256=${r.sha256} packet=${where}; one reader reads it, then carry --recheck <record.json> ({"from":"${from}","head":"${head}","range_diff_sha256":"${r.sha256}","reader":"<model>","verdict":"clean"}); a blocking finding means /ship on ${head.slice(0, 12)}\n`);
process.stderr.write(`RECHECK: eligible range_diff_lines=${r.lines} range_diff_sha256=${r.sha256} packet=${where}; one reader reads it, then carry --recheck <record.json> (${recheckRecordTemplate({ from, head, sha256: r.sha256, ...bound })}); a blocking finding means /ship on ${head.slice(0, 12)}\n`);
}
}
plan = planCarry({ from, head, changed, productPatchId: { from: reviewed as string, head: current as string }, testPatchId: { from: testFrom, head: testHead }, changedProduct, mergeCommits: Number(mergeCommits) || 0, rangeDiff, ...(retarget ? { retargetedFrom: reviewedBase } : {}), ...(stackParent ? { stackParent } : {}), ...(scopedReview ? { scopedReview } : {}), ...(stampedRows.length ? { stampedRows } : {}), ...(recheck ? { recheck } : {}) });
+1
View File
@@ -0,0 +1 @@
- **A receipt carry with both a changed path and a moved-context path gets one scoped re-check instead of deadlocking.** founderstar#563 merged a base that took `SAVE_VERSION := 83` under the branch's reviewed `:= 84` and also moved a block next to a reviewed hunk. Carry called the constant file changed, J1's re-check refused because context had moved, and the scoped review opened only when nothing had changed, so every rules-number PR needed a fresh /ship. Now a hunk whose added lines are byte-identical and whose removed lines differ only by the base's edit of the lines the branch replaces counts as a context path, so carry and finish put it in the scoped-review packet for one reader and never carry it silently. When real changed paths and context paths come together, carry writes one packet: the range-diff, the changed paths' diff, then the context hunks. The existing 400-line and 64 KB caps count the whole packet, `range_diff_sha256` binds all of it, and the reader's record names `range_base`, which the carry reason logs. Past the caps, carry refuses as before. A changed path alone, or context paths alone, keep their routes with unchanged output, and a branch-side change to an added line still needs review. (GST-1561)
+28 -1
View File
@@ -1933,12 +1933,39 @@ until one fix-check reviewer clears the packet; a working tree with uncommitted
changes is [`RECEIPT_CARRY_REFUSED`](troubleshooting.md#receipt-carry-refused).
A `RECEIPT_CARRY_PRODUCT_CHANGED` on linear history against the reviewed base
whose range-diff, plus the diff of the changed product paths, is at most 400
lines and 64 KB, with no binary path, no moved base context and a receipt that
lines and 64 KB, with no binary path and a receipt that
records its review depth, also prints `RECHECK: eligible … packet=<file>`: one reader on that
packet and `carry --recheck <record.json>` rebind the receipt instead of a
full second /ship, unless HEAD needs a deeper review depth or another risk
class ([scoped re-check](troubleshooting.md#receipt-carry-recheck), J1).
**Changed and context paths together (GST-1561).** When the base also moved
context next to reviewed hunks in other paths, the scoped review and J1 compose
into one bounded re-check instead of refusing each other (founderstar#563 hit
this on every rules-number PR). The one packet holds the range-diff and the
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
`RECEIPT_CARRY_PRODUCT_CHANGED`. A changed path alone, or context paths alone,
keep the routes above, and their output is unchanged.
The same scoped review also covers a hunk whose only difference from the
reviewed one is the base's edit of the lines the branch replaces. The base took
`SAVE_VERSION := 83` while the branch's `:= 84` line, reviewed against `82`,
stays byte-identical. The branch's added lines must match byte for byte, and
the lines just outside the reviewed hunk's removed range must be lines the base
kept, landing just outside the current hunk's range (`lib/ship-patch.ts`
`comparePath`). The path then counts as a context path. Carry and finish put it
in the scoped-review packet, and the reader checks that the branch's value
still holds next to the base's. It never carries without a reader. A hunk whose
added lines changed, that now also removes a line the base never touched, or
whose range holds a line the base added (a base hunk at or next to the replaced
lines that adds more lines than it removes) stays a changed path. A [mechanical retake](#mechanical-retake) does not apply
here, since the branch retook nothing.
**Rewritten history.** `write` records `head_tree` (the reviewed head's tree)
and `product_patch_id` (its product patch against the base), and every carry
rebinds both to the new head and keeps the earlier trees in `carry.trees`.
+13 -5
View File
@@ -6464,7 +6464,9 @@ the range-diff evidence ([unattended.md](unattended.md#the-terminal-line)).
**Meaning.** Every product path the branch touches has the reviewed +/- lines
in the same places, but the base edited context lines next to some of them, so
the 3-line patches differ (GST-384: main reworded the line above the PR's
hunk). The merge may have spliced the two, as in a JSON entry where the branch
hunk). A hunk whose added lines are unchanged and whose removed lines differ only
by the base's edit of the lines the branch replaces counts too (GST-1561: the base
took `SAVE_VERSION := 83` under the branch's `:= 84`). 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
@@ -6506,15 +6508,21 @@ against the current one is at most 400 lines (`RECHECK_RANGE_DIFF_CAP` in
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, base
context moved next to reviewed hunks, an empty or unreadable range-diff and a
`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
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
whole packet, `range_diff_sha256` binds all of it, and the RECHECK line's record
adds `"range_base"`.
**Fix.** Dispatch one reader on the packet (`.git/gstack/recheck-<head12>.md`;
`--dry-run` writes none). When it comes back clean, write
`{"from":"<reviewed head>","head":"<HEAD>","range_diff_sha256":"<from the RECHECK line>","reader":"<model>","verdict":"clean"}`
and rerun `gstack-ship-receipt carry ... --recheck <record.json>`. The carry
reason records the reader and the range-diff size. Carry still refuses when
(plus `"range_base"` when the line's record names one; a record without it then
refuses) and rerun `gstack-ship-receipt carry ... --recheck <record.json>`. The carry
reason records the reader and the range-diff size, and with moved context the
context paths and `range_base`. Carry still refuses when
HEAD needs a deeper review depth or a different risk class than the receipt
recorded; that, or a blocking finding, means /ship on the new head. In /ship
Step 1, `gstack-ship-preflight` prints this as `NEXT: carry route (Step 1.4, J1)`.
+24 -5
View File
@@ -9,7 +9,10 @@
* base merge only shifted line numbers; patch-ids ignore positions, so a block moved to an identical
* context elsewhere is changed);
* - context: the +/- lines match hunk for hunk, and each hunk sits where the base's own
* change (reviewedBase -> currentBase) puts it, so only context lines the base edited differ.
* change (reviewedBase -> currentBase) puts it, so only context lines the base edited differ;
* or a hunk's added lines match byte for byte and its removed lines are the base's own edit of
* the lines the reviewed hunk replaced (founderstar#563: the base took SAVE_VERSION 83 under the
* branch's 84, GST-1561), which the scoped review reads too.
* The merge may still have spliced the two (a JSON entry where the branch changed one key and
* the base the next), so a context path needs a scoped review of its hunks, never a silent pass;
* - changed: anything else, including the same lines moved to another place, a file-mode change, and any path whose
@@ -122,10 +125,26 @@ export function comparePath(repoRoot: string, s: PatchSides, rel: string): PathV
const c0 = zeroHunks(pathDiff(repoRoot, s.currentBase, s.current, rel, 0) ?? '');
if (ids[0] === ids[1] && r0 === null) return 'same';
const b0 = zeroHunks(pathDiff(repoRoot, s.reviewedBase, s.currentBase, rel, 0) ?? '');
if (!r0?.length || !c0 || !b0 || r0.length !== c0.length || r0.some((h, i) => h.body !== c0[i]!.body)) return 'changed';
if (!r0?.length || !c0 || !b0 || r0.length !== c0.length) return 'changed';
const map = mapper(b0);
if (!r0.every((h, i) => sameSpot(h, c0[i]!, map))) return 'changed';
return ids[0] === ids[1] ? 'same' : 'context';
if (!r0.every((h, i) => h.body === c0[i]!.body ? sameSpot(h, c0[i]!, map) : baseEditOnly(h, c0[i]!, map, b0))) return 'changed';
return ids[0] === ids[1] && r0.every((h, i) => h.body === c0[i]!.body) ? 'same' : 'context';
}
/**
* Whether a hunk whose body changed since review differs only by the base's edit of the lines it replaces: the added
* lines are the reviewed ones byte for byte, and the lines just outside the reviewed hunk's removed range are lines the
* base kept, landing just outside the current hunk's range, so the current removed lines are the base's version of the
* reviewed ones. A hunk that now removes another line, or adds anything else, is not, and neither is one whose range
* holds a line the base added before, inside or after the reviewed lines (a base hunk there that adds more lines than
* it removes): dropping it deletes base code rather than taking the base's edit.
*/
function baseEditOnly(r: ZeroHunk, c: ZeroHunk, map: (line: number) => number | null, base: ZeroHunk[]): boolean {
const added = (h: ZeroHunk) => h.body.split('\n').filter((l, i, all) => l.startsWith('+') || (l.startsWith('\\') && all[i - 1]?.startsWith('+'))).join('\n');
const start = (h: ZeroHunk) => (h.oldCount > 0 ? h.oldStart : h.oldStart + 1);
const end = start(r) + r.oldCount;
if (base.some(b => b.newCount > b.oldCount && b.oldStart >= start(r) - 1 && b.oldStart < end)) return false;
return r.oldCount > 0 && added(r) === added(c) && map(start(r) - 1) === start(c) - 1 && map(end) === start(c) + c.oldCount;
}
/**
@@ -178,7 +197,7 @@ export function contextPacket(repoRoot: string, s: PatchSides, paths: readonly s
const late = Object.keys(bounded).sort();
const sides = `reviewed ${s.reviewed.slice(0, 12)} against ${s.reviewedBase.slice(0, 12)}; current ${s.current.slice(0, 12)} against ${s.currentBase.slice(0, 12)}.`;
const parts = [paths.length || !late.length
? `# Scoped review: base context moved next to the reviewed hunks\n\n${sides} 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`
? `# Scoped review: base context moved next to the reviewed hunks\n\n${sides} The branch's added lines are unchanged, and a removed line differs only where the base edited the line the branch replaces; read whether the merge kept their meaning next to the base's edit (a version or number the base also took). Report clean, or findings at the blocking floor.\n`
: `# Scoped review: edits that landed after the review bound\n\n${sides}\n`];
if (late.length) parts.push(`Bounded edits since review in ${late.map(p => `${p} (${bounded[p]})`).join(', ')}: each landed after the review bound, and its kind was proven from the diff; this one read replaces another review round (docs/review-policy.md#bounded-post-review-edits). Read every line under "Changed since review" as gate code. Audit prose is policy text agents follow: flag any change to what a gate, cap, reviewer, approval or exception does. A lanes edit maps files to tests: flag a glob that marks files covered by tests that do not exercise them. A bookkeeping row is an exemption or a limit: flag a row that moves an exemption onto code it should not cover, or raises a limit without a reason in the diff. Read them through the targeted brief at the end for money, gate, auth and secret wording. Report clean, or findings at the blocking floor.\n`);
if (tests.length) parts.push(`Test code changed since review in ${tests.join(', ')}: read whether each test still checks what it did (no weakened or bypassed assertion, early return, new exemption, skip or mock). Report clean, or findings at the blocking floor.\n`);
+31 -16
View File
@@ -8,9 +8,14 @@
* Qualifies: RECEIPT_CARRY_PRODUCT_CHANGED on linear history (the receipt's head is an ancestor of HEAD) against the same base.
* Does not qualify: RECEIPT_CARRY_TREE_MISMATCH (rewritten history: the reviewed commits are gone, so there is no range to
* diff), a retargeted stack (the reviewed range was against another base), a legacy receipt with no review_depth, more
* changed product paths than the carry walk names, a path it cannot name, a binary path, base context moved next to reviewed hunks,
* an unreadable or empty range-diff, a packet over RECHECK_RANGE_DIFF_CAP lines or RECHECK_PACKET_BYTES bytes, and every RECEIPT_CARRY_REFUSED / RECEIPT_CARRY_PREGATE_STALE (those need their inputs
* repaired, not a review).
* changed product paths than the carry walk names, a path it cannot name, a binary path, base context moved next to reviewed hunks
* without its scoped-review sections (or with one over PACKET_SECTION_CAP), an unreadable or empty range-diff, a packet over
* RECHECK_RANGE_DIFF_CAP lines or RECHECK_PACKET_BYTES bytes, and every RECEIPT_CARRY_REFUSED / RECEIPT_CARRY_PREGATE_STALE (those need
* their inputs repaired, not a review).
*
* Moved base context composes (GST-1561, founderstar#563): the context paths' scoped-review sections (lib/ship-patch.ts contextPacket)
* join the range-diff in the one packet, the caps count both, the verdict binds both, and the record also names range_base, the
* commit the reviewed range starts from.
*/
import { createHash } from 'node:crypto';
import * as fs from 'node:fs';
@@ -27,8 +32,10 @@ export type RecheckInput = {
binary?: boolean;
/** The changed-path walk stopped at CARRY_NAMED_PATHS, so later changed product paths would be missing from the packet. */
walkCut?: boolean;
/** Paths whose only difference is base context next to reviewed hunks: they need RECEIPT_CARRY_SCOPED_REVIEW's packet, not this one. */
/** Paths whose only difference is base context next to reviewed hunks: the packet needs their scoped-review sections (`context`). */
contextPaths?: number;
/** contextPacket's sections for those paths, and every path with a section over PACKET_SECTION_CAP. */
context?: { text: string; over: readonly string[] };
};
export type RecheckPlan = { ok: true; lines: number; sha256: string } | { ok: false; reason: string };
@@ -43,39 +50,47 @@ export function planRecheck(i: RecheckInput): RecheckPlan {
if (i.walkCut) return { ok: false, reason: 'more changed product paths than the walk names, so the packet would miss some' };
if (i.unnamedPaths) return { ok: false, reason: 'a changed product path could not be named, so the packet would miss it' };
if (i.binary) return { ok: false, reason: 'a changed product path is binary, so the packet cannot show its contents' };
if (i.contextPaths) return { ok: false, reason: 'base context also moved next to reviewed hunks, which needs the scoped review of those hunks' };
if (i.contextPaths && !i.context) return { ok: false, reason: 'base context also moved next to reviewed hunks, which needs the scoped review of those hunks' };
if (i.context?.over.length) return { ok: false, reason: `base context next to reviewed hunks in ${i.context.over.join(', ')} is over the scoped review's section cap` };
if (i.rangeDiff === null) return { ok: false, reason: 'git range-diff failed or timed out' };
const text = i.rangeDiff.trim();
if (!text) return { ok: false, reason: 'git range-diff is empty, so it shows nothing for a reader to check' };
if (!i.rangeDiff.trim()) return { ok: false, reason: 'git range-diff is empty, so it shows nothing for a reader to check' };
const text = packetMaterial(i.rangeDiff, i.context?.text);
const lines = text.split('\n').length;
if (lines > RECHECK_RANGE_DIFF_CAP) return { ok: false, reason: `git range-diff is ${lines} lines, over the scoped re-check cap of ${RECHECK_RANGE_DIFF_CAP}` };
if (lines > RECHECK_RANGE_DIFF_CAP) return { ok: false, reason: `${i.context ? 'the packet' : 'git range-diff'} is ${lines} lines, over the scoped re-check cap of ${RECHECK_RANGE_DIFF_CAP}` };
const bytes = Buffer.byteLength(text);
if (bytes > RECHECK_PACKET_BYTES) return { ok: false, reason: `the packet is ${bytes} bytes, over the scoped re-check cap of ${RECHECK_PACKET_BYTES}` };
return { ok: true, lines, sha256: sha(text) };
}
/** The packet the one reader reads: what to judge, the record to write, then the range-diff verbatim. */
export function recheckPacket(o: { from: string; head: string; plan: Extract<RecheckPlan, { ok: true }>; rangeDiff: string }): string {
/** What range_diff_sha256 binds: the range-diff, then any moved base context's scoped-review sections. */
const packetMaterial = (rangeDiff: string, context?: string) => (context ? `${rangeDiff.trim()}\n\n${context.trim()}` : rangeDiff.trim());
/** The clean record a reader writes; range_base only when base context composes into the packet. */
export const recheckRecordTemplate = (o: { from: string; head: string; sha256: string; rangeBase?: string }) =>
JSON.stringify({ from: o.from, head: o.head, ...(o.rangeBase ? { range_base: o.rangeBase } : {}), range_diff_sha256: o.sha256, reader: '<model>', verdict: 'clean' });
/** The packet the one reader reads: what to judge, the record to write, then the range-diff verbatim and any moved base context. */
export function recheckPacket(o: { from: string; head: string; plan: Extract<RecheckPlan, { ok: true }>; rangeDiff: string; context?: string; rangeBase?: string }): string {
return [
`# Scoped re-check: the product patch changed since the reviewed head\n`,
`reviewed ${o.from}; current ${o.head}. Below is \`git range-diff\` of the reviewed range against the current one, then the diff of the changed product paths between the two heads (${o.plan.lines} lines).`,
`reviewed ${o.from}${o.context ? ` (range_base ${o.rangeBase})` : ''}; current ${o.head}. Below is \`git range-diff\` of the reviewed range against the current one, then the diff of the changed product paths between the two heads${o.context ? ', then the hunks where base context moved next to the reviewed ones' : ''} (${o.plan.lines} lines).`,
'Review only what changed between the two ranges, against the blocking floor: would fail a test, break production, or lose data or secrets.',
`Reply NO FINDINGS, or one finding per line with path:line, Severity and Blocks. A clean reply is recorded as {"from":"${o.from}","head":"${o.head}","range_diff_sha256":"${o.plan.sha256}","reader":"<model>","verdict":"clean"}.`,
`Reply NO FINDINGS, or one finding per line with path:line, Severity and Blocks. A clean reply is recorded as ${recheckRecordTemplate({ ...o, sha256: o.plan.sha256 })}.`,
'A blocking finding means the full /ship on the current head.\n',
'```diff', o.rangeDiff.trim(), '```', '',
'```diff', o.rangeDiff.trim(), '```', '', ...(o.context ? [o.context.trim(), ''] : []),
].join('\n');
}
export interface RecheckRecord { from: string; head: string; range_diff_sha256: string; reader: string; verdict: 'clean' }
export interface RecheckRecord { from: string; head: string; range_base?: string; range_diff_sha256: string; reader: string; verdict: 'clean' }
/** A clean re-check record bound to this carry's from, head and range-diff, or why it does not stand. */
export function readRecheck(file: string, bind: { from: string; head: string; sha256: string }): { ok: true; record: RecheckRecord } | { ok: false; message: string } {
export function readRecheck(file: string, bind: { from: string; head: string; sha256: string; rangeBase?: string }): { ok: true; record: RecheckRecord } | { ok: false; message: string } {
let v: Record<string, unknown>;
try { v = JSON.parse(fs.readFileSync(file, 'utf8')); } catch (e) { return { ok: false, message: `${file} is not readable JSON (${(e as Error).message.split('\n')[0]})` }; }
if (!v || typeof v !== 'object') return { ok: false, message: `${file} is not a JSON object` };
if (v.verdict !== 'clean') return { ok: false, message: `the re-check is ${JSON.stringify(v.verdict)}, not clean: run /ship on ${bind.head.slice(0, 12)}` };
if (typeof v.reader !== 'string' || !v.reader.trim()) return { ok: false, message: 'the re-check names no reader' };
for (const [k, want] of [['from', bind.from], ['head', bind.head], ['range_diff_sha256', bind.sha256]] as const) {
for (const [k, want] of [['from', bind.from], ['head', bind.head], ['range_diff_sha256', bind.sha256], ...(bind.rangeBase ? [['range_base', bind.rangeBase] as const] : [])] as const) {
if (v[k] !== want) return { ok: false, message: `the re-check is for ${k} ${String(v[k] ?? 'none').slice(0, 12)}, not ${want.slice(0, 12)}` };
}
return { ok: true, record: v as unknown as RecheckRecord };
+4
View File
@@ -411,6 +411,10 @@ export const KNOWN_WINDOWS_INCOMPATIBLE: Array<{ file: string; reason: string }>
file: 'test/ship-mechanical-retake.test.ts',
reason: 'drives bin/gstack-ship-release, gstack-review-plan, gstack-ship-receipt, gstack-docs-candidate and gstack-wtree (shebangs) through test/helpers/ship-bin-fixture.ts like test/ship-finish-stack-parent.test.ts; windows-latest fails every case at the first bin spawn (windows-scoped-shard, run 42156939158)',
},
{
file: 'test/ship-receipt-carry-mixed.test.ts',
reason: 'drives bin/gstack-ship-receipt, gstack-ship-release, gstack-docs-candidate and gstack-wtree (shebangs) through test/helpers/ship-bin-fixture.ts like test/ship-mechanical-retake.test.ts; windows-latest fails every case at the first bin spawn (windows-scoped-shard, run 42167292675)',
},
];
// Force-include overrides: files a WINDOWS_FRAGILE_PATTERNS regex excludes for
+202
View File
@@ -0,0 +1,202 @@
/**
* GST-1561 (founderstar#563): a receipt carry whose diff holds both a changed product path and a context path had no
* bounded route. RECEIPT_CARRY_SCOPED_REVIEW opened only when no path changed, and J1's scoped re-check refused whenever
* base context moved, so a base merge that bumped a constant the branch also replaces (SAVE_VERSION := 82 became 83 on
* the base; the branch's own `:= 84` line is byte-identical) next to a moved base block forced a full /ship every time.
* Here: a hunk whose only difference is a base-side removed line takes the scoped review with the context paths (finish
* and carry), a changed path plus context paths take one composed re-check packet bound to range_base, past the size cap
* it refuses as before, and a branch-side change to an added line still needs review.
*/
import { afterEach, describe, expect, setDefaultTimeout, test } from 'bun:test';
import * as fs from 'node:fs';
import * as path from 'node:path';
import { EXIT } from '../lib/headless-artifacts';
import { comparePath } from '../lib/ship-patch';
import { extractReceipt } from '../lib/ship-receipt';
import { RECHECK_RANGE_DIFF_CAP } from '../lib/ship-recheck';
import { lineOf, RELEASE, shipFixture, type ShipFx } from './helpers/ship-bin-fixture';
setDefaultTimeout(120_000);
const BIN = path.dirname(RELEASE);
const RECEIPT = path.join(BIN, 'gstack-ship-receipt');
const LANE = 'tests=printf " 3 pass\\n 0 fail\\nRan 3 tests across 1 file. [5.00ms]\\n"';
const READER = 'anthropic/claude-opus-5-5';
const ships: ShipFx[] = [];
afterEach(() => { while (ships.length) ships.pop()!.cleanup(); });
const GAME = 'scripts/core/game.gd';
const VESTING = 'scripts/core/vesting.gd';
const SCORE = 'scripts/core/score.gd';
const game = (save: number) => ['extends Node', '', `const SAVE_VERSION := ${save}`, '', 'func _ready():', '\tload_save()', '', 'func load_save():', '\tpass', ''].join('\n') + '\n';
const vesting = (edit: Record<number, string> = {}) => Array.from({ length: 12 }, (_, i) => edit[i + 1] ?? `var v${i + 1} := ${i + 1}`).join('\n') + '\n';
const BRANCH_VESTING = { 6: 'func was_vesting_clear(): return granted and not revoked' };
const BASE_VESTING = { 4: 'var v4 := 40 # moved by the base' };
const UNATTENDED = { GSTACK_SESSION_KIND: 'unattended' };
/** A branch reviewed at its first commit (receipt written by gstack-ship-receipt write, so it records review_depth). */
function reviewedBranch(base: Record<string, string>, feat: Record<string, string>) {
const f = shipFixture(base, feat); ships.push(f);
const from = f.git('rev-parse', 'HEAD').stdout.trim();
const reviewedTree = f.run(path.join(BIN, 'gstack-wtree'), []).stdout.trim();
const reviewedBase = f.git('merge-base', 'origin/main', 'HEAD').stdout.trim();
const body = path.join(f.root, 'body.md');
fs.writeFileSync(body, '## Summary\n');
const w = f.run(RECEIPT, ['write', '--pr', '5', '--base', 'main', '--body', body], UNATTENDED);
expect([w.status, w.stderr.slice(0, 400)]).toEqual([EXIT.ok, expect.any(String)]);
expect(extractReceipt(fs.readFileSync(body, 'utf8')).receipt!.review_depth).toBeDefined();
const carry = (extra: string[] = []) => f.run(RECEIPT, ['carry', '--body', body, '--base', 'main', ...extra], UNATTENDED);
return { f, from, reviewedTree, reviewedBase, body, carry };
}
/** Merge origin/main; a conflict takes `resolved` and commits the merge. */
function mergeMain(f: ShipFx, resolved: Record<string, string>) {
f.git('fetch', '-q', 'origin');
if (f.git('merge', '-q', '--no-edit', 'origin/main').status !== 0) { f.write(resolved); f.commit('merge main'); }
}
/** founderstar#563: the base took SAVE_VERSION 83 under the branch's 84, and moved a block next to the branch's vesting hunk. */
function founderstar(resolvedSave = 84, baseBlock = 0) {
const x = reviewedBranch({ [GAME]: game(82), [VESTING]: vesting(), [SCORE]: 'var score := 1\n' }, { [GAME]: game(84), [VESTING]: vesting(BRANCH_VESTING) });
const moved = { 4: [BASE_VESTING[4], ...Array.from({ length: baseBlock }, (_, i) => `var base${i} := ${i}`)].join('\n') };
x.f.advanceBase({ [GAME]: game(83), [VESTING]: vesting(moved) });
mergeMain(x.f, { [GAME]: game(resolvedSave), [VESTING]: vesting({ ...moved, ...BRANCH_VESTING }) });
const head = x.f.git('rev-parse', 'HEAD').stdout.trim();
const sides = { reviewedBase: x.reviewedBase, reviewed: x.from, currentBase: x.f.git('merge-base', 'origin/main', 'HEAD').stdout.trim(), current: head };
return { ...x, head, sides };
}
const scopedPacket = (err: string) => /one fix-check reviewer reads (\S+), then rerun carry/.exec(err)?.[1];
const eligible = (err: string) => /^RECHECK: eligible range_diff_lines=(\d+) range_diff_sha256=([0-9a-f]{64}) packet=(\S+);/m.exec(err);
const recordAt = (f: ShipFx, name: string, v: object) => { const file = path.join(f.root, name); fs.writeFileSync(file, JSON.stringify(v)); return file; };
describe('a base-side removed line next to byte-identical added lines (GST-1561 ask 2)', () => {
test('comparePath routes it with the context paths; a branch-side added-line change stays changed', () => {
const same = founderstar();
expect(comparePath(same.f.work, same.sides, GAME)).toBe('context');
expect(comparePath(same.f.work, same.sides, VESTING)).toBe('context');
const moved = founderstar(85);
expect(comparePath(moved.f.work, moved.sides, GAME)).toBe('changed');
});
test('a branch that now also removes a line the base never touched stays changed', () => {
const x = reviewedBranch({ [GAME]: game(82) }, { [GAME]: game(84) });
x.f.advanceBase({ [GAME]: game(83) });
mergeMain(x.f, { [GAME]: game(84).replace('\tload_save()\n', '') });
const head = x.f.git('rev-parse', 'HEAD').stdout.trim();
const sides = { reviewedBase: x.reviewedBase, reviewed: x.from, currentBase: x.f.git('merge-base', 'origin/main', 'HEAD').stdout.trim(), current: head };
expect(comparePath(x.f.work, sides, GAME)).toBe('changed');
});
for (const [where, inserted] of [['before', game(82).replace('const SAVE', 'const BASE_ADD := 1\nconst SAVE')], ['after', game(82).replace(':= 82\n', ':= 82\nconst BASE_ADD := 1\n')], ['beside its own edit of', game(83).replace(':= 83\n', ':= 83\nconst BASE_ADD := 1\n')]] as const) {
test(`a merge that drops a line the base inserted ${where} the replaced line stays changed`, () => {
const x = reviewedBranch({ [GAME]: game(82) }, { [GAME]: game(84) });
x.f.advanceBase({ [GAME]: inserted });
mergeMain(x.f, { [GAME]: game(84) });
if (fs.readFileSync(path.join(x.f.work, GAME), 'utf8') !== game(84)) { x.f.write({ [GAME]: game(84) }); x.f.commit('drop the base line'); }
const head = x.f.git('rev-parse', 'HEAD').stdout.trim();
const sides = { reviewedBase: x.reviewedBase, reviewed: x.from, currentBase: x.f.git('merge-base', 'origin/main', 'HEAD').stdout.trim(), current: head };
expect(comparePath(x.f.work, sides, GAME)).toBe('changed');
});
}
});
describe('carry: the founderstar#563 shape takes one scoped reader, then binds', () => {
test('one packet holds the constant hunk and the moved context block; a clean record carries', () => {
const { f, head, carry, body } = founderstar();
const ask = carry();
expect([ask.status, ask.stdout.trim().split('\n')[0]], ask.stderr).toEqual([EXIT.refused, 'CARRY: refused (RECEIPT_CARRY_SCOPED_REVIEW)']);
const packet = scopedPacket(ask.stderr);
expect(packet, ask.stderr).toBeDefined();
const text = fs.readFileSync(packet!, 'utf8');
for (const s of ['-const SAVE_VERSION := 82', '-const SAVE_VERSION := 83', '+const SAVE_VERSION := 84', 'was_vesting_clear', 'moved by the base']) expect(text).toContain(s);
const partial = recordAt(f, 'partial.json', { head, paths: [VESTING], reviewer: READER, verdict: 'clean' });
expect(carry(['--scoped-review', partial]).stderr).toContain(`the scoped review does not cover ${GAME}`);
const ok = carry(['--scoped-review', recordAt(f, 'scoped.json', { head, paths: [GAME, VESTING], reviewer: READER, verdict: 'clean' })]);
expect([ok.status, ok.stderr.slice(0, 600)]).toEqual([EXIT.ok, expect.any(String)]);
const carried = extractReceipt(fs.readFileSync(body, 'utf8')).receipt!;
expect(carried.head).toBe(head);
expect(carried.carry!.reason).toContain(`scoped review by ${READER}`);
expect(carried.carry!.reason).toContain(GAME);
});
test('a branch-side change to the added line still forces review (never the scoped context route)', () => {
const { carry } = founderstar(85);
const r = carry();
expect(r.status).toBe(EXIT.refused);
expect(r.stdout).toContain('RECEIPT_CARRY_PRODUCT_CHANGED');
expect(r.stderr).toContain(GAME);
expect(r.stdout).not.toContain('RECEIPT_CARRY_SCOPED_REVIEW');
});
});
describe('carry: a changed path plus context paths compose into one bounded re-check (GST-1561 ask 1)', () => {
const composed = (baseBlock = 0) => {
const x = founderstar(84, baseBlock);
x.f.write({ [SCORE]: 'var score0 := 2\n' }); x.f.commit('fix: score');
return { ...x, head: x.f.git('rev-parse', 'HEAD').stdout.trim() };
};
test('one packet holds the range-diff and the context hunks, bound to range_base; a clean record carries', () => {
const { f, from, head, reviewedBase, carry, body } = composed();
const ask = carry();
expect([ask.status, ask.stdout.trim().split('\n')[0]], ask.stderr).toEqual([EXIT.refused, 'CARRY: refused (RECEIPT_CARRY_PRODUCT_CHANGED)']);
const m = eligible(ask.stderr);
expect(m, ask.stderr).not.toBeNull();
const rangeBase = /"range_base":"([0-9a-f]{40,64})"/.exec(ask.stderr)?.[1];
expect(rangeBase).toBe(reviewedBase);
const text = fs.readFileSync(m![3]!, 'utf8');
for (const s of ['var score0 := 2', 'was_vesting_clear', 'moved by the base', 'const SAVE_VERSION := 83', `range_base ${reviewedBase}`]) expect(text).toContain(s);
const record = { from, head, range_base: rangeBase, range_diff_sha256: m![2], reader: READER, verdict: 'clean' };
const noBase = carry(['--recheck', recordAt(f, 'nobase.json', { ...record, range_base: undefined })]);
expect([noBase.status, noBase.stdout.includes('RECEIPT_CARRY_REFUSED'), noBase.stderr.includes('range_base')]).toEqual([EXIT.refused, true, true]);
const ok = carry(['--recheck', recordAt(f, 'recheck.json', record)]);
expect([ok.status, ok.stderr.slice(0, 600)]).toEqual([EXIT.ok, expect.any(String)]);
expect(ok.stdout).toContain(`scoped re-check by ${READER}`);
const reason = extractReceipt(fs.readFileSync(body, 'utf8')).receipt!.carry!.reason;
expect(reason).toContain(`scoped re-check by ${READER}`);
expect(reason).toContain(VESTING);
expect(reason).toContain(`range_base ${reviewedBase.slice(0, 12)}`);
});
test('past the size cap it refuses as before: RECEIPT_CARRY_PRODUCT_CHANGED, re-check not eligible', () => {
const { carry } = composed(RECHECK_RANGE_DIFF_CAP);
const r = carry();
expect([r.status, r.stdout.trim().split('\n')[0]]).toEqual([EXIT.refused, 'CARRY: refused (RECEIPT_CARRY_PRODUCT_CHANGED)']);
expect(r.stderr).toMatch(/RECHECK: not eligible \(the packet is \d+ lines, over the scoped re-check cap/);
expect(eligible(r.stderr)).toBeNull();
});
test('changed-only keeps J1 as it is; context-only keeps the scoped review', () => {
const changed = reviewedBranch({ [SCORE]: 'var score := 1\n' }, { [SCORE]: 'var score := 2\n' });
changed.f.write({ [SCORE]: 'var score := 3\n' }); changed.f.commit('fix: score 3');
const c = changed.carry();
const m = eligible(c.stderr);
expect(m, c.stderr).not.toBeNull();
expect(c.stderr).not.toContain('"range_base"');
expect(fs.readFileSync(m![3]!, 'utf8')).not.toContain('# Scoped review');
const context = reviewedBranch({ [VESTING]: vesting() }, { [VESTING]: vesting(BRANCH_VESTING) });
context.f.advanceBase({ [VESTING]: vesting(BASE_VESTING) });
mergeMain(context.f, { [VESTING]: vesting({ ...BASE_VESTING, ...BRANCH_VESTING }) });
const s = context.carry();
expect([s.status, s.stdout.trim().split('\n')[0]]).toEqual([EXIT.refused, 'CARRY: refused (RECEIPT_CARRY_SCOPED_REVIEW)']);
expect(s.stderr).not.toContain('RECHECK:');
});
});
describe('finish: the founderstar#563 shape takes the scoped reader too', () => {
test('ROUTE sends nothing to behavior, and one clean scoped record lets finish go on', () => {
const x = founderstar();
const post = path.join(x.f.root, 'post.json');
expect(x.f.run(path.join(BIN, 'gstack-docs-candidate'), ['snapshot', '--out', post, '--audit-id', 'a1', '--mode', 'edit', '--base', 'main', '--docs', 'README.md']).status).toBe(0);
const finish = (extra: string[] = []) => x.f.run(RELEASE, ['finish', '--base', 'main', '--reviewed-tree', x.reviewedTree, '--docs-post', post, '--lane', LANE, '--reviewed-base', x.reviewedBase, ...extra]);
const ask = finish();
expect([ask.status, lineOf(ask.stdout, 'ROUTE')], ask.stdout).toEqual([3, expect.stringMatching(/ behavior=0 .* context=[12]$/)]);
const packet = /reads (\S+\.md)[ ,]/.exec(ask.stdout)?.[1];
expect(fs.readFileSync(packet!, 'utf8')).toContain('moved by the base');
const tree = x.f.run(path.join(BIN, 'gstack-wtree'), []).stdout.trim();
const ok = finish(['--context-reviewed', recordAt(x.f, 'ctx.json', { tree, paths: [GAME, VESTING], reviewer: READER, verdict: 'clean' })]);
expect(lineOf(ok.stdout, 'SCOPED_REVIEW'), ok.stdout).toStartWith(`SCOPED_REVIEW: clean by ${READER} (`);
expect(ok.stdout).not.toContain('STOP: behavior');
});
});
+11 -5
View File
@@ -2,8 +2,8 @@
* J1 (GSTA-258 L4): a scoped re-check after a refused receipt carry. Each refusal reason that does or does not qualify,
* the record binding, and the carry end to end: a bounded product change prints RECHECK: eligible and writes the packet,
* a clean record bound to that packet carries the receipt with the reader named, and a changed risk class, a legacy
* receipt, more paths than the walk names and moved base context never take the re-check (the /ship p1 red-team and
* security findings).
* receipt and more paths than the walk names never take the re-check (the /ship p1 red-team and security findings).
* Moved base context joins the packet instead (GST-1561; test/ship-receipt-carry-mixed.test.ts).
*/
import { afterEach, describe, expect, test } from 'bun:test';
import * as fs from 'node:fs';
@@ -32,7 +32,8 @@ describe('planRecheck: which refusals qualify', () => {
['legacy receipt', { ...OK, legacy: true }, 'no review_depth'],
['walk cut at the named-path limit', { ...OK, walkCut: true }, 'more changed product paths'],
['unnamed product path', { ...OK, unnamedPaths: true }, 'could not be named'],
['base context moved', { ...OK, contextPaths: 1 }, 'base context'],
['base context moved without its sections', { ...OK, contextPaths: 1 }, 'base context'],
['base context over its section cap', { ...OK, contextPaths: 1, context: { text: '## src/c.ts', over: ['src/c.ts'] } }, 'section cap'],
['binary product path', { ...OK, binary: true }, 'binary'],
['wide packet over the byte cap', { ...OK, rangeDiff: 'x'.repeat(RECHECK_PACKET_BYTES + 1) }, 'bytes'],
['range-diff failed', { ...OK, rangeDiff: null }, 'failed or timed out'],
@@ -138,7 +139,7 @@ describe('carry with a scoped re-check (CLI)', () => {
expect(runBin(bin.repo, 'gstack-ship-receipt', ['carry', '--body', bin.body], bin.env).err).toContain('RECHECK: not eligible (a changed product path is binary');
}, 90_000);
test('base context moved next to a reviewed hunk plus a product edit elsewhere is not eligible (red-team p1 repro)', () => {
test('base context moved next to a reviewed hunk plus a product edit elsewhere composes the context hunks into the packet (red-team p1 repro, GST-1561)', () => {
const lines = (n: number, edits: Record<number, string> = {}) => Array.from({ length: 10 }, (_, i) => edits[i + 1] ?? `line ${i + 1}`).join('\n') + '\n';
const repo = makeFixtureRepo({ prefix: 'ship-recheck-ctx-' }); repos.push(repo);
fs.mkdirSync(path.join(repo.work, 'src'), { recursive: true });
@@ -161,6 +162,11 @@ describe('carry with a scoped re-check (CLI)', () => {
repo.git('commit', '-qam', 'fix: app');
const r = runBin(repo, 'gstack-ship-receipt', ['carry', '--body', body], env);
expect(r.status).toBe(EXIT.refused);
expect(r.err).toContain('RECHECK: not eligible (base context also moved');
const m = eligible(r.err);
expect(m, r.err).not.toBeNull();
const packet = fs.readFileSync(m![3]!, 'utf8');
expect(packet).toContain('export const a = 2;');
expect(packet).toContain('## src/c.ts');
expect(packet).toContain('main line 4');
}, 60_000);
});