fix(pr-merge): carry a hold-path lift over a later base merge

.changes/pr-pipeline/fix-hold-lift-survives-base-merge-5c81e0d4.md

@@ -0,0 +1,7 @@
+---
+bump: patch
+---
+
+### Changed
+
+- A trusted person's approving review of a head that changes a hold path or widens the trusted-actor list carries over a later merge of the base when the merge changes nothing of its own, so the merge no longer sends the pull request back to them. Only an approving review lifts these holds: removing the human-gated label does not, and a dismissed approval never counts. The approval carries only when every commit since is a merge of commits the base branch holds and the whole net change against the base's tip commit is the same at both heads; the base is read once by its full ref. The owner's cost decision carries by the same rule. A carried approval does not lift the verifier, issue-link or pr-verify FAIL holds.

.changes/source-control/fix-hold-lift-survives-base-merge-7d3a91c2.md

@@ -0,0 +1,8 @@
+---
+bump: patch
+---
+
+### Fixed
+
+- A trusted person's approving review of a head that changes a hold path now survives a later merge of the base when the merge changes nothing of its own. Each trusted person's newest review that approves, requests changes or was dismissed counts, and lifts only when it approves; a dismissed approval never counts, and removing the human-gated label lifts no hold path. The approval carries to the current head only when every commit since is a merge whose other parents the base branch holds and the net change against the base's tip commit is the same at both heads over every file (`base_merge_carry`, `base_parents`, `base_tip`, `carried_lifters`, and `merge-holds.mjs` `baseMergeCarry`, `baseParents`, `baseTip` and `liftersOf`). The base branch is read once by its full ref, so no tag of the same name stands in for it. A non-merge commit, a merge of another branch or of a commit the base does not hold, a force push that rewrites the approved head, a merge that changes any file differently, an untrusted approver, a head in a fork, and any unreadable input carry nothing. An honest merge after the base changed one of the pull request's files also carries nothing and needs a new approval. A carried approval lifts only the hold path; it never lifts the verifier hold.
+- The owner's cost decision carries to a later head only when every commit since is a merge whose other parents the base branch holds (`base_merges_only`), as well as the same net change against the base's tip commit. A rebase, or a commit of its own that leaves the net change the same, no longer carries it.

.github/scripts/pr-merge/enable-auto-merge.mjs

@@ -24,9 +24,10 @@
 //   wrote its Hold: line can add one. The rule is merge-holds.pathHold, which
 //   ci-status also runs on every lanes App pull request event and merge
 //   group, so a push after arming is held without waiting for this job. A
-//   trusted person's approving review of the head lifts it, as ci-status
-//   lifts it (merge-holds.pathLifters); it lifts no listing or check that
-//   could not run. A config that does not read arms nothing and labels
+//   trusted person's approving review of the head, or of an earlier head
+//   carried over base merges, lifts it, as ci-status lifts it
+//   (merge-holds.pathLifters and liftersOf); it lifts no listing or check
+//   that could not run. A config that does not read arms nothing and labels
 //   nothing; the next run reads it again.
 // - Its head changes no cost path, unless its body links the owner's cost
 //   decision: merge-holds.costHold and costDecision, which ci-status runs too.
@@ -174,7 +175,7 @@ import { createGitHub, errorLabel, GitHubError } from "../../actions/check-trust
 import { rateLimitLatch } from "../../actions/log-api-budget/rate-guard.mjs";
 import { humanGatedLabelFrom } from "../pr-unblock/address-feedback.mjs";
 import { readHoldPaths } from "../work-resolve-issue/report-outcome.mjs";
-import { BODY_HOLD, costDecision, costHold, humansOf, LANE_HOLD, pathHoldKind, pathLifters, personWrote, vetoers } from "./merge-holds.mjs";
+import { BODY_HOLD, costDecision, costHold, humansOf, LANE_HOLD, liftersOf, pathHoldKind, pathLifters, personWrote, vetoers } from "./merge-holds.mjs";
 
 export { BODY_HOLD, HUMAN_VETO } from "./merge-holds.mjs";
 
