mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-03 23:37:53 +01:00
v1.94.21.0 fix(ship-receipt): a stacked carry ignores files the child's own earlier main merge brought in (GST-1214) (#3940)
* fix(ship-receipt): a stacked carry ignores files the child's own earlier main merge brought in (GST-1214) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * changelog: GST-1214 fragment Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * fix(ship-receipt): never leave out a path with a compared path beneath it (review fix, GST-1214) 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
a1da77e6d4
commit
95fc2105ff
7 files changed
+89
-9
No files matched your search
@@ -1,5 +1,9 @@
|
|||||||
# Changelog
|
# Changelog
|
||||||
|
|
||||||
|
## [1.94.21.0] - 2026-11-03
|
||||||
|
|
||||||
|
- **A stacked PR's /ship receipt now carries even when the child merged main before its receipt was written (GST-1214).** After the parent squash-landed and main was merged in again, `gstack-ship-receipt carry` compared the range from the parent's head, which still held every file the child's first main merge brought in, and refused with `RECEIPT_CARRY_PRODUCT_CHANGED` naming those files. The carry now leaves out paths whose content at the reviewed head is main's, unless HEAD changed them. A real edit to one of those files still refuses.
|
||||||
|
|
||||||
## [1.94.20.0] - 2026-11-03
|
## [1.94.20.0] - 2026-11-03
|
||||||
|
|
||||||
### Changed
|
### Changed
|
||||||
|
|||||||
@@ -1,4 +1,4 @@
|
|||||||
# gstack digest v1.94.20.0 — regenerate/re-copy after upgrading gstack
|
# gstack digest v1.94.21.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,
|
||||||
|
|||||||
+13
-5
@@ -666,11 +666,19 @@ function cmdCarry(rest: string[], json: boolean): number {
|
|||||||
// receipt written against the base while stacked reviewed the parent's hunks too, and the squash moved them into the
|
// 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.
|
// base (GST-1199). A parent counts only when merging it into the current base changes no product path.
|
||||||
const measure = (rangeBase: string) => {
|
const measure = (rangeBase: string) => {
|
||||||
const listed = [touchedPaths(repoRoot, rangeBase, from), touchedPaths(repoRoot, headBase, head)];
|
const listed = [touchedPaths(repoRoot, rangeBase, from), touchedPaths(repoRoot, headBase, head), rangeBase === fromBase ? new Set<string>() : touchedPaths(repoRoot, fromBase, from)];
|
||||||
if (!listed[0] || !listed[1]) return `git could not list the paths the branch touched against ${base}; run /ship on ${head.slice(0, 12)}`;
|
if (!listed[0] || !listed[1] || !listed[2]) 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]]);
|
// From a stack parent, a path whose reviewed content is the base's (an earlier base merge brought it in) is base
|
||||||
const spec = productSpec(repoRoot, rules, from, head, { touched, sides: [[rangeBase, from], [headBase, head]], own: branchSides(repoRoot, baseRef, from, '') });
|
// context, not reviewed product, unless HEAD changed it against the base (GST-1214).
|
||||||
if (!spec) return `git diff ${from.slice(0, 12)} ${head.slice(0, 12)} failed`;
|
// A pathspec also names everything beneath a path, so a path with any compared path under it (a file the base turned
|
||||||
|
// into a directory) stays in.
|
||||||
|
const compared = [...listed[0], ...listed[1]];
|
||||||
|
const baseMerged = rangeBase === fromBase ? [] : [...listed[0]].filter(p => !listed[2]!.has(p) && !listed[1]!.has(p) && !compared.some(q => q.startsWith(`${p}/`)));
|
||||||
|
const touched = new Set([...listed[0], ...listed[1]].filter(p => !baseMerged.includes(p)));
|
||||||
|
const found = productSpec(repoRoot, rules, from, head, { touched, sides: [[rangeBase, from], [headBase, head]], own: branchSides(repoRoot, baseRef, from, '') });
|
||||||
|
if (!found) return `git diff ${from.slice(0, 12)} ${head.slice(0, 12)} failed`;
|
||||||
|
const skip = baseMerged.map(p => `:(exclude,literal)${p}`);
|
||||||
|
const spec = { ...found, product: [...found.product, ...skip], tests: [...found.tests, ...skip] };
|
||||||
const ids = [patchId(repoRoot, rangeBase, from, spec.product), patchId(repoRoot, headBase, head, spec.product)];
|
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}`;
|
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 };
|
return { rangeBase, touched, spec, productFrom: ids[0] as string, productHead: ids[1] as string };
|
||||||
|
|||||||
+4
-1
@@ -1536,7 +1536,10 @@ 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
|
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,
|
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
|
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
|
merge dropped its change. The comparison then runs from that commit, leaving out
|
||||||
|
every path whose content at the reviewed head is the base's (a base merge the
|
||||||
|
child made before its receipt brought it in) unless HEAD changed it against the
|
||||||
|
base (GST-1214): an equal
|
||||||
patch-id carries with reason `stack parent <sha12> landed`, base context next
|
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
|
to the child's unchanged hunks is `RECEIPT_CARRY_SCOPED_REVIEW`, and any other
|
||||||
difference refuses with `RECEIPT_CARRY_PRODUCT_CHANGED`.
|
difference refuses with `RECEIPT_CARRY_PRODUCT_CHANGED`.
|
||||||
|
|||||||
+1
-1
@@ -1,6 +1,6 @@
|
|||||||
{
|
{
|
||||||
"name": "gstack",
|
"name": "gstack",
|
||||||
"version": "1.94.20",
|
"version": "1.94.21",
|
||||||
"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",
|
||||||
|
|||||||
@@ -240,3 +240,68 @@ describe('carry after a stack parent squash-merges, receipt written against main
|
|||||||
expect(landedStackParent(repo.work, { rangeBase: fork, reviewed, currentBase: mainTip }, ['.', ':(exclude,glob)test/**'])).toBeNull();
|
expect(landedStackParent(repo.work, { rangeBase: fork, reviewed, currentBase: mainTip }, ['.', ':(exclude,glob)test/**'])).toBeNull();
|
||||||
}, 60_000);
|
}, 60_000);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('carry after a stack parent lands, the child having merged main before its receipt (GST-1214)', () => {
|
||||||
|
/** main <- parent (lib/util.js) <- feature (lib/retry.js), which merges main (wiki/) once before its receipt is written against main. */
|
||||||
|
function mergedStack(repo: FixtureRepo, mainFiles: Record<string, string | null> = { 'wiki/a.md': '# a\n', 'wiki/b.md': '# b\n' }): { parent: string; reviewed: string; body: string } {
|
||||||
|
repo.git('checkout', '-q', '-b', 'parent');
|
||||||
|
const parent = commit(repo, { 'lib/util.js': SUB }, 'parent: sub');
|
||||||
|
repo.git('push', '-q', 'origin', 'parent');
|
||||||
|
repo.git('checkout', '-q', 'feature');
|
||||||
|
repo.git('reset', '-q', '--hard', parent);
|
||||||
|
commit(repo, { 'lib/retry.js': RETRY4 }, 'child: four retries');
|
||||||
|
repo.git('checkout', '-q', 'main');
|
||||||
|
commit(repo, mainFiles, 'main: wiki');
|
||||||
|
repo.git('push', '-q', 'origin', 'main');
|
||||||
|
repo.git('checkout', '-q', 'feature');
|
||||||
|
repo.git('merge', '-q', '--no-edit', 'origin/main');
|
||||||
|
const reviewed = repo.git('rev-parse', 'HEAD');
|
||||||
|
const body = path.join(repo.root, 'body.md');
|
||||||
|
const w = runBin(repo, 'gstack-ship-receipt', ['write', '--pr', '8', '--base', 'main', '--version', '1.0.0', '--session-kind', 'unattended', '--body', body]);
|
||||||
|
expect(w.status).toBe(EXIT.ok);
|
||||||
|
return { parent, reviewed, body };
|
||||||
|
}
|
||||||
|
|
||||||
|
// Value: protects=a stacked child that merged main before its receipt keeps it once the parent squash-lands and main is
|
||||||
|
// merged again (gstack-evals#178 refused on 20 wiki/ paths its first base merge brought in); fails_when=the range from the
|
||||||
|
// landed parent counts paths that hold main's content at the reviewed head; seam=none; no_claim=gh pr edit (--body stands in)
|
||||||
|
test('the child carries when only its earlier base merge separates the parent range from HEAD\'s diff against main', () => {
|
||||||
|
const repo = makeFixtureRepo(); repos.push(repo);
|
||||||
|
const { parent, reviewed, body } = mergedStack(repo);
|
||||||
|
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 ${reviewed.slice(0, 12)} to ${head.slice(0, 12)}`);
|
||||||
|
expect(receiptIn(body).carry!.reason).toBe(`stack parent ${parent.slice(0, 12)} landed + base merge`);
|
||||||
|
}, 90_000);
|
||||||
|
|
||||||
|
// Value: protects=fail closed: an edit to a file the earlier base merge brought in is product and refuses; fails_when=every
|
||||||
|
// base-merged path drops out of both sides, the child's own edit included; seam=none; no_claim=-
|
||||||
|
test('an edit to a base-merged file after the receipt refuses with RECEIPT_CARRY_PRODUCT_CHANGED', () => {
|
||||||
|
const repo = makeFixtureRepo(); repos.push(repo);
|
||||||
|
const { reviewed, body } = mergedStack(repo);
|
||||||
|
landOnMain(repo, { 'lib/util.js': SUB });
|
||||||
|
commit(repo, { 'wiki/a.md': '# a, edited by the child\n' }, 'child: edit wiki');
|
||||||
|
const out = carry(repo, body);
|
||||||
|
expect(out.status).toBe(EXIT.refused);
|
||||||
|
expect(out.out).toContain('CARRY: refused (RECEIPT_CARRY_PRODUCT_CHANGED)');
|
||||||
|
expect(out.err).toContain('wiki/a.md');
|
||||||
|
expect(receiptIn(body).head).toBe(reviewed);
|
||||||
|
}, 90_000);
|
||||||
|
|
||||||
|
// Value: protects=fail closed when the earlier base merge turned a file into a directory: leaving out the old file must not
|
||||||
|
// hide an edit beneath it (a pathspec names a path and everything under it); fails_when=the exclusion covers a changed
|
||||||
|
// descendant; seam=none; no_claim=-
|
||||||
|
test('a base merge that replaced a file with a directory never hides an edit beneath it', () => {
|
||||||
|
const repo = makeFixtureRepo({ baseFiles: { services: 'old\n' } }); repos.push(repo);
|
||||||
|
const { reviewed, body } = mergedStack(repo, { services: null, 'services/worker.js': 'exports.allowed = false;\n' });
|
||||||
|
landOnMain(repo, { 'lib/util.js': SUB });
|
||||||
|
commit(repo, { 'services/worker.js': 'exports.allowed = true;\n' }, 'child: allow');
|
||||||
|
const out = carry(repo, body);
|
||||||
|
expect(out.status).toBe(EXIT.refused);
|
||||||
|
expect(out.out).toContain('CARRY: refused (RECEIPT_CARRY_PRODUCT_CHANGED)');
|
||||||
|
expect(receiptIn(body).head).toBe(reviewed);
|
||||||
|
}, 90_000);
|
||||||
|
});
|
||||||
Reference in new issue
Block a user