feat(pull-requests): add full PR workspace with overview, files, and conversation views#2810
feat(pull-requests): add full PR workspace with overview, files, and conversation views#2810znoraka wants to merge 6 commits into
Conversation
Introduce a full PR review workflow: list PRs, view diffs per file, read/post review and issue comments, mark files as viewed, submit reviews, merge PRs, and edit PR metadata — all powered by the `gh` CLI. Server: new GitHubCli service, GitHubCliPR/GitManagerPR layers, and dedicated WebSocket RPC handlers (ws-pr.ts). Web: PullRequestListPanel, PullRequestReviewView, CommitControl/Modal, SidebarPRButton, WorkspacePickerModal, notification sounds, and route. Contracts: PR-specific schemas, types, and RPC definitions. Also adds FORK.md documenting the fork isolation strategy. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Wire up useNotificationSounds hook in the root route so that notification sounds play when the user is authenticated. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
# Conflicts: # apps/web/src/components/chat/MessagesTimeline.tsx
Replace plain text rendering in UserMessageBody with ChatMarkdown component, enabling markdown formatting in user messages. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…conversation views Introduce a multi-pane PR workspace replacing the single review view. New components: PullRequestWorkspace, OverviewPanel, FilesPane, ConversationPane, FileTree, InlineComment, MergeButton, ReviewBar, and ReviewSidebar. Add PR detail query, merge mutation, and review submission to React Query hooks. Support checkout and worktree creation from the PR view. Wire up URL-driven view navigation (overview/files/conversation). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
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 Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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:unvouched
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5b13dc3. Configure here.
| collected.push(line); | ||
| } | ||
| } | ||
| return collected.length > 0 ? `${collected.join("\n")}\n` : ""; |
There was a problem hiding this comment.
Renamed PR files show empty diff
Medium Severity
extractFileDiff locates a file section only when the unified diff header is diff --git a/{path} b/{path}. Git rename patches use different a/ and b/ paths, so per-file diff extraction returns empty for renamed files while the file list still shows the new path.
Reviewed by Cursor Bugbot for commit 5b13dc3. Configure here.
| > | ||
| <GitMergeIcon className="size-3.5" aria-hidden="true" /> | ||
| Merge | ||
| </Button> |
There was a problem hiding this comment.
Merge button never mounted in UI
Medium Severity
PullRequestMergeButton is added with merge mutation wiring, but nothing in the pull-request workspace or route imports or renders it, so users cannot merge from the new PR UI despite backend and query support.
Reviewed by Cursor Bugbot for commit 5b13dc3. Configure here.
| ), | ||
| Effect.map((entries) => entries.map(normalizePrListEntry)), | ||
| Effect.catch(() => Effect.succeed([] as ReadonlyArray<GitHubPullRequestListEntry>)), | ||
| ); |
There was a problem hiding this comment.
PR list buckets hide CLI failures
Medium Severity
In listWorkspacePullRequests, each fetchBucket pipeline ends with Effect.catch(() => Effect.succeed([])), so auth can succeed while a failed gh pr list call is shown as an empty bucket instead of surfacing an error.
Reviewed by Cursor Bugbot for commit 5b13dc3. Configure here.
| const nodes = data?.data?.repository?.pullRequest?.files?.nodes ?? []; | ||
| return nodes | ||
| .filter((node) => node.viewerViewedState === "VIEWED") | ||
| .map((node) => node.path); |
There was a problem hiding this comment.
Viewed files capped at one hundred
Low Severity
getPullRequestViewedFiles GraphQL query uses files(first: 100) with no pagination, so viewed state from GitHub is incomplete when a pull request changes more than one hundred files.
Reviewed by Cursor Bugbot for commit 5b13dc3. Configure here.
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| function extractFileDiff(fullDiff: string, filePath: string): string { | ||
| const marker = `diff --git a/${filePath} b/${filePath}`; | ||
| const lines = fullDiff.split("\n"); | ||
| let inSection = false; | ||
| const collected: string[] = []; | ||
| for (const line of lines) { | ||
| if (line.startsWith("diff --git ")) { | ||
| if (inSection) break; | ||
| if (line === marker) { | ||
| inSection = true; | ||
| } | ||
| } | ||
| if (inSection) { | ||
| collected.push(line); | ||
| } | ||
| } | ||
| return collected.length > 0 ? `${collected.join("\n")}\n` : ""; | ||
| } |
There was a problem hiding this comment.
🟡 Medium Layers/GitManagerPR.ts:30
extractFileDiff constructs the diff header marker as diff --git a/${filePath} b/${filePath}, but git diff headers for renamed files use different paths (diff --git a/oldname b/newname). Requesting the diff for a renamed file by either its old or new name returns an empty string even when the file exists in the diff. Consider matching the header with a regex that handles both a/${oldPath} b/${newPath} patterns, or document that this function does not support renamed files.
-function extractFileDiff(fullDiff: string, filePath: string): string {
- const marker = `diff --git a/${filePath} b/${filePath}`;
- const lines = fullDiff.split("\n");
- let inSection = false;
- const collected: string[] = [];
- for (const line of lines) {
- if (line.startsWith("diff --git ")) {
- if (inSection) break;
- if (line === marker) {
- inSection = true;
- }
- }
- if (inSection) {
- collected.push(line);
- }
- }
- return collected.length > 0 ? `${collected.join("\n")}\n` : "";
-}🤖 Copy this AI Prompt to have your agent fix this:
In file apps/server/src/git/Layers/GitManagerPR.ts around lines 30-47:
`extractFileDiff` constructs the diff header marker as `diff --git a/${filePath} b/${filePath}`, but git diff headers for renamed files use different paths (`diff --git a/oldname b/newname`). Requesting the diff for a renamed file by either its old or new name returns an empty string even when the file exists in the diff. Consider matching the header with a regex that handles both `a/${oldPath} b/${newPath}` patterns, or document that this function does not support renamed files.
Evidence trail:
apps/server/src/git/Layers/GitManagerPR.ts lines 30-47 (extractFileDiff implementation, marker construction at line 31), line 172 (caller passing input.filePath). Git diff format for renames: `diff --git a/oldpath b/newpath` is standard git behavior (https://git-scm.com/docs/diff-format).
| [viewedMap, currentHashes], | ||
| ); | ||
|
|
||
| const setViewed = useCallback( |
There was a problem hiding this comment.
🟢 Low lib/pullRequestViewedFiles.ts:145
onSetViewed fires even when setViewedMap returns prev unchanged — for example, when toggling a file off that isn't in the map, or when the hash computation fails. This triggers unnecessary GitHub API mutations via setFileViewedMutation.mutate in the calling component. Consider moving the callback inside the functional updater so it only fires when next !== prev.
🤖 Copy this AI Prompt to have your agent fix this:
In file apps/web/src/lib/pullRequestViewedFiles.ts around line 145:
`onSetViewed` fires even when `setViewedMap` returns `prev` unchanged — for example, when toggling a file off that isn't in the map, or when the hash computation fails. This triggers unnecessary GitHub API mutations via `setFileViewedMutation.mutate` in the calling component. Consider moving the callback inside the functional updater so it only fires when `next !== prev`.
Evidence trail:
apps/web/src/lib/pullRequestViewedFiles.ts lines 145-165 (setViewed function): line 152 returns `prev` unchanged when hash is falsy; lines 156-157 return `prev` when file not in map; line 162 calls `onSetViewed` unconditionally after `setViewedMap`. apps/web/src/components/PullRequestFilesPane.tsx lines 310-315: onSetViewed calls setFileViewedMutation.mutate. apps/web/src/components/PullRequestReviewView.tsx lines 258-263: same pattern, onSetViewed calls setFileViewedMutation.mutate.
ApprovabilityVerdict: Needs human review 1 blocking correctness issue found. Diff is too large for automated approval analysis. A human reviewer should evaluate this PR. You can customize Macroscope's approvability policy. Learn more. |
1 participant
Summary
Test plan
🤖 Generated with Claude Code
Note
High Risk
Exposes merge, review, and comment mutations via
ghon the server RPC path with a large new UI surface; mistakes or auth issues could affect real GitHub PRs.Overview
Adds fork maintenance docs in
FORK.md(isolation under_lempire/, rebase workflow) while this change set still extends coreapps/serverandapps/webpaths rather than_lempire/wrappers.On the server,
GitManagergains a full GitHub PR surface (list, diffs, comments, viewed files, reviews, merge, detail/edit, collaborators) implemented via newGitHubCliPR/GitManagerPRlayers ongh, wired throughws-pr.tsinto the WebSocket RPC layer andGitHubCli.layerinserver.ts; tests now provideGitHubCliin the manager test harness.On the client, introduces a pull-request workspace: list panel, tabbed overview / files / conversation (
PullRequestWorkspace), rich file tree and Pierre-based diffs with GitHub “viewed” sync, conversation and inline/pending review UI, merge and review submission, review sidebar (agent review + checkout/worktree hooks), plus Commit header flow (CommitControl/CommitModal) that sends a structured agent prompt for commit/push/PR.Risk: Operations run through authenticated
ghagainst the user’s machine (merge, comments, GraphQL viewed state)—high impact if mis-invoked, but no new hosted credential store in this diff.Reviewed by Cursor Bugbot for commit 5b13dc3. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add full pull request workspace with overview, files, conversation, and review views
PullRequestWorkspacecomponent with tabbed overview, files, and conversation panes, plus a review sidebar for submitting COMMENT, REQUEST_CHANGES, and APPROVE events.PullRequestFilesPanerenders a filterable file tree with per-file diffs, viewed-file tracking (persisted to localStorage viausePullRequestViewedFiles), and unified/split diff toggle.PullRequestOverviewPanelshows PR metadata, labels, checks, reviewer states, and mergeability;PullRequestConversationPanelists and posts issue/review comments.GitManagerShapeand wires new WebSocket RPC handlers inws-pr.tsfor the full set of PR operations (list, diff, comments, review, merge, etc.), backed by the GitHub CLI viamakeGitManagerPRMethods.PullRequestListPaneland a/pull-requestsroute with search-param persistence, and adds a PR navigation button toSidebarChromeFooter.useNotificationSoundshook that plays audible chimes when chat threads finish or need attention, bootstrapped at the root route.UserMessageBodynow renders viaChatMarkdowninstead of plain text, enabling markdown in user messages.📊 Macroscope summarized 5b13dc3. 35 files reviewed, 8 issues evaluated, 0 issues filtered, 2 comments posted
🗂️ Filtered Issues