mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-03 15:27:56 +01:00
v1.94.15.0 fix(ship-receipt): carry a stacked PR's receipt after its parent squash-merges into main (GST-1199) (#3928)
* fix(ship-receipt): carry a stacked PR's receipt after its parent squash-merges into main (GST-1199) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(ship-receipt): a commit with no product change is no landed stack parent (review fix, GST-1199) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * changelog: name the fragment by the branch slug (GST-1199) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * docs: troubleshooting names the base-written stacked carry (Step 14.5 audit, GST-1199) 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:
1 parent
8d276f9e31
commit
bf2bcf14b0
9 files changed
+158
-23
No files matched your search
@@ -1,5 +1,9 @@
|
|||||||
# Changelog
|
# Changelog
|
||||||
|
|
||||||
|
## [1.94.15.0] - 2026-11-03
|
||||||
|
|
||||||
|
- **A stacked PR keeps its /ship receipt after its parent squash-merges, even when the receipt was written against main (GST-1199).** `gstack-ship-receipt carry` used to refuse with `RECEIPT_CARRY_PRODUCT_CHANGED` once the parent's hunks moved into main, forcing a full re-review of unchanged code. The carry now finds the newest reviewed commit that main already holds (`git merge-tree` into main changes no product path) and compares from there: an unchanged child carries as `stack parent <sha12> landed`, base edits next to the child's hunks go to `RECEIPT_CARRY_SCOPED_REVIEW`, and any other change still refuses. A parent that never landed never counts, even if the child's merge dropped its change.
|
||||||
|
|
||||||
## [1.94.14.0] - 2026-11-03
|
## [1.94.14.0] - 2026-11-03
|
||||||
|
|
||||||
**`review_exclude_paths` keeps code it used to exclude, and the docs say exactly what the receipt check stops (GST-972).** Three ways to get code or a credential out of review under a base pattern are closed, each with a failing-first test in `test/review-exclude.test.ts`, and the receipt docs no longer over-claim.
|
**`review_exclude_paths` keeps code it used to exclude, and the docs say exactly what the receipt check stops (GST-972).** Three ways to get code or a credential out of review under a base pattern are closed, each with a failing-first test in `test/review-exclude.test.ts`, and the receipt docs no longer over-claim.
|
||||||
|
|||||||
@@ -1,4 +1,4 @@
|
|||||||
# gstack digest v1.94.14.0 — regenerate/re-copy after upgrading gstack
|
# gstack digest v1.94.15.0 — regenerate/re-copy after upgrading gstack
|
||||||
|
|
||||||
Behavioral rules from gstack (https://github.com/garrytan/gstack), compressed
|
Behavioral rules from gstack (https://github.com/garrytan/gstack), compressed
|
||||||
for agent hosts without a full skill install. The full skills add workflows,
|
for agent hosts without a full skill install. The full skills add workflows,
|
||||||
|
|||||||
+26
-15
@@ -45,7 +45,7 @@ import { isUnavailable, loadPolicyFromBase, matchesAny, policyUnavailableLine, r
|
|||||||
import { approvalForReceipt } from '../lib/approval-citations';
|
import { approvalForReceipt } from '../lib/approval-citations';
|
||||||
import {
|
import {
|
||||||
CARRY_METADATA_GLOBS, CARRY_NAMED_PATHS, CARRY_TEST_GLOBS,
|
CARRY_METADATA_GLOBS, CARRY_NAMED_PATHS, CARRY_TEST_GLOBS,
|
||||||
bindReceipt, carryReceipt, classifyCarryPath, computeCensus, extractReceipt, missingIssueLines, orderReceipt, parseReviewLoop, placeReceiptFirst, planCarry, planTreeCarry,
|
bindReceipt, carryReceipt, classifyCarryPath, computeCensus, extractReceipt, landedStackParent, missingIssueLines, orderReceipt, parseReviewLoop, placeReceiptFirst, planCarry, planTreeCarry,
|
||||||
receiptIssuesFrom, receiptSummaryLine, renderCensus, renderReceiptBlock, validateReceipt, type CensusPr,
|
receiptIssuesFrom, receiptSummaryLine, renderCensus, renderReceiptBlock, validateReceipt, type CensusPr,
|
||||||
} from '../lib/ship-receipt';
|
} from '../lib/ship-receipt';
|
||||||
import { checkDocsCaps, docsCapLine, docsExceptionLine, parseReceiptDocs } from '../lib/ship-receipt-docs';
|
import { checkDocsCaps, docsCapLine, docsExceptionLine, parseReceiptDocs } from '../lib/ship-receipt-docs';
|
||||||
@@ -662,18 +662,29 @@ function cmdCarry(rest: string[], json: boolean): number {
|
|||||||
} else {
|
} else {
|
||||||
const fromBase = git(['merge-base', baseRef, from], repoRoot);
|
const fromBase = git(['merge-base', baseRef, from], repoRoot);
|
||||||
if (!fromBase) return refused('RECEIPT_CARRY_REFUSED', `cannot resolve the merge base of ${baseRef} (fetch it first)`);
|
if (!fromBase) return refused('RECEIPT_CARRY_REFUSED', `cannot resolve the merge base of ${baseRef} (fetch it first)`);
|
||||||
const listed = [touchedPaths(repoRoot, fromBase, from), touchedPaths(repoRoot, headBase, head)];
|
// The reviewed range runs from the fork point, or from a stack parent inside it that has since landed on the base: a
|
||||||
if (!listed[0] || !listed[1]) return refused('RECEIPT_CARRY_REFUSED', `git could not list the paths the branch touched against ${base}; run /ship on ${head.slice(0, 12)}`);
|
// receipt written against the base while stacked reviewed the parent's hunks too, and the squash moved them into the
|
||||||
|
// base (GST-1199). A parent counts only when merging it into the current base changes no product path.
|
||||||
|
const measure = (rangeBase: string) => {
|
||||||
|
const listed = [touchedPaths(repoRoot, rangeBase, from), touchedPaths(repoRoot, headBase, head)];
|
||||||
|
if (!listed[0] || !listed[1]) return `git could not list the paths the branch touched against ${base}; run /ship on ${head.slice(0, 12)}`;
|
||||||
const touched = new Set([...listed[0], ...listed[1]]);
|
const touched = new Set([...listed[0], ...listed[1]]);
|
||||||
const spec = productSpec(repoRoot, rules, from, head, { touched, sides: [[fromBase, from], [headBase, head]], own: branchSides(repoRoot, baseRef, from, '') });
|
const spec = productSpec(repoRoot, rules, from, head, { touched, sides: [[rangeBase, from], [headBase, head]], own: branchSides(repoRoot, baseRef, from, '') });
|
||||||
if (!spec) return refused('RECEIPT_CARRY_REFUSED', `git diff ${from.slice(0, 12)} ${head.slice(0, 12)} failed`);
|
if (!spec) return `git diff ${from.slice(0, 12)} ${head.slice(0, 12)} failed`;
|
||||||
|
const ids = [patchId(repoRoot, rangeBase, from, spec.product), patchId(repoRoot, headBase, head, spec.product)];
|
||||||
|
for (const r of ids) if (typeof r !== 'string') return `git could not compute the product patch-id at one of the heads: ${r.error}`;
|
||||||
|
return { rangeBase, touched, spec, productFrom: ids[0] as string, productHead: ids[1] as string };
|
||||||
|
};
|
||||||
|
let measured = measure(fromBase);
|
||||||
|
if (typeof measured === 'string') return refused('RECEIPT_CARRY_REFUSED', measured);
|
||||||
|
const stackParent = !retarget && measured.productFrom !== measured.productHead ? landedStackParent(repoRoot, { rangeBase: fromBase, reviewed: from, currentBase: headBase }, measured.spec.product) : null;
|
||||||
|
if (stackParent) measured = measure(stackParent);
|
||||||
|
if (typeof measured === 'string') return refused('RECEIPT_CARRY_REFUSED', measured);
|
||||||
|
const { rangeBase, touched, spec, productFrom, productHead } = measured;
|
||||||
const { changed } = spec;
|
const { changed } = spec;
|
||||||
const productFrom = patchId(repoRoot, fromBase, from, spec.product);
|
|
||||||
const productHead = patchId(repoRoot, headBase, head, spec.product);
|
|
||||||
const asId = (r: string | { error: string }) => (typeof r === 'string' ? r : '');
|
const asId = (r: string | { error: string }) => (typeof r === 'string' ? r : '');
|
||||||
const testFrom = asId(patchId(repoRoot, fromBase, from, spec.tests));
|
const testFrom = asId(patchId(repoRoot, rangeBase, from, spec.tests));
|
||||||
const testHead = asId(patchId(repoRoot, headBase, head, spec.tests));
|
const testHead = asId(patchId(repoRoot, headBase, head, spec.tests));
|
||||||
for (const r of [productFrom, productHead]) if (typeof r !== 'string') return refused('RECEIPT_CARRY_REFUSED', `git could not compute the product patch-id at one of the heads: ${r.error}`);
|
|
||||||
// A stacked child whose parent landed on the base (#3570): the receipt reviewed the child's own range against the
|
// A stacked child whose parent landed on the base (#3570): the receipt reviewed the child's own range against the
|
||||||
// parent branch, and the PR now targets the base. That reviewed range (the recorded product patch-id, else the one
|
// parent branch, and the PR now targets the base. That reviewed range (the recorded product patch-id, else the one
|
||||||
// against origin/<receipt base> when it still resolves) must be HEAD's whole product diff against the base, computed
|
// against origin/<receipt base> when it still resolves) must be HEAD's whole product diff against the base, computed
|
||||||
@@ -697,8 +708,8 @@ function cmdCarry(rest: string[], json: boolean): number {
|
|||||||
// which can differ between the two base diffs. A touched path with the same blob at both heads and at both bases has
|
// which can differ between the two base diffs. A touched path with the same blob at both heads and at both bases has
|
||||||
// the same patch, so only paths that moved on one side are compared (GST-448: 6000 touched paths). A path whose only difference is base context next to unchanged hunks
|
// the same patch, so only paths that moved on one side are compared (GST-448: 6000 touched paths). A path whose only difference is base context next to unchanged hunks
|
||||||
// (lib/ship-patch.ts) needs a scoped review instead of the loop.
|
// (lib/ship-patch.ts) needs a scoped review instead of the loop.
|
||||||
const sides = { reviewedBase: fromBase, reviewed: from, currentBase: headBase, current: head };
|
const sides = { reviewedBase: rangeBase, reviewed: from, currentBase: headBase, current: head };
|
||||||
const moved = [touchedPaths(repoRoot, from, head), touchedPaths(repoRoot, fromBase, headBase)];
|
const moved = [touchedPaths(repoRoot, from, head), touchedPaths(repoRoot, rangeBase, headBase)];
|
||||||
if (moved.some(l => !l)) return refused('RECEIPT_CARRY_REFUSED', `git could not list the paths the branch touched against ${base}; run /ship on ${head.slice(0, 12)}`);
|
if (moved.some(l => !l)) return refused('RECEIPT_CARRY_REFUSED', `git could not list the paths the branch touched against ${base}; run /ship on ${head.slice(0, 12)}`);
|
||||||
const [movedHead, movedBase] = moved as Set<string>[];
|
const [movedHead, movedBase] = moved as Set<string>[];
|
||||||
// A stamp file the branch changed as product (its own row) whose reviewed-to-current change is a stamp on its own lines
|
// A stamp file the branch changed as product (its own row) whose reviewed-to-current change is a stamp on its own lines
|
||||||
@@ -716,14 +727,14 @@ function cmdCarry(rest: string[], json: boolean): number {
|
|||||||
if (!changedProduct.length && !left.length) changedProduct.push('(a product path outside the branch\'s touched files; compare the two base diffs)');
|
if (!changedProduct.length && !left.length) changedProduct.push('(a product path outside the branch\'s touched files; compare the two base diffs)');
|
||||||
else if (!changedProduct.length) {
|
else if (!changedProduct.length) {
|
||||||
const rest = [...spec.product, ...left.map(p => `:(exclude,literal)${p}`)];
|
const rest = [...spec.product, ...left.map(p => `:(exclude,literal)${p}`)];
|
||||||
const restFrom = patchId(repoRoot, fromBase, from, rest);
|
const restFrom = patchId(repoRoot, rangeBase, from, rest);
|
||||||
const restHead = patchId(repoRoot, headBase, head, rest);
|
const restHead = patchId(repoRoot, headBase, head, rest);
|
||||||
if (typeof restFrom !== 'string' || typeof restHead !== 'string' || restFrom !== restHead) changedProduct.push(`(a product path outside ${left.join(', ')}; compare the two base diffs without them)`);
|
if (typeof restFrom !== 'string' || typeof restHead !== 'string' || restFrom !== restHead) changedProduct.push(`(a product path outside ${left.join(', ')}; compare the two base diffs without them)`);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
const stampedRows = changedProduct.length ? [] : stampOnly;
|
const stampedRows = changedProduct.length ? [] : stampOnly;
|
||||||
let scopedReview: string | undefined;
|
let scopedReview: string | undefined;
|
||||||
const packetBody = !changedProduct.length && contextPaths.length ? contextPacket(repoRoot, { reviewedBase: fromBase, reviewed: from, currentBase: headBase, current: head }, contextPaths) : null;
|
const packetBody = !changedProduct.length && contextPaths.length ? contextPacket(repoRoot, { reviewedBase: rangeBase, reviewed: from, currentBase: headBase, current: head }, contextPaths) : null;
|
||||||
if (packetBody?.over.length) changedProduct.push(...packetBody.over.map(p => `${p} (too large for the scoped review: over ${PACKET_SECTION_CAP} lines in a section)`));
|
if (packetBody?.over.length) changedProduct.push(...packetBody.over.map(p => `${p} (too large for the scoped review: over ${PACKET_SECTION_CAP} lines in a section)`));
|
||||||
if (packetBody && !changedProduct.length) {
|
if (packetBody && !changedProduct.length) {
|
||||||
const record = flag(rest, '--scoped-review');
|
const record = flag(rest, '--scoped-review');
|
||||||
@@ -749,9 +760,9 @@ function cmdCarry(rest: string[], json: boolean): number {
|
|||||||
}
|
}
|
||||||
const mergeCommits = (git(['rev-list', '--merges', '--count', `${from}..${head}`], repoRoot) ?? '0').trim();
|
const mergeCommits = (git(['rev-list', '--merges', '--count', `${from}..${head}`], repoRoot) ?? '0').trim();
|
||||||
// Evidence for the record, never the decision: a slow range-diff is reported unavailable rather than waited for.
|
// Evidence for the record, never the decision: a slow range-diff is reported unavailable rather than waited for.
|
||||||
const rangeRaw = spawnSync('git', ['range-diff', '--no-color', `${fromBase}..${from}`, `${headBase}..${head}`], { cwd: repoRoot, encoding: 'utf8', timeout: 15_000, maxBuffer: 64 * 1024 * 1024 });
|
const rangeRaw = spawnSync('git', ['range-diff', '--no-color', `${rangeBase}..${from}`, `${headBase}..${head}`], { cwd: repoRoot, encoding: 'utf8', timeout: 15_000, maxBuffer: 64 * 1024 * 1024 });
|
||||||
const rangeDiff = rangeRaw.status === 0 ? (rangeRaw.stdout.trim().split('\n').slice(0, 20).join('\n') || '(empty: identical ranges)') : `unavailable: ${(rangeRaw.stderr ?? '').trim().split('\n')[0] || (rangeRaw.signal ? `range-diff killed by ${rangeRaw.signal} after 15 s` : 'range-diff failed')}`;
|
const rangeDiff = rangeRaw.status === 0 ? (rangeRaw.stdout.trim().split('\n').slice(0, 20).join('\n') || '(empty: identical ranges)') : `unavailable: ${(rangeRaw.stderr ?? '').trim().split('\n')[0] || (rangeRaw.signal ? `range-diff killed by ${rangeRaw.signal} after 15 s` : 'range-diff failed')}`;
|
||||||
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 } : {}), ...(scopedReview ? { scopedReview } : {}), ...(stampedRows.length ? { stampedRows } : {}) });
|
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 } : {}) });
|
||||||
}
|
}
|
||||||
if (!plan.ok) return refused(plan.code, plan.message);
|
if (!plan.ok) return refused(plan.code, plan.message);
|
||||||
// Eligibility is not publication: a receipt that recorded a pregate is rebound only with a pregate current for HEAD,
|
// Eligibility is not publication: a receipt that recorded a pregate is rebound only with a pregate current for HEAD,
|
||||||
|
|||||||
@@ -1507,6 +1507,7 @@ receipt when the base moves:
|
|||||||
| a rebase or force-push onto a tree the receipt covers, product diff against the new base unchanged | carries (`carry.ancestry: rewritten`) |
|
| a rebase or force-push onto a tree the receipt covers, product diff against the new base unchanged | carries (`carry.ancestry: rewritten`) |
|
||||||
| any other rebase or force-push | a fresh /ship (`RECEIPT_CARRY_TREE_MISMATCH`) |
|
| any other rebase or force-push | a fresh /ship (`RECEIPT_CARRY_TREE_MISMATCH`) |
|
||||||
| a stacked PR whose parent landed (squash or merge), retargeted to the base with the base merged in, HEAD's product diff exactly the one reviewed against the parent branch | carries (reason `stack parent <branch> landed`) |
|
| a stacked PR whose parent landed (squash or merge), retargeted to the base with the base merged in, HEAD's product diff exactly the one reviewed against the parent branch | carries (reason `stack parent <branch> landed`) |
|
||||||
|
| a stacked PR whose receipt was written against the base, after its parent landed and the base was merged in, HEAD's product diff exactly the reviewed range from the parent's head | carries (reason `stack parent <sha12> landed`); base context next to the child's hunks is `RECEIPT_CARRY_SCOPED_REVIEW` |
|
||||||
| a stamp on the branch's own rows in a stamp file the branch also changed as product: every changed line at a branch-added position, the file's mode unchanged, the base not touching it, the rest of the product diff unchanged | carries (reason `a stamp on the branch's own rows (<file>)`) |
|
| a stamp on the branch's own rows in a stamp file the branch also changed as product: every changed line at a branch-added position, the file's mode unchanged, the base not touching it, the rest of the product diff unchanged | carries (reason `a stamp on the branch's own rows (<file>)`) |
|
||||||
|
|
||||||
**Stacked PRs (#3570).** A child PR's receipt is written against its parent
|
**Stacked PRs (#3570).** A child PR's receipt is written against its parent
|
||||||
@@ -1524,6 +1525,21 @@ since, or a parent that landed with changes the child never merged all leave a
|
|||||||
different diff and refuse with `RECEIPT_CARRY_PRODUCT_CHANGED`. An older
|
different diff and refuse with `RECEIPT_CARRY_PRODUCT_CHANGED`. An older
|
||||||
receipt whose parent branch is gone refuses with `RECEIPT_CARRY_REFUSED`.
|
receipt whose parent branch is gone refuses with `RECEIPT_CARRY_REFUSED`.
|
||||||
|
|
||||||
|
**Stacked PRs reviewed against the base (GST-1199).** A child PR that targets
|
||||||
|
the base while stacked has a receipt whose `base` is the base and whose
|
||||||
|
reviewed range holds the parent's hunks too, and the receipt records no stack
|
||||||
|
parent. Once the parent squash-merges and the base is merged into the child,
|
||||||
|
the two heads' product diffs differ by exactly the parent's hunks, which now
|
||||||
|
sit in the base. When they differ, the carry looks for the newest commit inside
|
||||||
|
the reviewed range (the reviewed head excluded, at most 100 commits) whose
|
||||||
|
product change the current base already holds: `git merge-tree` of that commit
|
||||||
|
into the base changes no product path. A conflicted product path never counts,
|
||||||
|
so a parent that never landed is never treated as one, even when the child's
|
||||||
|
merge dropped its change. The comparison then runs from that commit: an equal
|
||||||
|
patch-id carries with reason `stack parent <sha12> landed`, base context next
|
||||||
|
to the child's unchanged hunks is `RECEIPT_CARRY_SCOPED_REVIEW`, and any other
|
||||||
|
difference refuses with `RECEIPT_CARRY_PRODUCT_CHANGED`.
|
||||||
|
|
||||||
Since the release that added `review_scope`. A `/ship` receipt names the head it
|
Since the release that added `review_scope`. A `/ship` receipt names the head it
|
||||||
reviewed, so a conflict-only merge of the base or a test-only fix used to
|
reviewed, so a conflict-only merge of the base or a test-only fix used to
|
||||||
invalidate it and cost a full re-run on identical product code.
|
invalidate it and cost a full re-run on identical product code.
|
||||||
|
|||||||
@@ -5984,7 +5984,10 @@ the reviewed tree but the base moved under it, so the diff a reviewer would see
|
|||||||
is not the reviewed one. For a stacked PR retargeted to the base after its
|
is not the reviewed one. For a stacked PR retargeted to the base after its
|
||||||
parent landed, it means HEAD's product diff against the base is not the range
|
parent landed, it means HEAD's product diff against the base is not the range
|
||||||
reviewed against the parent branch: the parent has not fully landed, or the
|
reviewed against the parent branch: the parent has not fully landed, or the
|
||||||
branch changed since. The reviewed verdict does not cover HEAD, so the
|
branch changed since. A stacked PR whose receipt was written against the base
|
||||||
|
carries from the newest reviewed commit the base already holds
|
||||||
|
([ship-policy.md](ship-policy.md#receipt-carry)); it refuses here only when no
|
||||||
|
such commit exists or the branch changed since. The reviewed verdict does not cover HEAD, so the
|
||||||
receipt is not carried. The refusal names the changed product paths, or for a
|
receipt is not carried. The refusal names the changed product paths, or for a
|
||||||
retargeted stacked PR the parent branch and the base.
|
retargeted stacked PR the parent branch and the base.
|
||||||
|
|
||||||
|
|||||||
+28
-1
@@ -6,6 +6,7 @@
|
|||||||
* computes the P7 adoption census over merged PRs (deduplicated by run and
|
* computes the P7 adoption census over merged PRs (deduplicated by run and
|
||||||
* thread id against an owner-maintained thread cohort).
|
* thread id against an owner-maintained thread cohort).
|
||||||
*/
|
*/
|
||||||
|
import { spawnSync } from 'node:child_process';
|
||||||
import { ARTIFACT_SCHEMAS, checkSchema, type ShipReceipt, type ValidationError } from './headless-artifacts';
|
import { ARTIFACT_SCHEMAS, checkSchema, type ShipReceipt, type ValidationError } from './headless-artifacts';
|
||||||
import { approvalSummary } from './approval-citations';
|
import { approvalSummary } from './approval-citations';
|
||||||
import { FRAGMENT_README } from './changelog-fragments';
|
import { FRAGMENT_README } from './changelog-fragments';
|
||||||
@@ -177,6 +178,8 @@ export interface CarryPlanInput {
|
|||||||
rangeDiff: string;
|
rangeDiff: string;
|
||||||
/** The base the receipt was reviewed against when it differs from the carry's (a stack parent that landed, #3570): productPatchId.from is then the recorded diff against it. */
|
/** The base the receipt was reviewed against when it differs from the carry's (a stack parent that landed, #3570): productPatchId.from is then the recorded diff against it. */
|
||||||
retargetedFrom?: string;
|
retargetedFrom?: string;
|
||||||
|
/** A commit inside the reviewed range whose product change the base already holds (a stack parent that squash-merged under a receipt written against the base, GST-1199): productPatchId.from is then the range from it. */
|
||||||
|
stackParent?: string;
|
||||||
/** A clean scoped review of product paths whose only difference is base context next to unchanged hunks (lib/ship-patch.ts, GST-384): the carry reason it adds. */
|
/** A clean scoped review of product paths whose only difference is base context next to unchanged hunks (lib/ship-patch.ts, GST-384): the carry reason it adds. */
|
||||||
scopedReview?: string;
|
scopedReview?: string;
|
||||||
/** Stamp files the branch changed as product whose only change since the reviewed head is a stamp on the branch's own rows, the base untouched, and without which the two product patches match (GST-980). */
|
/** Stamp files the branch changed as product whose only change since the reviewed head is a stamp on the branch's own rows, the base untouched, and without which the two product patches match (GST-980). */
|
||||||
@@ -253,7 +256,8 @@ export function planCarry(input: CarryPlanInput): CarryPlan {
|
|||||||
const named = input.changedProduct.slice(0, CARRY_NAMED_PATHS);
|
const named = input.changedProduct.slice(0, CARRY_NAMED_PATHS);
|
||||||
return { ok: false, code: 'RECEIPT_CARRY_PRODUCT_CHANGED', message: `product diff changed between ${input.from.slice(0, 12)} and ${input.head.slice(0, 12)}: ${named.join(', ') || 'patch-id differs'}${input.changedProduct.length > named.length ? ` (+${input.changedProduct.length - named.length} more)` : ''}` };
|
return { ok: false, code: 'RECEIPT_CARRY_PRODUCT_CHANGED', message: `product diff changed between ${input.from.slice(0, 12)} and ${input.head.slice(0, 12)}: ${named.join(', ') || 'patch-id differs'}${input.changedProduct.length > named.length ? ` (+${input.changedProduct.length - named.length} more)` : ''}` };
|
||||||
}
|
}
|
||||||
const reasons: string[] = input.retargetedFrom ? [`stack parent ${input.retargetedFrom} landed`] : [];
|
const parent = input.retargetedFrom ?? input.stackParent?.slice(0, 12);
|
||||||
|
const reasons: string[] = parent ? [`stack parent ${parent} landed`] : [];
|
||||||
if (input.mergeCommits > 0) reasons.push(input.mergeCommits === 1 ? 'base merge' : `${input.mergeCommits} base merges`);
|
if (input.mergeCommits > 0) reasons.push(input.mergeCommits === 1 ? 'base merge' : `${input.mergeCommits} base merges`);
|
||||||
if (input.scopedReview && input.productPatchId.from !== input.productPatchId.head) reasons.push(input.scopedReview);
|
if (input.scopedReview && input.productPatchId.from !== input.productPatchId.head) reasons.push(input.scopedReview);
|
||||||
if (input.stampedRows?.length && input.productPatchId.from !== input.productPatchId.head) reasons.push(`a stamp on the branch's own rows (${input.stampedRows.join(', ')})`);
|
if (input.stampedRows?.length && input.productPatchId.from !== input.productPatchId.head) reasons.push(`a stamp on the branch's own rows (${input.stampedRows.join(', ')})`);
|
||||||
@@ -271,6 +275,29 @@ export function planCarry(input: CarryPlanInput): CarryPlan {
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** How many commits of the reviewed range landedStackParent tries, newest first. */
|
||||||
|
export const STACK_PARENT_WALK = 100;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The newest commit inside the reviewed range (rangeBase..reviewed, the reviewed head itself excluded) whose product
|
||||||
|
* change the current base already holds: merging it into currentBase changes no product path. That is a stack parent
|
||||||
|
* that squash-merged (or merged) into the base after the receipt was written against the base (GST-1199). A commit with no
|
||||||
|
* product change of its own (a test-first commit) is no parent, since any base trivially holds it. Null when no
|
||||||
|
* commit within STACK_PARENT_WALK qualifies or git cannot tell; a conflicted product path is never held.
|
||||||
|
*/
|
||||||
|
export function landedStackParent(repoRoot: string, s: { rangeBase: string; reviewed: string; currentBase: string }, product: readonly string[]): string | null {
|
||||||
|
const run = (args: string[]) => spawnSync('git', args, { cwd: repoRoot, encoding: 'utf8', timeout: 120_000, maxBuffer: 64 * 1024 * 1024 });
|
||||||
|
const commits = run(['rev-list', '--topo-order', `--max-count=${STACK_PARENT_WALK + 1}`, `${s.rangeBase}..${s.reviewed}`]);
|
||||||
|
if (commits.status !== 0) return null;
|
||||||
|
for (const commit of commits.stdout.split('\n').filter(c => c && c !== s.reviewed)) {
|
||||||
|
if (run(['diff', '--quiet', s.rangeBase, commit, '--', ...product]).status !== 1) continue;
|
||||||
|
const merged = run(['merge-tree', '--write-tree', '--no-messages', s.currentBase, commit]);
|
||||||
|
const tree = merged.status === 0 || merged.status === 1 ? merged.stdout.split('\n')[0]?.trim() : undefined;
|
||||||
|
if (tree && run(['diff', '--quiet', s.currentBase, tree, '--', ...product]).status === 0) return commit;
|
||||||
|
}
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
export interface TreeCarryInput {
|
export interface TreeCarryInput {
|
||||||
from: string; head: string;
|
from: string; head: string;
|
||||||
/** HEAD^{tree}, and every tree the receipt already covers (its head_tree plus earlier carries' trees). */
|
/** HEAD^{tree}, and every tree the receipt already covers (its head_tree plus earlier carries' trees). */
|
||||||
|
|||||||
+1
-1
@@ -1,6 +1,6 @@
|
|||||||
{
|
{
|
||||||
"name": "gstack",
|
"name": "gstack",
|
||||||
"version": "1.94.14",
|
"version": "1.94.15",
|
||||||
"description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.",
|
"description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.",
|
||||||
"license": "MIT",
|
"license": "MIT",
|
||||||
"type": "module",
|
"type": "module",
|
||||||
|
|||||||
@@ -8,7 +8,7 @@ import { afterEach, describe, expect, test } from 'bun:test';
|
|||||||
import * as fs from 'node:fs';
|
import * as fs from 'node:fs';
|
||||||
import * as path from 'node:path';
|
import * as path from 'node:path';
|
||||||
import { EXIT } from '../lib/headless-artifacts';
|
import { EXIT } from '../lib/headless-artifacts';
|
||||||
import { extractReceipt, renderReceiptBlock } from '../lib/ship-receipt';
|
import { extractReceipt, landedStackParent, renderReceiptBlock } from '../lib/ship-receipt';
|
||||||
import { bindShippedReceipt } from '../lib/ship-receipt-depth';
|
import { bindShippedReceipt } from '../lib/ship-receipt-depth';
|
||||||
import { cleanup, commit, makeFixtureRepo, runBin, write, type FixtureRepo } from './helpers/pregate-fixture';
|
import { cleanup, commit, makeFixtureRepo, runBin, write, type FixtureRepo } from './helpers/pregate-fixture';
|
||||||
|
|
||||||
@@ -21,7 +21,7 @@ const SUB = "exports.add = (a, b) => a + b;\nexports.sub = (a, b) => a - b;\n";
|
|||||||
const RETRY4 = "exports.MAX_RETRIES = 4;\nexports.TIMEOUT_MS = 5000;\nexports.retry = (fn) => fn();\n";
|
const RETRY4 = "exports.MAX_RETRIES = 4;\nexports.TIMEOUT_MS = 5000;\nexports.retry = (fn) => fn();\n";
|
||||||
|
|
||||||
/** main <- parent (lib/util.js) <- feature (lib/retry.js, its /ship receipt written against the parent branch). */
|
/** main <- parent (lib/util.js) <- feature (lib/retry.js, its /ship receipt written against the parent branch). */
|
||||||
function stack(repo: FixtureRepo, parentFiles: Record<string, string> = { 'lib/util.js': SUB }, childFiles: Record<string, string> = { 'lib/retry.js': RETRY4 }): { parent: string; child: string; body: string } {
|
function stack(repo: FixtureRepo, parentFiles: Record<string, string> = { 'lib/util.js': SUB }, childFiles: Record<string, string> = { 'lib/retry.js': RETRY4 }, base = 'parent'): { parent: string; child: string; body: string } {
|
||||||
repo.git('checkout', '-q', '-b', 'parent');
|
repo.git('checkout', '-q', '-b', 'parent');
|
||||||
const parent = commit(repo, parentFiles, 'parent: sub');
|
const parent = commit(repo, parentFiles, 'parent: sub');
|
||||||
repo.git('push', '-q', 'origin', 'parent');
|
repo.git('push', '-q', 'origin', 'parent');
|
||||||
@@ -29,7 +29,7 @@ function stack(repo: FixtureRepo, parentFiles: Record<string, string> = { 'lib/u
|
|||||||
repo.git('reset', '-q', '--hard', parent);
|
repo.git('reset', '-q', '--hard', parent);
|
||||||
const child = commit(repo, childFiles, 'child: four retries');
|
const child = commit(repo, childFiles, 'child: four retries');
|
||||||
const body = path.join(repo.root, 'body.md');
|
const body = path.join(repo.root, 'body.md');
|
||||||
const w = runBin(repo, 'gstack-ship-receipt', ['write', '--pr', '8', '--base', 'parent', '--version', '1.0.0', '--session-kind', 'unattended', '--body', body]);
|
const w = runBin(repo, 'gstack-ship-receipt', ['write', '--pr', '8', '--base', base, '--version', '1.0.0', '--session-kind', 'unattended', '--body', body]);
|
||||||
expect(w.status).toBe(EXIT.ok);
|
expect(w.status).toBe(EXIT.ok);
|
||||||
return { parent, child, body };
|
return { parent, child, body };
|
||||||
}
|
}
|
||||||
@@ -166,3 +166,77 @@ describe('carry after a stack parent squash-merges (#3570)', () => {
|
|||||||
expect(receiptIn(body).head).toBe(child);
|
expect(receiptIn(body).head).toBe(child);
|
||||||
}, 90_000);
|
}, 90_000);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
const CONFIG = ['a = 1', 'b = 2', 'c = 3', 'd = 4', 'e = 5', 'f = 6', 'g = 7', 'h = 8', ''].join('\n');
|
||||||
|
const configWith = (edits: Record<string, string>) => CONFIG.split('\n').map(l => edits[l] ?? l).join('\n');
|
||||||
|
|
||||||
|
describe('carry after a stack parent squash-merges, receipt written against main (GST-1199)', () => {
|
||||||
|
// Value: protects=a child PR stacked on its parent but targeting main keeps its receipt once the parent squash-merges and
|
||||||
|
// main is merged in (gstack-evals#178 faced a 45-60M-token gate re-review for it); fails_when=carry compares the child's
|
||||||
|
// whole range from the old fork point, parent hunks included, with HEAD's diff against a base that now holds them;
|
||||||
|
// seam=none; no_claim=gh pr edit (--body stands in)
|
||||||
|
test('the child carries when its range from the landed parent is HEAD\'s whole product diff against main', () => {
|
||||||
|
const repo = makeFixtureRepo(); repos.push(repo);
|
||||||
|
const { parent, child, body } = stack(repo, undefined, undefined, 'main');
|
||||||
|
landOnMain(repo, { 'lib/util.js': SUB });
|
||||||
|
const head = repo.git('rev-parse', 'HEAD');
|
||||||
|
const out = carry(repo, body);
|
||||||
|
expect(out.err).not.toContain('RECEIPT_CARRY_PRODUCT_CHANGED');
|
||||||
|
expect(out.status).toBe(EXIT.ok);
|
||||||
|
expect(out.out).toContain(`CARRY: carried from ${child.slice(0, 12)} to ${head.slice(0, 12)}`);
|
||||||
|
const r = receiptIn(body);
|
||||||
|
expect(r.head).toBe(head);
|
||||||
|
expect(r.base).toBe('main');
|
||||||
|
expect(r.carry!.reason).toBe(`stack parent ${parent.slice(0, 12)} landed + base merge`);
|
||||||
|
expect(bindShippedReceipt(repo.work, 'origin/main', fs.readFileSync(body, 'utf8'), '8', [head]).ok).toBe(true);
|
||||||
|
}, 90_000);
|
||||||
|
|
||||||
|
// Value: protects=fail closed: a product edit after the receipt, or a parent that never landed and whose change the
|
||||||
|
// child's merge dropped, takes a fresh /ship; fails_when=any reviewed ancestor counts as a landed parent, or the edit
|
||||||
|
// hides behind the shorter range; seam=none; no_claim=-
|
||||||
|
test('a product edit after the receipt, or a dropped parent that never landed, refuses with RECEIPT_CARRY_PRODUCT_CHANGED', () => {
|
||||||
|
const repo = makeFixtureRepo(); repos.push(repo);
|
||||||
|
const { child, body } = stack(repo, undefined, undefined, 'main');
|
||||||
|
landOnMain(repo, { 'lib/util.js': SUB });
|
||||||
|
commit(repo, { 'lib/retry.js': RETRY4.replace('4', '5') }, 'child: five');
|
||||||
|
const edited = carry(repo, body);
|
||||||
|
expect(edited.status).toBe(EXIT.refused);
|
||||||
|
expect(edited.out).toContain('CARRY: refused (RECEIPT_CARRY_PRODUCT_CHANGED)');
|
||||||
|
expect(receiptIn(body).head).toBe(child);
|
||||||
|
|
||||||
|
const other = makeFixtureRepo(); repos.push(other);
|
||||||
|
const s = stack(other, undefined, undefined, 'main');
|
||||||
|
landOnMain(other, {});
|
||||||
|
commit(other, { 'lib/util.js': other.git('show', 'origin/main:lib/util.js') + '\n' }, 'drop the parent change');
|
||||||
|
const dropped = carry(other, s.body);
|
||||||
|
expect(dropped.status).toBe(EXIT.refused);
|
||||||
|
expect(dropped.out).toContain('CARRY: refused (RECEIPT_CARRY_PRODUCT_CHANGED)');
|
||||||
|
expect(receiptIn(s.body).head).toBe(s.child);
|
||||||
|
}, 120_000);
|
||||||
|
|
||||||
|
// Value: protects=a base change next to the child's reviewed hunks gets one fix-check reviewer, not a carry and not a full
|
||||||
|
// /ship; fails_when=the landed-parent comparison ignores base context or refuses it outright; seam=none; no_claim=-
|
||||||
|
test('a base change next to the child\'s reviewed hunks routes to RECEIPT_CARRY_SCOPED_REVIEW', () => {
|
||||||
|
const repo = makeFixtureRepo({ baseFiles: { 'lib/config.js': CONFIG } }); repos.push(repo);
|
||||||
|
const { child, body } = stack(repo, undefined, { 'lib/config.js': configWith({ 'd = 4': 'd = 40' }) }, 'main');
|
||||||
|
landOnMain(repo, { 'lib/util.js': SUB, 'lib/config.js': configWith({ 'f = 6': 'f = 60' }) });
|
||||||
|
const out = carry(repo, body);
|
||||||
|
expect(out.status).toBe(EXIT.refused);
|
||||||
|
expect(out.out).toContain('CARRY: refused (RECEIPT_CARRY_SCOPED_REVIEW)');
|
||||||
|
expect(out.err).toContain('lib/config.js');
|
||||||
|
expect(receiptIn(body).head).toBe(child);
|
||||||
|
}, 90_000);
|
||||||
|
|
||||||
|
// Value: protects=the receipt never names a stack parent that does not exist; fails_when=a test-first commit with no
|
||||||
|
// product change counts as landed (any base holds it), so the reason claims a parent and the test range shrinks; seam=none;
|
||||||
|
// no_claim=-
|
||||||
|
test('a test-only commit in a branch that was never stacked is no landed parent', () => {
|
||||||
|
const repo = makeFixtureRepo({ baseFiles: { 'lib/config.js': CONFIG } }); repos.push(repo);
|
||||||
|
const fork = repo.git('rev-parse', 'HEAD');
|
||||||
|
commit(repo, { 'test/config.test.js': "const test = require('node:test');\ntest('d', () => {});\n" }, 'test first');
|
||||||
|
const reviewed = commit(repo, { 'lib/config.js': configWith({ 'd = 4': 'd = 40' }) }, 'fix d');
|
||||||
|
repo.git('checkout', '-q', 'main');
|
||||||
|
const mainTip = commit(repo, { 'docs/README.md': '# main moves on\n' }, 'main moves on');
|
||||||
|
expect(landedStackParent(repo.work, { rangeBase: fork, reviewed, currentBase: mainTip }, ['.', ':(exclude,glob)test/**'])).toBeNull();
|
||||||
|
}, 60_000);
|
||||||
|
});
|
||||||
Reference in new issue
Block a user