Skip to content

Fix critical GitHub API pagination bugs in auto-merge workflow #5580

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
xwang233 merged 2 commits into from
Nov 22, 2025
+17 −42
Merged

Fix critical GitHub API pagination bugs in auto-merge workflow #5580

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5e1a2f0
Fix critical GitHub API pagination bugs in auto-merge workflow
xwang233 Nov 22, 2025
ae5a528
Simplify auto-merge logic by removing confusing pr_approved check
xwang233 Nov 22, 2025
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
59 changes: 17 additions & 42 deletions .github/workflows/auto-merge.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,19 +61,19 @@ jobs:
pr = data;
} else {
// For check_run events, we need to find the PR
const prs = await github.rest.pulls.list({
const prsData = await github.paginate(github.rest.pulls.list, {
owner: context.repo.owner,
repo: context.repo.repo,
state: 'open',
head: `${context.repo.owner}:${context.payload.check_run?.head_branch}`,
});

if (prs.data.length === 0) {
if (prsData.length === 0) {
core.info('No open PR found for this commit');
return { should_skip: true };
}

pr = prs.data[0];
pr = prsData[0];
}

// Defensive checks: skip if PR is from a fork
Expand Down Expand Up @@ -132,12 +132,11 @@ jobs:
no_failed_checks_reason: '',
pr_mergeable: false,
pr_mergeable_reason: '',
pr_approved: false,
pr_approved_reason: '',
pr_mergeable_state: '',
};

// 1. Get all commit statuses (for nvfuser-ci and other status checks)
const { data: statuses } = await github.rest.repos.listCommitStatusesForRef({
const statuses = await github.paginate(github.rest.repos.listCommitStatusesForRef, {
owner: context.repo.owner,
repo: context.repo.repo,
ref: sha,
Expand Down Expand Up @@ -170,11 +169,12 @@ jobs:
);

// 2. Get all check runs
const { data: checkRuns } = await github.rest.checks.listForRef({
const checkRunsData = await github.paginate(github.rest.checks.listForRef, {
owner: context.repo.owner,
repo: context.repo.repo,
ref: sha,
});
const checkRuns = { check_runs: checkRunsData };

core.info(`Found ${checkRuns.check_runs.length} check runs`);

Expand Down Expand Up @@ -236,6 +236,9 @@ jobs:
core.info('Refetched PR to get mergeable status');
}

// Store mergeable_state for display in status comment
checks.pr_mergeable_state = pr.mergeable_state;

if (pr.mergeable === false) {
checks.pr_mergeable = false;
checks.pr_mergeable_reason = 'has merge conflicts';
Expand All @@ -250,35 +253,10 @@ jobs:
core.info('✅ PR is mergeable');
}

// 5. Check approval status based on mergeable_state
// mergeable_state values: clean, blocked, unstable, behind, dirty, unknown, draft
// - 'blocked': branch protection rules not satisfied (missing approvals or required checks)
// - 'clean': all requirements met, ready to merge
// - 'unstable'/'behind': mergeable but pending/failed non-required checks
// Note: We check approvals separately from check completion status
if (pr.mergeable_state === 'blocked') {
// Blocked means branch protection rules aren't satisfied
// This could be missing approvals OR required checks not passing
checks.pr_approved = false;
checks.pr_approved_reason = 'branch protection rules not satisfied';
core.info('❌ PR approval: blocked by branch protection');
} else if (pr.mergeable_state === 'clean' || pr.mergeable_state === 'unstable' || pr.mergeable_state === 'behind') {
// These states mean the PR has required approvals (not blocked)
// Check completion is validated separately in no_failed_checks
checks.pr_approved = true;
core.info('✅ PR is approved');
} else {
// Other states (dirty, unknown, draft, etc.)
checks.pr_approved = false;
checks.pr_approved_reason = `mergeable_state: ${pr.mergeable_state}`;
core.info(`❌ PR approval: ${checks.pr_approved_reason}`);
}

// Determine if ready to merge
const ready = checks.internal_ci_passed &&
checks.no_failed_checks &&
checks.pr_mergeable &&
checks.pr_approved;
checks.pr_mergeable;

return {
ready: ready,
Expand Down Expand Up @@ -310,14 +288,6 @@ jobs:
const checks = checkResult.checks;
const statusLines = [];

// PR approved
if (checks.pr_approved) {
statusLines.push('✅ PR is approved');
} else {
const reason = checks.pr_approved_reason ? ` (${checks.pr_approved_reason})` : '';
statusLines.push(`❌ PR is approved${reason}`);
}

// Internal CI
if (checks.internal_ci_passed) {
statusLines.push('✅ Internal CI is finished');
Expand All @@ -342,10 +312,15 @@ jobs:
statusLines.push(`❌ PR is mergeable${reason}`);
}

// Show mergeable_state for transparency
if (checks.pr_mergeable_state) {
statusLines.push(`ℹ️ PR mergeable_state: \`${checks.pr_mergeable_state}\``);
}

const statusText = statusLines.join('\n');

// Find the comment with placeholders
const { data: comments } = await github.rest.issues.listComments({
const comments = await github.paginate(github.rest.issues.listComments, {
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: pr_number,
Expand Down