Skip to content

Fix: VCS branch pagination#12311

Open
HarshMN2345 wants to merge 6 commits into
1.9.xfrom
vcs-branch-pagination
Open

Fix: VCS branch pagination#12311
HarshMN2345 wants to merge 6 commits into
1.9.xfrom
vcs-branch-pagination

Conversation

@HarshMN2345
Copy link
Copy Markdown
Member

What does this PR do?

(Provide a description of what this PR does and why it's needed.)

Test Plan

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Screenshots may also be helpful.)

Related PRs and Issues

  • (Related PR or issue)

Checklist

  • Have you read the Contributing Guidelines on issues?
  • If the PR includes a change to an API's metadata (desc, label, params, etc.), does it also include updated API specs and example docs?

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 14, 2026

Greptile Summary

This PR replaces the previously unbounded GitHub branch-listing loop with a bounded listBranches helper that pages through up to 1,000 branches (10 pages × 100 per page) and throws an explicit exception when a repository exceeds that limit; it also bumps utopia-php/vcs from 4.0.0 to 4.1.0 to expose the perPage / page pagination parameters on GitHub::listBranches.

  • Introduces GITHUB_BRANCHES_PER_PAGE = 100 and GITHUB_BRANCHES_MAX_RESULTS = 1000 constants and a private listBranches method that loops over pages with a sentinel check (page 11, perPage 100) to detect repos with >1,000 branches.
  • The sentinel arithmetic is correct: page 11 at 100-per-page covers positions 1001–1100, so the exception fires only when a repository genuinely exceeds the declared cap and is never a false positive for a repo with exactly 1,000 branches.

Confidence Score: 4/5

The change is safe to merge with the caveat that repositories with more than 1,000 branches will receive an unhelpful exception; the core pagination logic is correct.

The bounded loop and sentinel check are arithmetically correct. The remaining concern is the error message telling users to narrow results via search when search is applied after listBranches returns, leaving users of large repositories with no viable recovery path.

src/Appwrite/Platform/Modules/VCS/Http/Installations/Repositories/Branches/XList.php — the exception message in listBranches warrants a closer look

Important Files Changed

Filename Overview
src/Appwrite/Platform/Modules/VCS/Http/Installations/Repositories/Branches/XList.php Adds a bounded listBranches helper that fetches up to 1,000 branches across 10 pages and throws an exception when the repo exceeds that cap; sentinel check logic is correct (perPage=100, page=11); error message guidance is misleading as previously noted
composer.lock Bumps utopia-php/vcs from 4.0.0 to 4.1.0 to gain pagination parameters in GitHub::listBranches; no other dependency changes

Reviews (5): Last reviewed commit: "Merge branch '1.9.x' into vcs-branch-pag..." | Re-trigger Greptile

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 14, 2026

✨ Benchmark results

Comparing 1.9.x (before) to vcs-branch-pagination (after).

Before

Scenario P50 (ms) P95 (ms) Requests RPS
API total 13.83 147.79 185 37.19
Account 23.59 179.55 35 7.71
TablesDB 13.53 24.29 35 8.35
Storage 11.91 50.37 75 17.63
Functions 19.72 29.1 40 9.55

After

Scenario P50 (ms) P95 (ms) Requests RPS
API total 15.32 154.45 185 32.75
Account 25.58 218.61 35 6.74
TablesDB 15.18 21.4 35 7.96
Storage 12.32 50.26 75 16.77
Functions 21.12 34.18 40 9.05

Delta

Scenario P95 delta (ms)
API total +6.66
Account +39.06
TablesDB -2.89
Storage -0.12
Functions +5.08
Top API waits
API request Max wait (ms)
account.sessions.email.create 627.17
account.password.update 218.49
account.create 176.22

$branches = [];
$maxPages = \intdiv(self::GITHUB_BRANCHES_MAX_RESULTS, self::GITHUB_BRANCHES_PER_PAGE);

