Skip to content

fix(import): paginate GitHub PR commits in REST fallback - #1292

Merged
jeff-r2026 merged 1 commit into
Tencent:mainfrom
ydflow:fix/github-pr-commit-pagination
Oct 23, 2026
Merged

jeff-r2026 merged 1 commit into
Tencent:mainfrom
ydflow:fix/github-pr-commit-pagination

Conversation

@ydflow

@ydflow ydflow commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

Summary

When gh is unavailable or fails, GitHub PR import fetches only commits?per_page=50. A PR with 51 or more commits therefore loses later commit headlines from the learning-extraction prompt without reporting an error.

Reuse the existing readPages helper for the REST commit list. It reads pages of 100 in order and propagates errors from any page before extraction, instead of treating a partial history as complete. Add unit and packaged-CLI regressions, plus a short English/Chinese guide note.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)

Test Plan

  • npm run build
  • npx tsc --noEmit
  • npm run lint
  • npx vitest run src/__tests__/github-mr-fetch.test.ts src/__tests__/import-mr.test.ts src/__tests__/import-mr-threads.test.ts --maxWorkers=2: 48 passed.
  • npx vitest run --config vitest.e2e.config.ts src/__tests__/e2e/import-mr-commit-pagination.test.ts src/__tests__/e2e/import-mr-response-errors.test.ts --retry=0: 5 passed after building.
  • npx vitest run --config vitest.windows.config.ts --maxWorkers=4: 665 passed.
  • git diff --check and contributor preflight.
  • Full unit suite: attempted on Windows with npx vitest run --maxWorkers=4, then stopped after failures in unrelated modules (local-agent, env-loader, uninstall, hook-handlers). The first local-agent failure, expecting POSIX mode 0600 but receiving 0666, also fails on pristine upstream 0d57ac4. Other full-suite failures were not individually audited. Full Linux/macOS coverage is left to CI; this is not a full-suite pass.

Real CLI verification

Both new CLI cases failed before the production change. With a local HTTP fixture and gh forced unavailable, the rebuilt teamai import --from-mr ... --all --no-discussions --output ... now:

  1. Requests both commit pages for a 101-commit PR, passes all 101 headlines (including commits 51 and 101) to the AI subprocess, writes the learning, and exits 0.
  2. When page 2 returns 429, exits 1 before calling the AI subprocess and writes no learning.

The HTTP client, prompt construction, command handling and output writes are real; the external AI CLI is a deterministic subprocess fixture. No real model was called.

Unit cases cover 0, 1, 50, 51, 100, 101 and 250 commits, preserving order and headlines, plus HTTP errors, malformed lists and interrupted streams on a later page.

Related Issues

No matching open issue or duplicate PR found during screening. This follows the REST fallback error handling in #1150 / #1189 and reuses the pagination helper introduced in #1190.

Notes for Reviewers

The production change is confined to one call in src/providers/github/mr-fetch.ts. No new pagination abstraction or dependency is added. The GitHub PR commits endpoint's own 250-commit limit still applies; this does not promise arbitrary-length commit histories.

Signed-off-by: ydflow <314143294+ydflow@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown

No findings in the specified PR diff.

The PR description satisfies the testing requirement: it records rebuilt real-CLI verification for both 101-commit pagination and a second-page HTTP failure, with explicit exit-code, extraction, and output checks. Deferred Linux/macOS coverage is clearly disclosed.

Review was read-only; I did not run, build, or install PR code.

@SaulMoro SaulMoro left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was generated by AI during triage.

@ydflow Thanks, this fixes what the description says. We built e11f7f4f and origin/main (0d57ac46) and ran teamai import --from-mr <github PR> --all against a local GitHub stand-in with gh unavailable and a PR of 101 commits:

Build Commit pages requested Commit lines in the extraction prompt 429 on the second page
main per_page=50 50 never requested; exit 0
this PR per_page=100, pages 1 and 2 101 exit 1 (GitHub API error 429), no learning written

tsc, lint, the unit files and the new e2e pass on this head, and the e2e fails on main as expected. The guide note is in both languages.

Non-blocking

[P2 non-blocking] GitHub's list-commits endpoint stops at 250 commits, which the guide note already says. A PR with more commits still gets a truncated history and no warning. The PR object carries a commits count, so the REST path could compare the two and warn, or fail, when the list comes back short. That's optional, and a follow-up is fine.

Note, outside this PR: the GitLab and GitCode fetchers still use per_page=50.

@jeff-r2026
jeff-r2026 merged commit 328e87c into Tencent:main Oct 23, 2026
18 checks passed
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.

3 participants