Skip to content

fix: list pull requests of more than 250 commits via compare - #362

Open
gilescope wants to merge 2 commits into
dcoapp:mainfrom
gilescope:giles-dco-250-commits
Open

gilescope wants to merge 2 commits into
dcoapp:mainfrom
gilescope:giles-dco-250-commits

Conversation

@gilescope

Copy link
Copy Markdown

Fixes #327.

Problem

A pull request with more than 250 commits can never get a DCO verdict. At 250
REST commits the app falls back to GraphQL pullRequest.commits, but that
connection has the same 250 cap while reporting the true totalCount, so the
completeness check always throws ("returned 250 of N") - hence
HTTP status: unknown on every retry.

Measured on nodejs/node#66667 (340 commits):

Source Commits returned
pulls.listCommits 250 (the newest)
GraphQL pullRequest.commits 250, totalCount 340
compare/{base.sha}...{head.sha}, paginated 340

Fix

  • At the cap, page repos.compareCommits(base.sha...head.sha) - the List
    commits family GitHub's docs point to - reusing the merge group pager.
  • Fail closed unless the range is complete and contains every commit
    pulls.listCommits returned.
  • GraphQL query and mapping code removed (-930/+182 lines overall).
  • The failure summary includes the reason when no HTTP status exists, which
    is what hid the cause in DCO fails on PRs with more than 250 commits #327.

Notes for review

  • pulls.listCommits stays the source below 250, so ordinary pull requests
    are untouched.
  • base.sha...head.sha matched pull_request.commits on 40 of 40 sampled
    open fork pull requests with old bases (kubernetes/kubernetes) and on the
    merged 340-commit one above. A compare against the base ref returns 0
    commits once merged, so shas are used.
  • Not verified against a live installation of the app.

🤖 Generated with Claude Code

https://claude.ai/code/session_01W9Bas7vxoZujc8jZZ88DLq

gilescope and others added 2 commits November 11, 2026 18:03
pulls.listCommits serves only the newest 250 commits of a pull request,
and GraphQL's pullRequest.commits has the same cap while still reporting
the true totalCount. The GraphQL fallback therefore always ended in
"returned 250 of N" and the check failed closed on every retry.

At the cap, page repos.compareCommits(base.sha...head.sha) instead, with
the pager the merge group path already uses. The range is rejected unless
it is complete and contains every commit pulls.listCommits returned.

The failure summary now carries the reason when there is no HTTP status.

Fixes dcoapp#327

Assisted-by: Claude:claude-fable-5-2 claude-code
Claude-Session: https://claude.ai/code/session_01W9Bas7vxoZujc8jZZ88DLq
Signed-off-by: Giles Cope <gilescope@gmail.com>
@vercel

vercel Bot commented Nov 11, 2026

Copy link
Copy Markdown

@giles-bot is attempting to deploy a commit to the DCO App Team on Vercel.

A member of the Team first needs to authorize it.

@gilescope
gilescope marked this pull request as ready for review November 11, 2026 18:08
@gilescope
gilescope requested a review from a team November 11, 2026 18:08

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DCO fails on PRs with more than 250 commits

3 participants