for ($page = 1; $page <= $maxPages; $page++) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should accept pagination details as params to endpoint. Notice we already accept queries, lets utilize those - offset and limit queries.

I think we already have something similar done in other VCS endpoints, lets check there.

If not, any other list endpoint should showcase how it should be done - queries param, supporting offset and limit.

We should also proxy search term itself into github call, if that is possible by github APIs


$remainingBranches = $github->listBranches($owner, $repositoryName, self::GITHUB_BRANCHES_PER_PAGE, $maxPages + 1);
if (!empty($remainingBranches)) {
throw new Exception(Exception::GENERAL_QUERY_INVALID, 'Repository has too many branches to list. Narrow the results using the search parameter.');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Misleading error message — search cannot prevent this exception

The error message instructs users to "narrow the results using the search parameter," but the search parameter is applied client-side in the action method (line 104) after listBranches returns. Because the exception is thrown inside listBranches before any search filtering, passing search still triggers the same exception for any repository with more than 1,000 branches. Users who follow this advice will receive the exact same error and have no way to recover using the suggested approach.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 14, 2026

🔄 PHP-Retry Summary

Flaky tests detected across commits:

Commit 1e22d7c - 28 flaky tests
Test Retries Total Time Details
UsageTest::testFunctionsStats 1 10.18s Logs
UsageTest::testPrepareSitesStats 1 7ms Logs
UsageTest::testEmbeddingsTextUsageDoesNotBreakProjectUsage 1 5ms Logs
WebhooksCustomServerTest::testDeleteDeployment 1 28ms Logs
WebhooksCustomServerTest::testDeleteFunction 1 13ms Logs
WebhooksCustomServerTest::testCreateCollection 1 10ms Logs
WebhooksCustomServerTest::testCreateAttributes 1 10ms Logs
WebhooksCustomServerTest::testCreateDocument 1 10ms Logs
WebhooksCustomServerTest::testUpdateDocument 1 10ms Logs
WebhooksCustomServerTest::testDeleteDocument 1 10ms Logs
WebhooksCustomServerTest::testCreateTable 1 10ms Logs
WebhooksCustomServerTest::testCreateColumns 1 11ms Logs
WebhooksCustomServerTest::testCreateRow 1 11ms Logs
WebhooksCustomServerTest::testUpdateRow 1 11ms Logs
WebhooksCustomServerTest::testDeleteRow 1 11ms Logs
WebhooksCustomServerTest::testCreateStorageBucket 1 7ms Logs
WebhooksCustomServerTest::testUpdateStorageBucket 1 11ms Logs
WebhooksCustomServerTest::testCreateBucketFile 1 11ms Logs
WebhooksCustomServerTest::testUpdateBucketFile 1 5ms Logs
WebhooksCustomServerTest::testDeleteBucketFile 1 5ms Logs
WebhooksCustomServerTest::testDeleteStorageBucket 1 14ms Logs
WebhooksCustomServerTest::testCreateTeam 1 5ms Logs
WebhooksCustomServerTest::testUpdateTeam 1 10ms Logs
WebhooksCustomServerTest::testUpdateTeamPrefs 1 12ms Logs
WebhooksCustomServerTest::testDeleteTeam 1 5ms Logs
WebhooksCustomServerTest::testCreateTeamMembership 1 13ms Logs
WebhooksCustomServerTest::testDeleteTeamMembership 1 12ms Logs
WebhooksCustomServerTest::testWebhookAutoDisable 1 29ms Logs
Commit 0073ddd - 4 flaky tests
Test Retries Total Time Details
UsageTest::testFunctionsStats 1 10.16s Logs
UsageTest::testPrepareSitesStats 1 7ms Logs
UsageTest::testEmbeddingsTextUsageDoesNotBreakProjectUsage 1 5ms Logs
LegacyPermissionsGuestTest::testReadDocuments with data set #2 1 241.09s Logs

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.

2 participants