feat: multi-provider pull requests page with in-app reviews - #4849
feat: multi-provider pull requests page with in-app reviews#4849Bil0000 wants to merge 177 commits into
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 |
There was a problem hiding this comment.
Reviewed the new Effect services (GitHubPullRequestCli, PullRequestService), the new contract errors, and the touched call sites. Module layout, namespace imports, Context.Service + inline interface, make/layer, and dependency acquisition via yield* Foo.Foo all follow the conventions. The findings below are all in error modelling.
Posted via Macroscope — Effect Service Conventions
fc40c4f to
8912648
Compare
8912648 to
4fcb7b9
Compare
4fcb7b9 to
e1fca58
Compare
Models a GitHub pull request for the upcoming pull requests page: list entries, detail with checks/comments/commits, diff, and the merge, ready, draft, close, reopen and comment operations. Two error shapes rather than one: PullRequestUnavailableError covers the states that switch the whole feature off (no gh, logged out, non-GitHub remote) so the UI can explain the fix, while PullRequestOperationError carries per-request failures.
Normalizes the gh payloads into the contract shapes. Enum-ish fields decode as plain strings and are mapped here, so a gh release that adds a check conclusion or review state cannot fail a whole payload, and a malformed row is skipped rather than blanking the batch. Also folds reviews into the comment list (dropping bodyless approvals, which the review decision already reports) and reads unresolved review threads, which gh pr view cannot return.
Wraps the gh invocations the page needs, reusing the existing process wrapper rather than adding a second one. Two of them needed capabilities GitHubCli.execute did not expose: - comment bodies travel over stdin, because argv is visible in process listings and is echoed back inside process-runner failure messages - pr diff raises the output cap to 8 MiB and reports truncation instead of failing on a large pull request Review threads go through gh api graphql, since gh pr view --json has no field for them.
Resolves each project to its GitHub repository through the repository identity already stored on the project, so no extra remote lookup is needed, and lists repositories once even when several worktrees share one. Failure handling is deliberately split: an unreachable repository becomes an entry in the result's errors so healthy repositories still render, while a missing or logged-out gh stops the whole listing, because it is not repository-specific. The repository travels through the client, so it is checked against the project's own remote before reaching gh --repo.
Reads reuse a recent result and refresh explicitly, since every one of them shells out to the GitHub CLI. Mutations run serially per environment: gh actions on the same pull request are order-sensitive and the detail view refetches after each one.
One place resolves how a pull request state reads, so no surface can drift: draft outranks conflicts, because a draft is not heading for a merge yet. Involvement filtering and grouping run over the state's superset returned by the server, so switching between All, Reviewing and Authored never waits on the network. The unavailable state reads the server's message to name the fix — install gh, sign in — rather than reporting a generic failure.
The markdown renderer has no element for a video, so an embed would show up as a bare link. Splits a body into markdown runs and the two video shapes GitHub itself produces: a video or source tag, and a bare link on its own line to a video file or an uploaded attachment. Three cases stay markdown on purpose: anything inside fenced code, an image drop written as an image, and a source that is not http(s).
Summary, Timeline and the gh-backed actions: merge with the repository's allowed methods, ready, draft, close, reopen, comment, copy link, open on GitHub. Merge and close confirm first. Fix findings and Resolve conflicts hand the work to a thread: both check the pull request out into its own worktree, open a thread there, and hand over a task-specific prompt. Everything quoted into those prompts is bounded and marked untrusted, since review bodies and check output are attacker controlled on a public repository. The timeline reports a merge rather than the close GitHub records alongside it, which would otherwise misstate what happened.
Renders the patch through the same viewer as the thread diff panel, and lazily, so its worker pool only ships once the tab is opened. The viewer renders at its natural height and expects its host element to scroll, which is the contract the diff panel already relies on. Deliberately not the annotatable wrapper: that one writes review comments into a thread's composer draft, and this page has no thread. A patch the viewer cannot structure falls back to raw text rather than an empty tab.
Reachable from the sidebar. Filters live in the URL so a view can be shared, while the page size stays local: a shared link should open the first page. Loading further results holds the last page on screen while the larger one arrives, so the list grows underneath instead of falling back to skeletons, and a sentinel starts the next page before it scrolls into view. gh pr list exposes no cursor, so this re-reads a larger page rather than continuing from an offset — cheap at the sizes a pull request list reaches.
A pull request link in a thread, the sidebar or the branch toolbar now lands on the in-app page, which offers the browser as one of its actions. The page resolves the owning project from the repository, so the link only needs the repository and number and no call site has to change. Anything that is not a GitHub pull request URL — a GitLab merge request, an unrelated host — still goes straight out to the system browser.
Locks in the behaviour that is easy to regress and invisible from the UI: non-GitHub projects are skipped, worktrees sharing a repository read it once, one unreachable repository leaves the healthy ones listed while a missing CLI stops the whole listing, entries order by most recent update, a review request counts for the viewer but not on their own pull request, and a repository that does not belong to the project never reaches gh. Also names the conversation page size the truncation flag was comparing against.
A closed pull request was grey here and red in the thread badge, so one pull request read as two different things in two places. The page now uses the same ink for open, closed and merged; draft and conflicts are states the badge never shows. The meta line was reading children with Array.isArray, which is wrong for a single child or a fragment. Children.toArray handles both, drops the nullish segments, and lets a separator borrow the key of the segment it precedes.
The method is a preference for the merge action, not five separate actions, and the project filter in the same feature already expresses that with a radio group. Replaces the hand-written "(selected)" suffix.
Addresses the review of the server error model and gh decoding: - PullRequestUnavailableError derives its message from `reason` instead of copying the CLI's text, and now carries the wrapped failure as `cause`, which the cli-missing and cli-unauthenticated branches were discarding. - An unusable gh response reports the read it came from. The shared decode helper reused one tag whose message was hard-coded to getPullRequest, so every read misreported itself. - An empty viewer login gets its own error rather than a JSON decode tag with a synthetic cause; nothing failed underneath it. - Repository merge settings are required, not optional-defaulting-to-true. They are requested together, so a partial response now fails instead of offering a merge method the repository forbids. - Team review requests no longer enter reviewRequestLogins. The viewer check compares those against a login, so a team slug could read as the viewer. - Truncation is measured on the raw row count. It was read after tolerant decoding, so one malformed row could end pagination early.
The state was substring-matching the error text to recover which of the three reasons it was looking at. The server now derives a stable sentence from the reason itself, so this renders it.
Three body-segmentation bugs from review: - A video tag on a line with text around it replaced the whole line, so the prose disappeared. Only a tag that owns its line is an embed now. - A fence closed on any marker, so a ~~~ line ended a ``` block and exposed its contents to the video rules. The opening marker is tracked instead. - Runs were trimmed on both ends, which drops the four leading spaces that open an indented code block. Only blank lines around a run go now.
The filter only matched whole reviews, so review-thread roots — the inline feedback the detail service labels review-comment — never reached the prompt, which could then report no findings on a pull request full of them. Each finding now names its file, which is the point of an inline comment.
The clipboard write was unawaited for failure, so a denied clipboard threw past the success toast and left the handoff button stuck — with the worktree and the thread already created. The two outcomes are now reported apart.
…first one Where a host has no cursor to continue from, asking for more means asking for a longer page — and the answer was thrown away. With nothing typed the page read the baseline, which asks for one page and always will, so a list grown to two hundred rows was rebuilt from the first ninety-nine every time and those hosts could never show more than one page. The baseline is what a first page and a cleared search are read from. Past that, the list's own answer is the one that has the rows.
Co-authored-by: maria-rcks <254055478+maria-rcks@users.noreply.github.com>
# Conflicts: # apps/web/src/components/ChatView.tsx # apps/web/src/components/RightPanelTabs.tsx # apps/web/src/rightPanelStore.ts
…in a reused draft
…st continuation slice
# Conflicts: # apps/web/src/components/ChatView.tsx # apps/web/src/components/DiffPanel.tsx # apps/web/src/components/RightPanelTabs.tsx # apps/web/src/rightPanelStore.test.ts # apps/web/src/rightPanelStore.ts
- Add consistent, tab-specific loading ghosts - Preserve opened tabs and skip offscreen list content - Memoize pull request rows
The stats endpoint arrived reading the host's search API — the scarcest rate limit of them all — once per client. It now sits behind the same cache as the listing it decorates: refs sorted so one page of rows is one key however the client assembled them, the listings epoch in the key so the refresh that forgets the listing forgets its decorations, failures never cached. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s known to lack it
There was a problem hiding this comment.
One convention violation found in the pull request's Effect service code.
Posted via Macroscope — Effect Service Conventions
…t3code into feat/pull-requests-page
Adds a dedicated Pull Requests page: browse, read, review, and act on change requests from every host a workspace uses, without leaving T3 Code.
Summary
A sidebar entry opens a full page listing pull requests across all projects in the environment, with a detail panel beside it for reading, reviewing and acting on one. Four hosts are supported — GitHub, GitLab, Bitbucket and Azure DevOps — behind a single provider interface.
Each host runs through a wrapper that already existed in this repo (
gh,glab, the Bitbucket HTTP client,az), so there is no new authentication story and nothing new to configure.Design
One interface per host.
PullRequestProviderApidefines the operations; the service knows only the registry, so no adapter knows another exists. Adding a host is one file.Capabilities are declared, not assumed. Each provider states what it supports — patch, comment, actions, merge strategies, review verdicts — and the surface follows it. Nothing renders a control that would fail: Azure DevOps has no Code tab because
azexposes no patch, GitLab offers no Request changes because GitLab has no such verdict, and Bitbucket offers no Reopen because Bitbucket has no such endpoint. The server refuses an undeclared operation independently of the UI.A review is one request. The summary, every line comment and the verdict travel together, so a half-written review is invisible to everyone else. GitHub takes that natively; GitLab and Bitbucket have no pending review, so the provider replays it as the requests it is made of, with the verdict last — a review that fails part-way is never an approval.
Hosts, not provider kinds. github.com and a GitHub Enterprise install are the same kind and different accounts, so the viewer, de-duplication and every row key are scoped by host.
Degrade, never blank. One unreachable repository becomes an entry in
errorswhile the rest render. A malformed row is skipped rather than failing its batch. Enum-like fields decode as strings and are normalised, so a host adding a new status cannot break a payload.Scope
What each host allows
Azure DevOps exposes no patch through
az, so it has no lines to write against.Testing
Repo-wide typecheck and lint clean. ~450 tests cover the service, the four decoders, and the invocations each wrapper makes — including the exact request body a review submission produces on each host.
Response shapes for GitHub, GitLab and Bitbucket were verified against the live APIs rather than documentation, which is what caught Bitbucket's page ceiling of 50 (above it the API returns an empty page and no error), its redirect-served diff endpoints, and the absence of a reopen endpoint in its published OpenAPI spec. GitHub's review-submission payload was verified against the live endpoint using a pending review, which was then deleted. Azure DevOps refuses anonymous REST, so its decoding follows the schema already in this repo and stays tolerant of missing fields.
Not exercised end to end: Bitbucket and Azure DevOps write operations, and GitLab's discussion endpoints. No credentials for any of them were available while building; reads are shape-verified where the API allows it and writes are unit-tested at the invocation level only.
Notes for reviewers
T3CODE_BITBUCKET_EMAIL+T3CODE_BITBUCKET_API_TOKEN), which are pre-existing and server-wide rather than per user.Note
Add multi-provider pull requests page with in-app review tabs
/pull-requestsroute with list, filtering (state/involvement/host/project), and a detail panel with Summary, Timeline, and Code tabs supporting inline review threads, pending comments, and review submission.useOpenChangeRequestLinkhook.isOnPullRequestHeadis returned to callers.'pull-request'are re-keyed by reference on load.Macroscope summarized c035a0d.