fix(apply-review-hold): carry no cost decision past a deleted comment

.github/actions/apply-review-hold/hold.cjs

@@ -78,6 +78,11 @@ const PULL_QUERY = `query($owner: String!, $name: String!, $number: Int!) {
 }`;
 const EDITED_QUERY =
   "query($id: ID!) { node(id: $id) { ... on IssueComment { lastEditedAt } } }";
+const DELETED_QUERY = `query($owner: String!, $name: String!, $number: Int!) {
+  repository(owner: $owner, name: $name) {
+    pullRequest(number: $number) { timelineItems(itemTypes: [COMMENT_DELETED_EVENT], last: 100) { filteredCount nodes { ... on CommentDeletedEvent { createdAt } } } }
+  }
+}`;
 
 // Whether comment `c` was ever edited: "unedited" when GraphQL answers
 // lastEditedAt null, "edited" when it answers a time, else "unknown" (a
@@ -93,6 +98,34 @@ async function editState(github, c) {
   }
 }
 
+// Whether a comment on pull request `number` was deleted at or after `at`
+// (merge-holds.mjs deletedSince at claude-code-plugins#9569's head
+// 149f3214): "deleted", "none", or "unknown" when the read fails, lists
+// fewer deletions than its filteredCount, has no list or integer count, or
+// holds a time that does not parse. A deleted newer marker would otherwise
+// make an older one the decided head.
+async function deletedSince(github, owner, repo, number, at) {
+  let items;
+  try {
+    items = (await github.graphql(DELETED_QUERY, { owner, name: repo, number }))
+      ?.repository?.pullRequest?.timelineItems;
+  } catch {
+    return "unknown";
+  }
+  const when = Date.parse(at);
+  const nodes = items?.nodes;
+  if (
+    !Number.isFinite(when) ||
+    !Array.isArray(nodes) ||
+    !Number.isInteger(items.filteredCount) ||
+    items.filteredCount > nodes.length
+  )
+    return "unknown";
+  const times = nodes.map((node) => Date.parse(node?.createdAt));
+  if (!times.every(Number.isFinite)) return "unknown";
+  return times.some((t) => t >= when) ? "deleted" : "none";
+}
+
 // The issue one plain `Refs: #<n>` line names, or null: claude-code-plugins@eda837d4c8
 // plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py:1851-1895
 // refs_issue. A line counts from its first column, outside an HTML comment
@@ -225,14 +258,17 @@ const BRANCH = /^(?!-)(?!.*\.\.)[A-Za-z0-9._/-]{1,255}$/u;
 // the sha of this lane's newest cost hold marker posted before the decision,
 // and only while that marker is unedited: an edited marker, or one whose
 // edit state cannot be read, carries nothing, and no older marker is used.
-// The decision carries when the head is that sha, or when the pull request's
+// It carries nothing either when any comment on the pull request was
+// deleted at or after that marker was posted, or the deletion list cannot
+// show none was (deletedSince), read before the head check so deleting a
+// newer marker cannot promote an older head. The decision carries when the head is that sha, or when the pull request's
 // net diff against its base branch is the same at both: compare
 // base...decided and base...head (three-dot, so each is the diff from its
 // merge base) list the same files, each with the same status,
 // previous_filename and patch text. So a base-only merge or a rebase that
 // leaves every patch as it was carries, and any other change does not. One