@@ -1018,8 +1019,9 @@ export function decide(pull, { bot, humanGated, holdPaths = null }, detail) {
   if (detail.filesCapped === true) {
     return { action: armed ? "disarm" : "none", reason: FILES_CAPPED_REASON, clears: "person" };
   }
-  // A hold path a person clears, lifted by a trusted approval of the head
-  // (`lifters`) as ci-status lifts it; a listing or check that could not run
+  // A hold path a person clears, lifted by a trusted person's approval of the
+  // head or of an earlier head carried over base merges (`lifters`) as
+  // ci-status lifts it; a listing or check that could not run
   // is a lane's, which the next run reads again.
   const pathHeld = pathHoldKind(detail.files, holdPaths);
   if (pathHeld !== null && pathHeld.kind !== "hold-path") {
@@ -1302,7 +1304,15 @@ export async function main({ env = process.env, github, actions, verify, room, s
             const { paths: files, capped } = await changedFiles(github, `${repo[1]}/${repo[2]}`, pull.number);
             const humans = trustedHumans(env);
             const decision = costHold(files, null) ? await costDecision(github, repo[0], pull.number, pull.body, humans, pull.headRefOid, actions) : null;
-            detail = { ...detail, files, filesCapped: capped, costDecision: decision, lifters: headLifters(detail, humans, pull.headRefOid) };
+            let lifters = headLifters(detail, humans, pull.headRefOid);
+            // An approval of an earlier head carried over base merges
+            // (merge-holds.liftersOf) lifts the hold path as ci-status lifts
+            // it, so a base merge after a person's approval does not send the
+            // pull request back to them.
+            if (!lifters.length && Array.isArray(holdPaths) && pathHoldKind(files, holdPaths)?.kind === "hold-path") {
+              lifters = (await liftersOf(github, repo[0], pull.number, pull.headRefOid, humans)).lifters;
+            }
+            detail = { ...detail, files, filesCapped: capped, costDecision: decision, lifters };
           }
         } catch (error) {
           // A read that fails decides nothing and writes nothing.

.github/scripts/pr-merge/enable-auto-merge.test.mjs

@@ -128,6 +128,12 @@ function world(pulls, { failArm = false, armErrors = {}, filesError = null, note
   const notes = [];
   const github = async (method, path, body, headers) => {
     calls.push({ method, path, body, headers });
+    // A pull request's own REST answers (`rest`, by path), such as the
+    // reads of a lift carried over base merges (merge-holds.liftersOf).
+    const own = method === "GET" ? pulls.find((candidate) => candidate.rest && path in candidate.rest) : undefined;
+    if (own) {
+      return own.rest[path];
+    }
     const listing = /^\/repos\/melodic-software\/claude-code-plugins\/pulls\/([0-9]+)\/files\?per_page=100&page=([0-9]+)$/.exec(path);
     if (method === "GET" && listing && filesError) {
       throw filesError;
@@ -152,7 +158,11 @@ function world(pulls, { failArm = false, armErrors = {}, filesError = null, note
     if (method === "GET" && path === "/repos/melodic-software/claude-code-plugins/activity?ref=refs%2Fheads%2Ffix%2Fcost&direction=desc&per_page=100") {
       return [{ after: HEAD, timestamp: "2026-10-22T10:45:00Z", activity_type: "push", ref: "refs/heads/fix/cost" }];
     }
-    if (method === "GET" && path === `/repos/melodic-software/claude-code-plugins/compare/main...${HEAD}`) {
+    // main resolves to its tip commit, which the lane-change vet compares.
+    if (method === "GET" && path === "/repos/melodic-software/claude-code-plugins/git/ref/heads/main") {
+      return { ref: "refs/heads/main", object: { type: "commit", sha: "3".repeat(40) } };
+    }
+    if (method === "GET" && path === `/repos/melodic-software/claude-code-plugins/compare/${"3".repeat(40)}...${HEAD}`) {
       return { files: [{ filename: "infra/main.tf" }] };
     }
     if (method === "GET" && path === "/repos/melodic-software/claude-code-plugins/actions/runs/9/jobs?filter=all&per_page=100&page=1") {
@@ -1367,6 +1377,48 @@ test("a hold path stays held without a trusted approval of the head", async () =
   assert.deepEqual(writes, [{ mutation: "disarm", id: "PR_55" }]);
 });
 
+// The churn measured on #10224: Kyle approved OLD, then a lane merged the
+// 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 () => {
+  const OLD = "a".repeat(40);
+  const MAIN = "5".repeat(40);
+  const P = "/repos/melodic-software/claude-code-plugins";
+  const held = { filename: "docs/held.md", status: "modified", patch: "@@ -1 +1 @@\n-a\n+b" };
+  const approval = { id: 41, user: { login: "kyle-sexton", id: 5, type: "User" }, state: "APPROVED", commit_id: OLD, submitted_at: "2026-10-27T23:26:14Z" };
+  // main resolves to TIP, its tip commit, which every compare names.
+  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}/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 },
+    [`${P}/compare/${MAIN}...${TIP}`]: { status: "ahead" },
+    [`${P}/compare/${TIP}...${HEAD}`]: { files: headFiles, commits: [{ sha: OLD, parents: [{ sha: MAIN }] }, { sha: HEAD, parents: [{ sha: OLD }, { sha: MAIN }] }], total_commits: 2 },
+  });
+  const lifted = await run([pull(60, { files: HELD, rest: rest(60, [held]) })], trustedKyle());
+  assert.deepEqual(lifted.writes, [{ mutation: "arm", id: "PR_60", head: HEAD }]);
+  assert.deepEqual(lifted.notes, []);
+  const evil = await run([pull(61, { files: HELD, rest: rest(61, [{ ...held, patch: "@@ -1 +1 @@\n-a\n+evil" }]) })], trustedKyle());
+  assert.deepEqual(evil.writes, []);
+  assert.deepEqual(evil.notes.map(({ path }) => path), [`${ISSUES}/61/comments`, `${ISSUES}/61/labels`]);
+  // The label history the earlier reader bound a removal by.
+  const removal = {
+    ...rest(62, [held], []),
+    [`${P}/issues/62/events?per_page=100&page=1`]: [
+      { event: "labeled", label: { name: "needs-human" }, actor: { id: 2, login: "lanes[bot]" }, performed_via_github_app: { slug: "lanes" }, created_at: "2026-10-27T23:05:00Z" },
+      { event: "unlabeled", label: { name: "needs-human" }, actor: { id: 5, login: "kyle-sexton" }, performed_via_github_app: null, created_at: "2026-10-27T23:26:14Z" },
+    ],
+    [`${P}/activity?ref=refs%2Fheads%2Ffix%2Fheld-62&direction=desc&per_page=100`]: [
+      { after: HEAD, timestamp: "2026-10-27T23:32:00Z" },
+      { after: OLD, timestamp: "2026-10-27T23:00:00Z" },
+    ],
+  };
+  const removed = await run([pull(62, { headRefName: "fix/held-62", files: HELD, rest: removal })], trustedKyle());
+  assert.deepEqual(removed.writes, []);
+});
+
 test("an approval lifts no unread hold-path check and no capped file listing", () => {
   const options = { bot: LANES.login, humanGated: "needs-human", holdPaths: [] };
   const armed = pull(1, { autoMergeRequest: {} });