fix(ci): retire superseded Ladon change requests

Author: bokelley

.github/workflows/ai-review.yml

@@ -16,6 +16,7 @@ on:
     paths-ignore:
       - ".github/workflows/ai-review.yml"
       - "LADON.md"
+      - "scripts/retire-superseded-ladon-reviews.cjs"
 
 jobs:
   code_review:
@@ -60,7 +61,7 @@ jobs:
           MODIFIED=""
           while IFS= read -r f; do
             case "$f" in
-              .github/workflows/ai-review.yml|LADON.md)
+              .github/workflows/ai-review.yml|LADON.md|scripts/retire-superseded-ladon-reviews.cjs)
                 MODIFIED="${MODIFIED}${f}"$'\n' ;;
             esac
           done <<< "$CHANGED"
@@ -104,3 +105,28 @@ jobs:
           skip-bot-authors: dependabot[bot],renovate[bot],github-actions[bot],aao-ipr-bot[bot]
           # Optional; defaults to the review action's pinned model.
           model: claude-opus-4-8
+
+      # GitHub keeps an earlier CHANGES_REQUESTED review blocking even after
+      # the same App approves a corrected head. Retire only Ladon's own older
+      # change requests, and only when its latest review approves this head.
+      - name: Retire superseded Ladon change requests
+        if: steps.workflow-mod.outputs.modified != 'true'
+        uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8
+        env:
+          LADON_BOT_LOGIN: ${{ steps.app-token.outputs.app-slug }}[bot]
+          PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }}
+        with:
+          github-token: ${{ steps.app-token.outputs.token }}
+          script: |
+            const { retireSupersededLadonReviews } = require(
+              `${process.env.GITHUB_WORKSPACE}/scripts/retire-superseded-ladon-reviews.cjs`
+            );
+            const dismissed = await retireSupersededLadonReviews({
+              github,
+              owner: context.repo.owner,
+              repo: context.repo.repo,
+              pullNumber: context.issue.number,
+              botLogin: process.env.LADON_BOT_LOGIN,
+              headSha: process.env.PR_HEAD_SHA,
+            });
+            core.info(`Dismissed ${dismissed.length} superseded Ladon review(s).`);

.github/workflows/ci.yml

@@ -37,6 +37,8 @@ jobs:
           # catches up.
           flags: >-
             -ignore ^unexpected\s+key\s+\x22queue\x22\s+for\s+\x22concurrency\x22\s+section
+      - name: Test Ladon review-state helper
+        run: node --test scripts/retire-superseded-ladon-reviews.test.cjs
       - name: Audit workflows
         uses: zizmorcore/zizmor-action@3dc1ecc9bcb9e94e9b2c709687979e1298497054 # v0.6.2
         with:

scripts/retire-superseded-ladon-reviews.cjs

@@ -0,0 +1,67 @@
+#!/usr/bin/env node
+
+const APPROVED = 'APPROVED';
+const CHANGES_REQUESTED = 'CHANGES_REQUESTED';
+
+function normalizeLogin(login) {
+  return String(login || '').toLowerCase();
+}
+
+function latestBotReview(reviews, botLogin) {
+  const normalizedBotLogin = normalizeLogin(botLogin);
+  return reviews
+    .filter((review) => normalizeLogin(review.user?.login) === normalizedBotLogin)
+    .sort((left, right) => Number(left.id) - Number(right.id))
+    .at(-1);
+}
+
+function supersededChangeRequests(reviews, botLogin, headSha) {
+  const latest = latestBotReview(reviews, botLogin);
+  if (!latest || latest.state !== APPROVED || latest.commit_id !== headSha) {
+    return [];
+  }
+
+  const normalizedBotLogin = normalizeLogin(botLogin);
+  return reviews.filter(
+    (review) =>
+      normalizeLogin(review.user?.login) === normalizedBotLogin &&
+      review.state === CHANGES_REQUESTED &&
+      Number(review.id) < Number(latest.id),
+  );
+}
+
+async function retireSupersededLadonReviews({
+  github,
+  owner,
+  repo,
+  pullNumber,
+  botLogin,
+  headSha,
+}) {
+  const reviews = await github.paginate(github.rest.pulls.listReviews, {
+    owner,
+    repo,
+    pull_number: pullNumber,
+    per_page: 100,
+  });
+  const superseded = supersededChangeRequests(reviews, botLogin, headSha);
+
+  for (const review of superseded) {
+    await github.rest.pulls.dismissReview({
+      owner,
+      repo,
+      pull_number: pullNumber,
+      review_id: review.id,
+      message: `Superseded by Ladon approval of ${headSha}.`,
+      event: 'DISMISS',
+    });
+  }
+
+  return superseded.map((review) => review.id);
+}
+
+module.exports = {
+  latestBotReview,
+  retireSupersededLadonReviews,
+  supersededChangeRequests,
+};

