Skip to content

fix(server): Forgejo branch status no longer pages through every pull request - #17201

Closed
aFlamee wants to merge 1 commit into
pingdotgg:mainfrom
aFlamee:fix/forgejo-pr-lookup-page-cap
Closed

aFlamee wants to merge 1 commit into
pingdotgg:mainfrom
aFlamee:fix/forgejo-pr-lookup-page-cap

Conversation

@aFlamee

@aFlamee aFlamee commented Oct 8, 2026

Copy link
Copy Markdown

Problem

On a Forgejo repository with many pull requests, branch status refreshes take minutes and the "Some requests are slow" toast for vcs.refreshStatus stays up permanently.

The Forgejo pulls API has no head-branch filter, so ForgejoSourceControlProvider.listChangeRequests pages through /pulls?state=all until it finds limit (20) matches or reaches the end. A branch has at most a few PRs, so a branch without a PR always reaches the end and requests every page. On a self-hosted Forgejo 15 server with 535 PRs, each 50-item page takes 13–17 s, and about half hit the 30 s timeout. The PR lookup takes 2–6 minutes per refresh and repeats when the 5-minute "no PR" cache expires.

Change

Cap the head-branch scan at the two most recently updated pages (100 PRs). The list is already sorted by recentupdate, so a branch that is being worked on finds its PR there. A PR on an old branch that is outside the 100 most recently updated is no longer shown as a badge. This affects both findLatestPrForHeadContext (state: "all") and findOpenPr (state: "open"). If an open PR is missed, Forgejo still rejects a duplicate PR on creation.

Scope and approval

This is a very small, focused fix for an obvious bug: an unbounded scan over every PR in the repository on each branch status refresh. It changes one loop bound in the Forgejo adapter and does not affect other providers, which filter by head branch on the server.

Verification

  • Added ForgejoSourceControlProvider.test.ts, which mocks a repository with five full pages and no matching PR. Without the fix, the lookup makes 6 API requests. With the fix, it makes 2. vp test run src/sourceControl/ForgejoSourceControlProvider.test.ts passes.
  • vpr typecheck in apps/server reports no errors. vp fmt --check and vp lint pass on the changed files.
  • Measured on the affected server: GET /pulls?state=all&sort=recentupdate&limit=50 takes about 15 s per page with X-Total-Count: 535. The server traces showed findLatestPrForHeadContext at up to 217 s before the fix. I did not run a patched desktop build against that server.

Written with Claude Opus 5.5 in Claude Code (running inside T3 Code).

🤖 Generated with Claude Code

… request

Forgejo cannot filter pulls by head branch, so a branch without a PR scanned
the whole repository on every status refresh. Search only the two most
recently updated pages.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Oct 8, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at e7bb2cc

Macroscope's review found this PR approvable — This is a focused Forgejo performance fix that bounds branch-status lookups to two pages and includes a regression test, with no broader API or infrastructure impact. Very old pull requests may no longer appear in branch status, but that tradeoff is explicit and contained.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 8, 2026

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 66aa0c9c-af48-44d5-9c3e-d6c920b0439b
📥 Commits

Reviewing files that changed from the base of the PR and between a4c9494 and e7bb2cc.

📒 Files selected for processing (2)
  • apps/server/src/sourceControl/ForgejoSourceControlProvider.test.ts
  • apps/server/src/sourceControl/ForgejoSourceControlProvider.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

Walkthrough

Forgejo pull-request listing now searches at most two pages when matching a requested head branch. A provider test checks that the search stops after two API requests when the requested branch has no matching pull requests.

Changes

Forgejo pull-request lookup

Layer / File(s) Summary
Bound head-branch search
apps/server/src/sourceControl/ForgejoSourceControlProvider.ts, apps/server/src/sourceControl/ForgejoSourceControlProvider.test.ts
listChangeRequests now stops searching after two pages. The test uses mocked pages with pull requests for another branch and checks that the provider makes two API requests and returns no results.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to e7bb2

The change bounds Forgejo branch-status lookups to the 100 most recently updated pull requests. Older pull requests may no longer appear as branch badges, which the PR states as intended. No merge-blocking risk was identified.

Security Architecture Review

Security architecture risk: 🔵 Low · up to e7bb2

The change limits slow Forgejo searches, but an older pull request may disappear from branch status and trigger an unnecessary creation attempt. Repository access still follows the existing Forgejo path; duplicate-request handling by the deployed server has not been verified.

Retained concerns

  • Low · architecture · inferred: An existing pull request beyond the first two pages can be treated as absent by both branch status and the pre-create check. A later creation attempt still requires Forgejo authorization, but the application’s existing-request guard and status recovery no longer reliably identify that request.
Security review details

Security Blast Radius

  • inferred — The effective change is confined to Forgejo repositories whose relevant request falls outside the scanned pages. Their branch-status and create workflows can receive a false negative; the change does not expand repository selection or credentials.

Security Findings and Attack Paths

  • inferred — A false-negative pre-create lookup can lead to an additional authenticated POST. The inspected code does not establish that Forgejo will accept a duplicate, and the deployed server’s duplicate enforcement and ambiguous-POST recovery remain unverified; no privilege-bypass path was established.

Trust Boundaries and Controls

  • observed — Repository resolution, credential selection, HTTP-error handling, and the Forgejo POST boundary remain in the existing CLI path; the changed loop does not grant an additional authority.

Resilience and Maintainability Implications

  • inferred — A successful false-negative lookup differs from a transient lookup failure: it replaces last-known status rather than using the failure fallback. Refreshing the cache cannot rediscover a request that remains outside the two-page window.

Hardening Proposals

  • proposed — If creation requires authoritative existing-request detection, keep that check distinct from the bounded status lookup or reconcile the POST outcome with an authoritative request identity.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: limiting Forgejo pull-request pagination during branch status checks.
Description check ✅ Passed The description covers the required Problem, Change, Scope and approval, and Verification sections. It explains the performance issue, the two-page limit, the accepted behavior trade-off, focused test…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@juliusmarminge

Copy link
Copy Markdown
Member

Note

Grok responding on behalf of Julius.

Thanks for digging into this and for the measurements. The same unbounded Forgejo pull request scan in findLatestPrForHeadContext was just fixed on main by #12223, which passes head=<branch> to Forgejo's pulls endpoint so the server filters by branch instead of the client paging through every pull request (it keeps the client-side match as a fallback for older servers). Since that covers this PR's change, I'm closing this one as superseded. If you still see slow vcs.refreshStatus on your Forgejo 15 server after updating to a build that includes #12223, please open an issue with the trace and we'll take another look.

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

Labels

size:XS 0-9 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants