Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changes/pr-pipeline/fix-exact-base-merge-carry-9c3a5e16.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
bump: patch
---

### Fixed

- `ci-status` and `enable-auto-merge` carry a trusted person's hold-path approval over a base merge in two more cases (`merge-holds.mjs` `liftersOf`). An approval GitHub dismissed as stale on the merge's push counts, bound to its own commit and only through the carry (`autoDismissed`: a `review_dismissed` timeline event with no message, a Bot actor and a dismissal commit the merges pushed); a dismissal with a message, by a person, or by a person's push revokes it. When the base changed a file the pull request also changes, the approval carries when the merges brought exactly the base's changes (`exactBaseCarry`, `approvalCarry`): the compare from the approved head to the current head equals, file for file, the compare between the two merge bases. A merge that writes anything of its own carries nothing, and a list of 300 files or more fails closed. The cost-decision carry is unchanged. The carry runs only into the repository's default branch, and the exact path also reads the recursive trees of the current head and of its merge base with the base's tip, so a merge that flips the mode of a file the base changed carries nothing; a truncated tree fails closed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
bump: patch
---

### Fixed

- A trusted person's approval of an earlier head now carries a hold-path lift over a base merge in the two cases that sent every such pull request back to them. An approval GitHub dismissed as stale on the merge's push still counts, bound to its own commit and only through the carry: the issue timeline's `review_dismissed` event has no message, a Bot actor and a dismissal commit among those pushed since the approved head (`auto_dismissed`). A dismissal with a message, by a person, or by a person's push still revokes it. When the base changed a file the pull request also changes, so the net change differs, the approval carries when the merges brought exactly the base's changes: the compare from the approved head to the current head equals, file for file with the same status and patch, the compare between the two merge bases with the base's tip (`exact_base_carry`, `approval_carry`, `carried_lifters`). A merge that writes any change of its own or drops a base change, a list of 300 files or more, or a commit list not read whole carries nothing. The owner's cost-decision carry is unchanged. The carry runs only into the repository's default branch, and the exact path also reads the recursive trees of the current head and of its merge base with the base's tip, so a merge that flips the mode of a file the base changed carries nothing; a truncated tree fails closed.
21 changes: 20 additions & 1 deletion .github/scripts/pr-merge/enable-auto-merge.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -1391,7 +1391,7 @@ test("a hold path stays held without a trusted approval of the head", async () =
// base. The approval carries to the merge, so it arms and is not labeled
// again; a merge that also edits the hold path is sent back to a person, and
// a label removal alone lifts nothing (the owner's decision on #10596).
test("an approval before a base merge lifts the hold path at the merge, and an evil merge or a label removal does not", async () => {
test("an approval before a base merge, or its stale dismissal, lifts the hold path at the merge, and an evil merge, a dismissal with a message or a label removal does not", async () => {
const OLD = "a".repeat(40);
const MAIN = "5".repeat(40);
const P = "/repos/melodic-software/claude-code-plugins";
Expand All @@ -1401,6 +1401,7 @@ test("an approval before a base merge lifts the hold path at the merge, and an e
const TIP = "3".repeat(40);
const rest = (number, headFiles, reviews = [approval]) => ({
[`${P}/pulls/${number}`]: { head: { sha: HEAD, ref: `fix/held-${number}`, repo: { full_name: "melodic-software/claude-code-plugins" } }, base: { ref: "main" }, changed_files: headFiles.length },
[P]: { full_name: "melodic-software/claude-code-plugins", default_branch: "main" },
[`${P}/git/ref/heads/main`]: { ref: "refs/heads/main", object: { type: "commit", sha: TIP } },
[`${P}/pulls/${number}/reviews?per_page=100&page=1`]: reviews,
[`${P}/compare/${TIP}...${OLD}`]: { files: [held], commits: [{ sha: OLD, parents: [{ sha: MAIN }] }], total_commits: 1 },
Expand All @@ -1427,6 +1428,24 @@ test("an approval before a base merge lifts the hold path at the merge, and an e
};
const removed = await run([pull(62, { headRefName: "fix/held-62", files: HELD, rest: removal })], trustedKyle());
assert.deepEqual(removed.writes, []);
// The owner's decision of 2026-10-28: the approval GitHub dismissed as
// stale on the merge's push (a Bot's dismissal with no message, by the
// pushed head) still carries; one dismissed with a message does not.
const dismissedEvent = (message) => ({
event: "review_dismissed",
actor: { login: "lanes[bot]", type: "Bot" },
dismissed_review: { review_id: 41, state: "approved", dismissal_message: message, dismissal_commit_id: HEAD },
});
const staleRest = (number, message) => ({
...rest(number, [held], [{ ...approval, state: "DISMISSED" }]),
[`${P}/issues/${number}/timeline?per_page=100&page=1`]: [dismissedEvent(message)],
});
const stale = await run([pull(63, { files: HELD, rest: staleRest(63, null) })], trustedKyle());
assert.deepEqual(stale.writes, [{ mutation: "arm", id: "PR_63", head: HEAD }]);
assert.deepEqual(stale.notes, []);
const revoked = await run([pull(64, { files: HELD, rest: staleRest(64, "stale") })], trustedKyle());
assert.deepEqual(revoked.writes, []);
assert.deepEqual(revoked.notes.map(({ path }) => path), [`${ISSUES}/64/comments`, `${ISSUES}/64/labels`]);
});

test("an approval lifts no unread hold-path check and no capped file listing", () => {
Expand Down
Loading
Loading