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
28 changes: 27 additions & 1 deletion .github/workflows/ai-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ on:
paths-ignore:
- ".github/workflows/ai-review.yml"
- "LADON.md"
- "scripts/retire-superseded-ladon-reviews.cjs"

jobs:
code_review:
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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).`);
2 changes: 2 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
67 changes: 67 additions & 0 deletions scripts/retire-superseded-ladon-reviews.cjs
Original file line number Diff line number Diff line change
@@ -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,
};
99 changes: 99 additions & 0 deletions scripts/retire-superseded-ladon-reviews.test.cjs
Original file line number Diff line number Diff line change
@@ -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',
},
]);
});
Loading