feat(web): add a pull requests page - #4849
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.
The project filter and the selected pull request shared one search field, so opening a row in a cross-project list silently narrowed the list to that project, and switching tabs or closing the panel discarded a filter the user had chosen. The selection now carries its own field.
- The findings prompt interpolated the PR title as a directive. Title, URL and branch names are quoted now and the untrusted-data warning covers them, not just the findings below it. - The viewer login tried only the first project's workspace, so one broken checkout blanked every healthy repository. It falls through to the next. - A check completing with ACTION_REQUIRED read as neutral, so a blocking check could be summarised as passing. - Comment bodies are no longer trimmed by the contract: they are markdown, where leading spaces open a code block and two trailing spaces are a line break. The service rejects a whitespace-only body instead. - The comment box locks while posting; clearing it on success used to discard a draft typed during the request. - A failed refresh kept the rows already on screen and says so, rather than replacing the list with an error. - The retained page is keyed by environment, so switching environments cannot show the previous one's pull requests. - An info-string fence (```ts) inside an open fence no longer closes it, and an indented code line stays markdown even when it is only a video link.
- An unclosed <video> tag made the scanner walk the rest of the body, and every later unclosed tag rescanned the same text, so a crafted body could stall the parser. The lookahead is bounded to eight lines and no longer joins the text until a closing tag is actually found. - listReviewThreadComments split a host-qualified selector from the front, sending owner=host and name=owner to GraphQL and dropping the repository. It now reads owner and name off the end and passes the host as --hostname. - The project filter listed projects from every environment while the query only ever reads one, so a remote project could be selected and then fail. The list is scoped to the active environment.
e1fca58 to
021451b
Compare
- A failed page left the infinite-scroll observer armed, and the retained rows kept the sentinel on screen, so it asked for the next page again without bound — an unbounded run of gh invocations. The observer now stands down on error, and the warning offers a retry. - The findings prompt kept the oldest entries when it had to cut. Comments arrive oldest-first and failing checks are appended after them, so the bound dropped every current failure. It is taken from the end now. - A review with a state but no body is an approval or a change request, and nothing else surfaces those, so they are kept in the conversation instead of being discarded as empty.
There was a problem hiding this comment.
One residual convention issue from the previous pass: the readError alias in GitHubPullRequestCli.ts. The operation misreporting and the synthetic viewer-login cause are both resolved.
Posted via Macroscope — Effect Service Conventions
The helper had shrunk to a construction alias with a constant command, so the four decode sites now build GitHubPullRequestReadError themselves, which also names each operation at the point it fails.
Adds a Pull requests page: a list of your projects' GitHub pull requests, with a detail panel beside it for reading and acting on one without leaving T3 Code.
Reachable from the sidebar, and any GitHub pull request link elsewhere in the app (thread rows, sidebar, branch toolbar) now lands here instead of the browser — the browser is one of the page's actions rather than the only option.
What's in it
List — All / Reviewing / Authored crossed with Open / Closed / Merged, free-text search, and a project filter. The server returns the state's involvement superset, so switching involvement tabs never waits on the network. Rows group by your relationship to the pull request.
Detail — three tabs beside the list, resizable and persisted like the preview panel:
Actions — merge (with the repository's allowed methods), ready, draft, close, reopen, comment, copy link, open on GitHub. Fix findings and Resolve conflicts check the pull request out into its own worktree, open a thread there, and hand over a task-specific prompt.
Notes for review
ghCLI wrapper, so there is no new auth story. Projects on other providers are simply not listed, and a missing or logged-outghrenders a state that names the fix rather than a generic failure.ghover stdin rather than argv; the repository sent by the client is checked against the project's own remote before reachinggh --repo; embedded videos only render forhttp(s)sources.errorsso healthy repositories still render. Malformed rows are skipped instead of blanking a batch. Enum-ishghfields decode as strings and are normalized, so aghrelease that adds a check conclusion cannot fail a payload.gh pr listexposes no cursor, so loading more re-reads a larger page rather than continuing from an offset. Cheap at the sizes a pull request list reaches, and the CLI pages internally. The last page stays on screen while the larger one arrives.Known gaps
Deliberately left out, happy to follow up:
useNewThreadHandler.Verification
Rebased on latest
main. Repo-wide typecheck clean, lint clean, full test suite passing, plus 44 tests: the pull request service (project selection, repository de-duplication, partial failure, CLI-missing, ordering, review-request attribution, cross-project rejection), theghJSON decoding (including fail-closed merge settings and raw-row truncation), list and involvement logic, timeline and prompt bounding, and body segmentation. Driven manually in the browser: list, filters, search, project scope, infinite scroll, detail tabs, diff scrolling, and the actions menu.All 13 Macroscope findings are addressed and answered in-thread.