fix(pi): full PR review parity with Bun server#503
Merged
backnotprop merged 8 commits intomainfrom Apr 7, 2026
Merged
Conversation
The Pi extension's plannotator-review command ignored PR URL arguments entirely, always falling back to local git diffs. This brings Pi to full parity with the Bun hook implementation across PR review, agent jobs, server lifecycle, and all review endpoints. PR review flow: - Parse PR/MR URLs, authenticate, fetch PR metadata and diff - Create local worktree (same-repo) or shallow clone (cross-repo) for agent file access, with cleanup on server stop - Pass prMetadata, agentCwd, onCleanup to review server - Forward prUrl through event payload and command handler Server parity fixes: - Add Codex P0/P1 blocking findings override to onJobComplete - AI endpoints getCwd now respects agentCwd for PR worktrees - /api/pr-action: add pre-submission, success, failure logging - /api/pr-viewed: add error logging - /api/diff: add isWSL field, use hasLocalAccess guard for diffType/gitContext conditionals (not isPRMode) - /api/diff/switch: guard on hasLocalAccess with aligned error message - /api/git-add: add canStageFiles check for unsupported diff types - /api/file-content: reorder to local-first priority, add "no file access available" guard - Add isRemote to ReviewServerResult return structure - Add onReady callback option with (url, isRemote, port) signature - Align all error messages to match Bun server text For provenance purposes, this commit was AI assisted.
hasLocalAccess must be !!gitContext only (not agentCwd) — it gates UI-facing features like diff switching and file content routing. Agent-facing access uses hasAgentLocalAccess (agentCwd || gitContext) inside buildCommand only. Without this fix, PR mode with a worktree would incorrectly allow diff switching and return stale diffType. For provenance purposes, this commit was AI assisted.
1. Exit handler missing cwd: worktree remove now passes cwd: repoDir so git can locate the parent repo on abnormal exit. 2. Exit listener accumulation: store handler ref, remove it in worktreeCleanup on normal stop. Prevents MaxListenersExceeded warnings after multiple PR reviews in a single Pi session. 3. Cross-repo clone --hostname flag: gh/glab repo clone doesn't accept --hostname. Use GH_HOST/GITLAB_HOST env var instead, fixing self-hosted GHE/GitLab cross-repo PR reviews. 4. AI endpoints getCwd duplication: replace verbatim copy of resolveAgentCwd closure with direct function reference. 5. Worktree staging broken: canStage check now parses composite "worktree:/path:uncommitted" diff types via parseWorktreeDiffType to extract the base subType before checking stageability. For provenance purposes, this commit was AI assisted.
…eanup
1. Share reviewRuntime: export from serverReview.ts, import in
plannotator-browser.ts. Remove duplicate nodeGitRuntime and
runSync helper — all git calls now use shared runtime.
2. Fix parseRemoteUrl for GitLab subgroups: capture full multi-segment
paths (group/subgroup/project) in SSH, HTTPS, and SSH+port formats.
Fixes same-repo detection for nested GitLab groups (shared fix,
affects both Bun and Pi).
3. Increase cross-repo shallow fetch depth from 50 to 200 commits
in both Bun hook and Pi. Reduces "no merge base" failures for
long-lived branches during agent reviews.
4. Fix Bun hook --hostname flag on gh/glab repo clone: replace with
GH_HOST/GITLAB_HOST env vars (repo clone doesn't accept --hostname).
Pi already had this fix from previous round.
5. Deregister exit handler in catch block: prevents dangling
process.once("exit") listeners when worktree creation fails
partway through.
For provenance purposes, this commit was AI assisted.
1. Fix parseRemoteUrl SSH regex matching HTTPS URLs with non-standard ports (e.g. https://gitlab.example.com:8443/...). Guard the SSH regex with !url.includes("://") so it only fires on actual SSH URLs, letting HTTPS+port fall through to the HTTPS regex. 2. Fix Pi PR approve sending "please address this feedback" to agent. Platform PR actions (approve/comment) return approved:false with a status message. Add isPRReview guard to skip the "please address" suffix, matching the Bun hook's if (!isPRMode) pattern. 3. Disable Git Add button in PR review mode. The server sends diffType: undefined in PR mode, but the client defaults to 'uncommitted' which enables staging. Override canStageFiles to false when prMetadata is present. Fixes both Bun and Pi servers. For provenance purposes, this commit was AI assisted.
1. Fix parseRemoteUrl SSH regex matching HTTPS URLs with non-standard ports (e.g. https://gitlab.example.com:8443/...). Guard SSH regex with !url.includes("://") so it only fires on actual SSH URLs. 2. Fix Pi PR approve sending "please address this feedback" to agent. Platform PR actions return approved:false with a status message — add isPRReview guard to skip the instruction suffix. 3. Disable Git Add button in PR review mode. Override canStageFiles to false when prMetadata is present. Fixes broken staging button for both Bun and Pi servers. 4. Add Resolved/Outdated toggle filters to PR comments tab. Click to hide resolved or outdated review threads. Uses same color tokens as the existing badges. Resets with Clear Filters. For provenance purposes, this commit was AI assisted.
Change [^:]+? (one-or-more) to [^:]*? (zero-or-more) so that git@host:x.git correctly captures 'x' instead of 'x.git'. Without this, same-repo detection fails for short path segments. For provenance purposes, this commit was AI assisted.
DiffViewer initialized pierreTheme with { type: 'dark', css: '' },
then computed the real theme asynchronously via requestAnimationFrame.
This caused a single frame of the diff library's default dark theme
(black background) before the actual theme CSS was applied.
Fix: compute initial bg/fg CSS synchronously in useState initializer
by reading --background/--foreground from computed styles. The full
theme (muted, primary, fonts) still applies via the existing useEffect
on the next frame, but the critical background color is correct from
the first render.
For provenance purposes, this commit was AI assisted.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
plannotator-reviewcommand ignored PR URL arguments, always falling back to local git diffsparseRemoteUrlfor GitLab subgroups, shallow clone depth,--hostnameflag onrepo clone, Git Add button in PR modeChanges
PR Review Flow (new)
plannotator-browser.ts: Full PR mode with worktree/clone support matchingapps/hook/server/index.tsindex.ts: Command handler forwards PR URL argument, handles platform PR action feedback correctlyplannotator-events.ts: Event payload includesprUrlfieldServer Parity Fixes (
serverReview.ts)getCwdrespectsagentCwdfor PR worktrees (uses sharedresolveAgentCwd)/api/pr-action+/api/pr-viewed: diagnostic logging/api/diff:isWSLfield,hasLocalAccessguard (was usingisPRMode)/api/diff/switch: guard onhasLocalAccess(wasisPRMode)/api/git-add:canStageFilescheck with worktree composite diff type support/api/file-content: local-first priority, "no file access" guardisRemotein return structure,onReadycallbackreviewRuntime— shared by browser and server, no duplicationShared Fixes (both Bun and Pi)
parseRemoteUrl: full multi-segment GitLab path support, SSH+port handling, HTTPS non-standard port fix--hostnameflag replaced withGH_HOST/GITLAB_HOSTenv vars (repo clonedoesn't accept--hostname)canStageFilesdisabled in PR review mode (client-side fix inApp.tsx)PR Comments Tab
Test plan
tsc --noEmit)plannotator-review <PR-URL>in Pi opens PR diff (not local diff)plannotator-review(no URL) still works as before