scripts/retire-superseded-ladon-reviews.test.cjs

@@ -0,0 +1,99 @@
+#!/usr/bin/env node
+
+const assert = require('node:assert/strict');
+const test = require('node:test');
+
+const {
+  retireSupersededLadonReviews,
+  supersededChangeRequests,
+} = require('./retire-superseded-ladon-reviews.cjs');
+
+const BOT = 'aao-secretariat[bot]';
+const HEAD = 'new-head';
+
+function review(id, state, commitId = HEAD, login = BOT) {
+  return { id, state, commit_id: commitId, user: { login } };
+}
+
+test('selects only the bot change requests superseded by its final head approval', () => {
+  const reviews = [
+    review(10, 'CHANGES_REQUESTED', 'old-head'),
+    review(11, 'CHANGES_REQUESTED'),
+    review(12, 'CHANGES_REQUESTED', 'old-head', 'human-reviewer'),
+    review(13, 'DISMISSED', 'old-head'),
+    review(14, 'APPROVED'),
+  ];
+
+  assert.deepEqual(
+    supersededChangeRequests(reviews, 'AAO-SECRETARIAT[BOT]', HEAD).map(
+      ({ id }) => id,
+    ),
+    [10, 11],
+  );
+});
+
+test('keeps change requests when the latest bot review is not an approval', () => {
+  const reviews = [
+    review(20, 'CHANGES_REQUESTED', 'old-head'),
+    review(21, 'APPROVED'),
+    review(22, 'COMMENTED'),
+  ];
+
+  assert.deepEqual(supersededChangeRequests(reviews, BOT, HEAD), []);
+});
+
+test('keeps change requests when the latest approval targets an older head', () => {
+  const reviews = [
+    review(30, 'CHANGES_REQUESTED', 'older-head'),
+    review(31, 'APPROVED', 'previous-head'),
+  ];
+
+  assert.deepEqual(supersededChangeRequests(reviews, BOT, HEAD), []);
+});
+
+test('dismisses every selected review through the pull request API', async () => {
+  const dismissed = [];
+  const listReviews = Symbol('listReviews');
+  const github = {
+    paginate: async (method, params) => {
+      assert.equal(method, listReviews);
+      assert.deepEqual(params, {
+        owner: 'adcontextprotocol',
+        repo: 'adcp-client-python',
+        pull_number: 1134,
+        per_page: 100,
+      });
+      return [
+        review(40, 'CHANGES_REQUESTED', 'old-head'),
+        review(41, 'APPROVED'),
+      ];
+    },
+    rest: {
+      pulls: {
+        listReviews,
+        dismissReview: async (params) => dismissed.push(params),
+      },
+    },
+  };
+
+  const ids = await retireSupersededLadonReviews({
+    github,
+    owner: 'adcontextprotocol',
+    repo: 'adcp-client-python',
+    pullNumber: 1134,
+    botLogin: BOT,
+    headSha: HEAD,
+  });
+
+  assert.deepEqual(ids, [40]);
+  assert.deepEqual(dismissed, [
+    {
+      owner: 'adcontextprotocol',
+      repo: 'adcp-client-python',
+      pull_number: 1134,
+      review_id: 40,
+      message: `Superseded by Ladon approval of ${HEAD}.`,
+      event: 'DISMISS',
+    },
+  ]);
+});