Synced from GitHub
fix(forgejo): merged and closed pull request lists only return their own rows#24772
Problem
Forgejo's API has no merged list state, so listChangeRequests maps a merged request to state=closed — which returns merged and closed-unmerged rows alike. Every row was then pushed without filtering, so a merged list also delivered closed-unmerged pull requests and a closed list also delivered merged ones. The web client discards rows whose state does not match the tab, so the visible symptom is shortened or stalled lists: both lists burn their page limit on rows the client throws away, and a repository whose closed pull requests are mostly merged can page repeatedly without filling the Closed tab.
The row schema already decodes merged and forgejoChangeRequest already maps each row's true state, so the information to filter was present; the GitHub and GitLab providers already narrow their lists by state, so this is a provider-side omission rather than an API limitation.
Change
Filter listed rows by their mapped state when the requested state is not all, and count only matching rows toward input.limit. consumed still counts raw rows, so the delivered cursor keeps advancing over the same row stream the next request resumes from — no row is skipped or delivered twice. state=open queries are already narrowed server-side and state=all keeps returning everything, so only the closed/merged paths change.
Scope and approval
No linked issue; filed under the small focused fix for an obvious bug. One provider file plus a regression test, no contract, client, or product-behavior changes beyond returning the rows the list asked for.
Verification
- New test
filters listed pull requests by the requested stateinForgejoPullRequestProvider.test.ts: two closed rows, one merged. On main it fails withexpected [ 1, 2 ] to deeply equal [ 1 ](the merged list returns both rows); with the fix, merged returns[1], closed returns[2], all returns[1, 2], open returns[]. vp test run packages/source-control-forgejo— 49 pass, 1 fail. The failure isForgejoCheckout.test.ts, which fails identically on pristineorigin/mainon this Windows machine (expected 'pull request change\r\n' to equal 'pull request change\n', a CRLF checkout assertion) and is unrelated to this change.vp fmt --checkandvp linton both changed files — clean.vp run --filter ./packages/source-control-forgejo typecheck— passes (one pre-existing effect-fn suggestion in a file this PR does not touch).
Fix, tests, and PR description by GLM-5.3 in ZCode, directed and verified by @rishabhx29.
Checks
22 successful · 14 skipped
Added 1 commit
- fix(forgejo): merged and closed pull request lists only return their own rowsc3ef01f
Synced from GitHub
Approvability
Verdict: Approved at c3ef01f
Macroscope's review found this PR approvable — The PR makes Forgejo's existing merged and closed pull-request lists return only rows matching the requested state, while preserving raw-row cursor advancement. The production change is small and is covered by a focused regression test.
You can add or adjust custom eligibility rules. Learn more.
Added 1 commit
- Merge branch 'main' into fix/forgejo-pull-request-state-filterce57f08
Synced from GitHub
Important
Review skipped
Review was skipped as selected files did not have any reviewable changes.
External image Run configuration
- Source mode: Auto
- Repository: pingdotgg/t3code
- Branch: main
- File: .coderabbit.config.ts
- Commit: 665351f
- Layers: Repository TS config
- Review profile: CHILL
- Plan: Advanced
- Run ID:
d1f3ddde-143d-4789-be9c-b3dfe3459651
External image Commits
Reviewing files that changed from the base of the PR and between 3325f124960fcd441f3abd46562a71f422762362 and 6087c15a37769c19e28baef72fd5dd618e33fff4.
You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.
Use the checkbox below for a quick retry:
- Trigger review
No actionable comments were generated in the recent review.
External image Recent review info
External image Run configuration
- Source mode: Auto
- Repository: pingdotgg/t3code
- Branch: main
- File: .coderabbit.config.ts
- Commit: b3d382e
- Layers: Repository TS config
- Review profile: CHILL
- Plan: Advanced
- Run ID:
38b82ec1-77c5-4afa-b36d-81dd81f73647
External image Commits
Reviewing files that changed from the base of the PR and between b3d382e1009d5430de8c528aef99a39854a7cbc4 and c3ef01f908f61982b6edb9f00423fa41c115a58c.
External image Files selected for processing: 2
packages/source-control-forgejo/src/server/ForgejoPullRequestProvider.test.tspackages/source-control-forgejo/src/server/ForgejoPullRequestProvider.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 Forgejo pull request listing now filters results by the requested state, except for
"all". The test covers merged, closed, all, and open requests.Changes: Forgejo pull request state filtering
Layer / File(s) Summary Filter results by requested state
packages/source-control-forgejo/src/server/ForgejoPullRequestProvider.ts,packages/source-control-forgejo/src/server/ForgejoPullRequestProvider.test.tsThe listing filters mapped rows by the requested state and counts matching rows toward the limit. The test checks results for merged, closed, all, and open states. Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix
Suggested reviewers:
juliusmarminge,rjwadley
Merge Risk: External image Minimal · previous review c3ef0
The state-filtering change is ready to merge after normal checks; no merge-blocking issue remains.
Pre-merge checks: Passed 4
Check name Status Explanation Title check Passed The title names the primary change: merged and closed pull request lists now return only their own rows, which matches the filtering fix in ForgejoPullRequestProvider.ts and its regression test. It us… 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. Description check Passed The description follows the template's main sections (Problem, Change, Scope and approval, Verification) and is detailed. It explains the root cause, the fix, and why state=open and state=all are unaf… Full details: Title check
Explanation
The title names the primary change: merged and closed pull request lists now return only their own rows, which matches the filtering fix in ForgejoPullRequestProvider.ts and its regression test. It uses a conventional commit prefix and is a single, specific sentence.
Full details: Description check
Explanation
The description follows the template's main sections (Problem, Change, Scope and approval, Verification) and is detailed. It explains the root cause, the fix, and why state=open and state=all are unaffected. It gives concrete verification: a regression test with before/after results, the full package test run with one unrelated Windows-specific CRLF failure, and clean fmt, lint, and typecheck runs. The Scope and approval section cites no linked issue or maintainer approval. It argues the change qualifies as a small focused fix of an obvious bug, which the template allows. The template's trailing line asks for the model and harness used; the description credits GLM-5.3 in ZCode, so that is covered. The template's header comment asks for a conventional commit title, which the PR objectives reference as 'fix(forgejo): ...'.
Finishing Touches
🧪 Generate unit tests (beta)
- Create a new PR
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Added 3 commits
Synced from GitHub
@juliusmarminge can u check out this pr when u get time
Added 2 commits
Note
Written by gpt-6.1-sol on behalf of Maria
Verified in a real dev environment
verified as-is at 687871a3c8637210c83a31477982e6831eb67d8d. no code changes.
installed and ran github/main (5fe18d79ac) in a separate worktree, then the exact pr head, with one background dev server at a time. drove the web app using the repo's playwright and /usr/bin/google-chrome. added the real git project and threads through the app; real codex gpt-6.1-sol turns read the readme and ran git remote -v on both revisions.
a real local forgejo 12.0.4 server, authenticated through tea 0.16.0, held 128 pull requests backed by real commits and branches: 3 open, 14 closed-unmerged, 111 merged.
- main: the closed view displayed only 7 of 14 rows.
pullRequests.listwithstate=closed, limit=1returned merged #128. both closed and merged responses mixed states. - pr: the closed view displays all 14 rows without load more. the exact one-row request returns closed #56. closed pagination with limit 5 returns 5 + 5 + 4 matching rows, with every expected id exactly once.
- neighboring behavior: open returns 3 open rows; merged pagination returns 99 + 12 = 111 unique merged rows; all-state pagination returns 99 + 29 = 128 unique rows. exercised state selection and merged load more through the web ui. the recording includes real loading waits and is embedded as an animated gif for github.
- blacksmith: all 53 forgejo package tests passed.
vp run check:changed 80a2351fcb909034e65a512dfabeb3cdfd367762passed format, lint, knip, typechecks for 11 packages, and tests. the worker snapshot initially lacked a merge base; fetching this pr's immediate history resolved it. current ci: 29 successful, 0 pending, 0 failing.
server verification transcript
before on main: closed list shows 7 rows
after on the pr: closed list shows all 14 rows
state filters and merged pagination in the real dev app
real codex gpt-6.1-sol turn through the pr web client
Not verified
native ios/android, electron desktop, and macos/windows were not exercised on this linux vm. remote relay/tunnel connections, the fj cli, and externally hosted forgejo/gitea versions were not exercised; this pass used the real local forgejo service and tea path.