-// edit read and at most two compare reads; any failure, a missing patch, or
-// a list of 300 or more files carries nothing.
+// edit read, one deletion read and at most two compare reads; any failure,
+// a missing patch, or a list of 300 or more files carries nothing.
 async function carryDecision(github, owner, repo, env, prComments, decision) {
   const cutoff = Date.parse(decision.created_at);
   if (!Number.isFinite(cutoff)) return { carry: false, why: "the decision has no time" };
@@ -260,6 +296,14 @@ async function carryDecision(github, owner, repo, env, prComments, decision) {
     return { carry: false, why: "the decided-head marker was edited" };
   if (state !== "unedited")
     return { carry: false, why: "could not confirm the marker is unedited" };
+  const deleted = await deletedSince(github, owner, repo, Number(env.PR_NUMBER), held.created_at);
+  if (deleted === "deleted")
+    return { carry: false, why: "a comment was deleted since the decided-head marker was posted" };
+  if (deleted !== "none")
+    return {
+      carry: false,
+      why: "could not confirm no comment was deleted since the decided-head marker was posted",
+    };
   if (decided === env.HEAD_SHA)
     return { carry: true, why: `the head is the decided head ${decided}` };
   const base = String(env.BASE_REF ?? "");

.github/scripts/apply-review-hold.test.cjs

@@ -47,7 +47,9 @@ const TRUST_PATHS = [
 // edit read fails; `pullNumbers` the numbers
 // that are pull requests, not issues; `misnumbered` the numbers whose read
 // answers another number; `compares` maps a basehead to its compare
-// response data.
+// response data; `deletions` the created_at of each COMMENT_DELETED_EVENT
+// on the pull request, `deletedItems` a raw timelineItems answer in their
+// place, and `failDeleted` fails that read.
 function fakeGithub({
   files = [],
   changedFiles,
@@ -64,6 +66,9 @@ function fakeGithub({
   pullNumbers = new Set([7]),
   misnumbered = new Set(),
   compares = {},
+  deletions = [],
+  deletedItems,
+  failDeleted = false,
 }) {
   const calls = [];
   const refuse = (name) => () => {
@@ -75,6 +80,22 @@ function fakeGithub({
       calls.push(["graphql", variables]);
       if (/lastEditedAt/u.test(query) && failEdited.has(variables.id))
         throw new Error("HTTP 502");
+      if (/COMMENT_DELETED_EVENT/u.test(query)) {
+        if (failDeleted) throw new Error("HTTP 502");
+        return {
+          repository: {
+            pullRequest: {
+              timelineItems:
+                deletedItems !== undefined
+                  ? deletedItems
+                  : {
+                      filteredCount: deletions.length,
+                      nodes: deletions.map((createdAt) => ({ createdAt })),
+                    },
+            },
+          },
+        };
+      }
       if (/lastEditedAt/u.test(query))
         return {
           node: {
@@ -476,14 +497,17 @@ test("a cost decision on the pull request keeps the cost hold from coming back",
   assert.equal(core.outputs.hold, "false");
   assert.deepEqual(writes, []);
   // The lookup's reads, the pull request's comments and the decision's edit
-  // check, then the marker's edit check, then hold-paths' own file list: the
-  // head is the decided head, so no compare.
+  // check, then the marker's edit check and the deletion read, then
+  // hold-paths' own file list: the head is the decided head, so no compare.
   assert.deepEqual(
     github.calls.map(([name]) => name),
-    ["listComments", "graphql", "graphql", "listFiles", "get"],
+    ["listComments", "graphql", "graphql", "graphql", "listFiles", "get"],
   );
+  assert.deepEqual(github.calls[3][1], { owner: "o", name: "r", number: 7 });
   assert.deepEqual(
-    github.calls.filter(([name]) => name === "graphql").map(([, v]) => v.id),
+    github.calls
+      .filter(([name, v]) => name === "graphql" && "id" in v)
+      .map(([, v]) => v.id),
     ["IC_1", "IC_100"],
   );
 });
@@ -509,6 +533,7 @@ test("a cost decision on the Refs issue that names the pull request keeps the co
       "listComments",
       "graphql",
       "graphql",
+      "graphql",
       "listFiles",
       "get",
     ],
@@ -895,15 +920,16 @@ test("an unusable base branch name reads no compare and keeps the cost hold", as
   assert.equal(compares(github).length, 0);
 });
 
-test("the carry check reads at most two compares and nothing else", async () => {
+test("the carry check reads one deletion list, at most two compares and nothing else", async () => {
   const { github } = await carryCase(atDecided);
   const names = github.calls.map(([name]) => name);
   // The pull request's comments, the decision's and the marker's edit
-  // checks, two compares, then hold-paths' own file list.
+  // checks, the deletion read, two compares, then hold-paths' own file list.
   assert.deepEqual(names, [
     "listComments",
     "graphql",
     "graphql",
+    "graphql",
     "compare",
     "compare",
     "listFiles",
@@ -964,3 +990,98 @@ test("a failed marker edit check keeps the cost hold", async () => {
     /no recorded cost decision carries \(could not confirm the marker is unedited\)/u,
   );
 });
+
+// A deleted comment can remove the newest marker, which would make an older
+// one the decided head (as merge-holds.mjs deletedSince reads it): any
+// comment deleted at or after the decided-head marker was posted carries
+// nothing. The pull request timeline's COMMENT_DELETED_EVENT
+// items give the deletion times.
+test("a comment deleted after the decided-head marker keeps the cost hold, with no older marker standing in", async () => {
+  // The newer marker, at 11:00, was deleted; the one left, at 10:30, holds
+  // the head's sha and would carry with no compare.
+  const { core, writes, github } = await carryCase(atDecided, {
+    comments: [
+      marker({ id: 101, sha: LATER, createdAt: "2026-10-23T10:30:00Z" }),
+      decision(),
+    ],
+    deletions: ["2026-10-23T11:05:00Z"],
+  });
+  assert.equal(core.outputs.hold, "true");
+  assert.equal(core.outputs.criteria, "cost");
+  // The label comes back; the remaining marker already names this head.
+  assert.deepEqual(
+    writes.map(([name]) => name),
+    ["addLabels"],
+  );
+  assert.equal(compares(github).length, 0);
+  assert.match(
+    core.infos.join("\n"),
+    /no recorded cost decision carries \(a comment was deleted since the decided-head marker was posted\)/u,
+  );
+});
+
+test("a deletion at the marker's own time keeps the cost hold, and one before it does not", async () => {
+  const at = await apply(costEnv(), {
+    comments: [marker(), decision()],
+    deletions: ["2026-10-23T10:00:00Z", "2026-10-23T11:00:00Z"],
+  });
+  assert.equal(at.core.outputs.hold, "true");
+  const before = await apply(costEnv(), {
+    comments: [marker(), decision()],
+    deletions: ["2026-10-23T10:59:59Z"],
+  });
+  assert.equal(before.core.outputs.hold, "false");
+  assert.match(
+    before.core.infos.join("\n"),
+    /the recorded cost decision carries \(the head is the decided head/u,
+  );
+});
+
+test("a deletion list that cannot show no comment was deleted keeps the cost hold", async () => {
+  const listed = (filteredCount, nodes) => ({ filteredCount, nodes });
+  for (const [name, options] of [
+    ["a failed read", { failDeleted: true }],
+    ["no timeline", { deletedItems: null }],
+    ["no node list", { deletedItems: { filteredCount: 0 } }],
+    ["no count", { deletedItems: { nodes: [] } }],
+    ["a count that is not a number", { deletedItems: listed("0", []) }],
+    [
+      "more deletions than listed",
+      {
+        deletedItems: listed(
+          101,
+          Array.from({ length: 100 }, () => ({
+            createdAt: "2026-10-23T09:00:00Z",
+          })),
+        ),
+      },
+    ],
+    [
+      "a time that does not parse",
+      { deletedItems: listed(1, [{ createdAt: "yesterday" }]) },
+    ],
+    ["an event with no time", { deletedItems: listed(1, [{}]) }],
+  ]) {
+    const { core, error, github } = await apply(costEnv(), {
+      comments: [marker(), decision()],
+      ...options,
+    });
+    assert.equal(error, undefined, name);
+    assert.equal(core.outputs.hold, "true", name);
+    assert.equal(compares(github).length, 0, name);
+    assert.match(
+      core.infos.join("\n"),
+      /no recorded cost decision carries \(could not confirm no comment was deleted since the decided-head marker was posted\)/u,
+      name,
+    );
+  }
+});
+
+test("an edited marker reads no deletion list", async () => {
+  const { github } = await apply(costEnv(), {
+    comments: [marker(), decision()],
+    edited: new Set(["IC_100"]),
+    failDeleted: true,
+  });
+  assert.equal(github.calls.filter(([name]) => name === "graphql").length, 2);
+});

.github/workflows/pr-review-cli.yml

@@ -112,7 +112,10 @@ name: pr-review-cli
 #     before the decision; with none, nothing carries. That marker must be
 #     unedited (GraphQL lastEditedAt null): an edited marker, or one whose
 #     edit state cannot be read, carries nothing, and no older marker is
-#     used. It covers the head when the head is that sha, or when the pull
+#     used. Nor does it carry when the pull request timeline shows a
+#     comment deleted at or after that marker was posted
+#     (COMMENT_DELETED_EVENT), or cannot show none was: a failed read, more
+#     deletions than it lists, or a time that does not parse. It covers the head when the head is that sha, or when the pull
 #     request's net diff is the same at both: the three-dot compares
 #     base...decided and base...head list the same files with the same
 #     status, previous_filename and patch text. A base-only merge or a rebase that leaves every patch alone