fix(web): browse pull requests across remote servers - #6108
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
github-actions
Bot
added
vouch:trusted
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| ...previous, | ||
| projectCount: previous.projectCount + provider.projectCount, | ||
| searchesOnHost: previous.searchesOnHost && provider.searchesOnHost, | ||
| configured: previous.configured && provider.configured, |
There was a problem hiding this comment.
🟡 Medium state/pullRequests.ts:132
mergeProviders merges same-host providers with configured: previous.configured && provider.configured, so if one environment has github.com configured but another does not, the combined summary reports configured: false. The host menu then marks the entire host unavailable even though pull requests from the configured environment are readable, preventing selection of that host in the All servers view. Consider using || so the merged host is available when at least one environment can read it.
| configured: previous.configured && provider.configured, | |
| configured: previous.configured || provider.configured, |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/state/pullRequests.ts around line 132:
`mergeProviders` merges same-host providers with `configured: previous.configured && provider.configured`, so if one environment has `github.com` configured but another does not, the combined summary reports `configured: false`. The host menu then marks the entire host unavailable even though pull requests from the configured environment are readable, preventing selection of that host in the All servers view. Consider using `||` so the merged host is available when at least one environment can read it.
| : { | ||
| ...previous, | ||
| projectCount: previous.projectCount + provider.projectCount, | ||
| searchesOnHost: previous.searchesOnHost && provider.searchesOnHost, |
There was a problem hiding this comment.
🟠 High state/pullRequests.ts:131
mergeProviders merges same-host providers from multiple environments using searchesOnHost: previous.searchesOnHost && provider.searchesOnHost, so when two servers share a host and only one supports server-side search, the merged host is treated as not searchable. The UI then falls back to local filtering of displayed fields and discards valid matches that the capable server returned from PR bodies, reviews, or commit messages — fields not present in the local row. Consider preserving searchesOnHost as previous.searchesOnHost || provider.searchesOnHost so the aggregate host is searchable when any contributing environment is.
| searchesOnHost: previous.searchesOnHost && provider.searchesOnHost, | |
| searchesOnHost: previous.searchesOnHost || provider.searchesOnHost, |
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/state/pullRequests.ts around line 131:
`mergeProviders` merges same-host providers from multiple environments using `searchesOnHost: previous.searchesOnHost && provider.searchesOnHost`, so when two servers share a host and only one supports server-side search, the merged host is treated as not searchable. The UI then falls back to local filtering of displayed fields and discards valid matches that the capable server returned from PR bodies, reviews, or commit messages — fields not present in the local row. Consider preserving `searchesOnHost` as `previous.searchesOnHost || provider.searchesOnHost` so the aggregate host is searchable when any contributing environment is.
| const multiEnvironmentList = Atom.family((key: string) => { | ||
| const { environmentIds, input } = JSON.parse(key) as MultiEnvironmentListKey; | ||
| const atoms = environmentIds.flatMap((environmentId) => { | ||
| const cursors = pullRequestCursorsForEnvironment(environmentId, input.cursors); |
There was a problem hiding this comment.
🟠 High state/pullRequests.ts:186
multiEnvironmentList skips any environment that has no scoped cursor whenever a continuation is requested. When one server returns a cursor while another returns truncated: true with no cursor (the contract's signal to increase limit), Load More sends the first server's cursor and line 189 drops the cursor-less server entirely — so its remaining pull requests can never be loaded while any cursor-bearing server still has pages. On line 189, pullRequestCursorsForEnvironment returns undefined both for environments absent from the continuation and for environments that need limit-based pagination, and the if treats both as "skip." Consider distinguishing the two cases so cursor-less-but-truncated environments are still queried (without a cursor) rather than excluded.
🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/web/src/state/pullRequests.ts around line 186:
`multiEnvironmentList` skips any environment that has no scoped cursor whenever a continuation is requested. When one server returns a cursor while another returns `truncated: true` with no cursor (the contract's signal to increase `limit`), Load More sends the first server's cursor and line 189 drops the cursor-less server entirely — so its remaining pull requests can never be loaded while any cursor-bearing server still has pages. On line 189, `pullRequestCursorsForEnvironment` returns `undefined` both for environments absent from the continuation and for environments that need limit-based pagination, and the `if` treats both as "skip." Consider distinguishing the two cases so cursor-less-but-truncated environments are still queried (without a cursor) rather than excluded.
… request page (adapted from pingdotgg#6108)
…vers (adapted from pingdotgg#6108)
1 participant
Problem
The Pull Requests page was permanently scoped to the primary environment, so projects and pull requests from connected remote T3 Code servers never appeared.
Fix
Verification
vp linton the changed web filesvp run --filter @t3tools/web typecheckvp run --filter @t3tools/web buildThe local Vitest runner currently fails before collecting tests with the same runner/config error on both changed and untouched baseline tests. Static checks and the production build pass. Authenticated before/after UI evidence will be added to this draft.
Generated with GPT-5.6 Codex in T3 Code.
Note
Support browsing pull requests across multiple remote servers
pullRequestsAcrossEnvironmentsatoms that aggregate per-environment list and stats results, with environment-scoped cursor keys to avoid cross-server collisions.environmentIdandselectedEnvironmentIdURL params, enabling users to scope the list to a single server or browse all connected servers.PullRequestFiltersMenuto include a 'Server' radio group for picking an environment independently of the project filter; project scope values are now JSON-encoded[environmentId, projectId]strings (previously plain project IDs).environmentIdtopullRequestEntryKey, diff stat keys, and right panel surface IDs so identical repository/PR numbers on different servers don't collide.'all'to'all-projects'; stored URLs or state relying on the old value will not match.📊 Macroscope summarized 97598ea. 9 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.