Skip to content

fix(gitlab): keep paging merge requests when GitLab omits the totals - #33801

Closed
Hotragn wants to merge 1 commit into
stablyai:mainfrom
Hotragn:fix/gitlab-mr-list-missing-pagination-headers
Closed

Hotragn wants to merge 1 commit into
stablyai:mainfrom
Hotragn:fix/gitlab-mr-list-missing-pagination-headers

Conversation

@Hotragn

@Hotragn Hotragn commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

ELI5

On a big GitLab project, GitLab sometimes doesn't say how many pages of merge requests there are. Orca then assumed "1 page", so you could never get past the first page in the Merge Requests list. Now, if a page comes back full, Orca offers the next one, the same way the GitLab issues list already works.

What Changed

  • Before: listMergeRequests (src/main/gitlab/merge-request-list.ts) used x-total-pages, else ceil(x-total / perPage). GitLab omits both headers once a listing has more than 10,000 rows (common for state=all or merged MRs on large projects), and a proxy can strip them. Then the second fallback became Math.max(1, ceil(0 / perPage)) = 1, and the pager stopped at page 1 even when that page was full.
  • After:
    • If x-total-pages is present, it still wins.
    • Else if x-total is present, it still decides.
    • Only when both are missing: a full page reports page + 1, and a short page reports page.
    • totalCount stays x-total, or 0 when it's unknown, as before.

Why

This is the rule src/main/gitlab/issues.ts already uses for the same missing-headers case (#13357), and the one the glab mr list fallback in this same file uses. Reusing it keeps the two GitLab lists consistent instead of adding a second pagination strategy. A probe costs at most one extra request on the last page, which comes back short and ends paging.

Linked Issue

Fixes #33800

Visual Proof

N/A: no UI change. The existing pager now gets a correct page count, which the test below checks.

Testing

  • I manually tested these changes locally

  • Automated tests added/updated, or explained why not below

  • New case in client-mr-listing.test.ts:

    • a full page with no headers reports one more page;
    • a short page with no headers is the last;
    • with only x-total present, the count still decides.
  • It fails on the old code (totalPages: 1).

  • src/main/gitlab passes (19 files, 301 tests), and so do pnpm typecheck, oxlint and pnpm run check:code-quality:changed. Tested on macOS.

AI Disclosure

I used Claude Code (Claude Opus 5.5) to search the GitLab helpers for bugs; it found the mismatch with the issue list. I checked it against issues.ts, wrote the fix and test, and reviewed them.

Review

My review of the change, with the agent's help:

  • Cross-platform: pure logic, no platform code.
  • SSH / remote: glab runs wherever it runs today; only the page-count math changed.
  • Git providers: GitLab only. GitHub and the other providers are untouched.
  • Performance: at most one extra request, at the end of a list whose size GitLab didn't report.
  • Compatibility: the result shape is unchanged.

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

Ensure no issues in: Security, Cross-platoform support (Linux, Windows, Mac), Remote SSH, Mobile, general backwards compatibility, performance

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

GitLab leaves out x-total and x-total-pages once a listing passes 10,000
rows, and a proxy can strip them. The merge request list then computed
ceil(0 / perPage) and reported one page, so later merge requests could
not be reached.

Use the rule the issue list already uses: with no totals, a full page
advertises one more page to probe and a short page is the last.

Fixes stablyai#33800
@coderabbitai

coderabbitai Bot commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 34e9cc72-ca09-4803-943f-fff1a6ae93a9
Commits

Reviewing files that changed from the base of the PR and between 6cb6d0d and 528716a.

Files selected for processing: 2
  • src/main/gitlab/client-mr-listing.test.ts
  • src/main/gitlab/merge-request-list.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Walkthrough

The GitLab merge-request listing now infers totalPages from the response length when both pagination total headers are absent. A full page indicates a possible next page; a shorter page indicates the current page is the last. When x-total is available, the listing uses it to calculate totalPages and totalCount. Tests cover these cases.

Priority: Normal

Severity of issue fixed: Medium

Merge Risk: Minimal · up to 52871

The merge-request list should continue past a full page when pagination totals are absent. No actionable merge-blocking risk is identified.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly and concisely describes the GitLab merge-request pagination fix when total headers are missing.
Description check Passed The description is complete and aligned with the template. It explains the behavior change, motivation, linked issue, testing, AI use, compatibility considerations, and checklist status. Manual testin…
Linked Issues check Passed Issue #33800 requires MR pagination to continue when both total headers are absent. merge-request-list.ts now uses page + 1 for a full page and page for a short page. It preserves `x-total-pages…
Out of Scope Changes check Passed The changes are limited to MR pagination logic and its automated tests. The test and comments directly support issue #33800. No unrelated provider, UI, or behavior changes appear in the reviewed diff.
Full details: Description check

Explanation

The description is complete and aligned with the template. It explains the behavior change, motivation, linked issue, testing, AI use, compatibility considerations, and checklist status. Manual testing was not performed, and results for lint and build are not explicitly reported, but these omissions are non-critical because automated tests and several validation commands are documented.

Full details: Linked Issues check

Explanation

Issue #33800 requires MR pagination to continue when both total headers are absent. merge-request-list.ts now uses page + 1 for a full page and page for a short page. It preserves x-total-pages, uses x-total when available, and keeps totalCount at 0 when the total is unknown. client-mr-listing.test.ts covers full-page, short-page, and x-total cases.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@nwparker

nwparker commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

Closing as superseded — the linked issue (#33800) has been resolved on main by #34171 (8f3d8c8583). Thanks for the contribution!

@nwparker nwparker closed this Nov 5, 2026
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.

[Bug]: GitLab merge request list stops at page 1 when GitLab omits the pagination headers

2 participants