feat(web): refresh pull request details#7148

🟡 Medium routes/_chat.pull-requests.tsx:1947

While refreshing is true, repeated clicks still invoke onRefresh, so duplicate invalidations start the list, partitions, stats, and detail refresh cascade concurrently. Disable the Button with disabled={refreshing} or otherwise guard the click, matching the existing empty-state refresh controls.

✅ Resolved in 89fac2a9dc591dd8d696ec484495ed574ac41650

🟡 Medium pullRequest/pullRequestDetail.logic.ts:83

isStackedPullRequestBase treats origin/feature/main as matching base branch main, so distinct branches are reported as non-stacked and the stacked-PR indicator is hidden. The unrestricted endsWith check matches nested branch names; strip only the remote-name prefix before comparing with baseBranch.

✅ Resolved in 89fac2a9dc591dd8d696ec484495ed574ac41650

Repo link lowercases the opened path

Low Severity

changeRequestRepositoryUrl rebuilds the repository path from parseChangeRequestUrl, which lowercases the captured path for identity matching. The opened URL therefore loses the host’s original casing and can 404 on case-sensitive GitLab or similar installs, while the pull request number still uses the original detail.url.

^{Reviewed by Cursor Bugbot for commit 8c3da41600c7ab9da4c3c561ce9057bbdbc48943. Configure here.}

Stacked check over-matches branch suffixes

Low Severity

isStackedPullRequestBase treats any default ref that merely ends with /${baseBranch} as the same branch. That is meant for origin/main versus main, but it also matches unrelated names such as release/1.0 versus 1.0, so a stacked pull request can lose the stacked indicator.

^{Reviewed by Cursor Bugbot for commit 8c3da41600c7ab9da4c3c561ce9057bbdbc48943. Configure here.}

pullRequest/PullRequestDetailPanel.tsx:1641

The tab nav's padding now changes with the chrome state (py-2 → py-1.5), but the scroll compensation below still refunds only foldHeight - condensedRow height. The chrome loses 4px more than that when it collapses, so the content under the reader jumps up ~4px mid-scroll — exactly what the compensation exists to prevent. Keeping the nav height constant is the smallest fix; otherwise this row has to be measured into chromeDelta too.

No longer relevant as of 0437edb65b7dc3483cfa26bb49e9366d6d2e60b0

pullRequest/PullRequestSummaryTab.tsx:199

Trading py-1.5 for min-h-8 drops the row's vertical padding, and both the Reviewers and Labels cells are flex-wrap — once they wrap, the pills sit flush against the neighbouring rows. Keeping py-1.5 alongside min-h-8 preserves the 32px single-line rhythm the ghost mirrors and restores the gap when a row wraps.

No longer relevant as of 0437edb65b7dc3483cfa26bb49e9366d6d2e60b0

pullRequest/PullRequestDetailPanel.tsx:1393

This button replaced the conflict row's only consumer of handoffLabels.resolve, so the shared label contract (pullRequestHandoffLabels, "names where a pull-request task will land, without letting each surface guess independently") now has an unused entry while this surface hardcodes its own copy. Since the click still hands off to a thread, the thread context loses the "in this thread" destination the contract exists to state. Reusing the shared label keeps the two consistent.

No longer relevant as of b45d74ed7e0e01be81d283140f980943dfa14849

🟡 Medium lib/openPullRequestLink.ts:126

changeRequestRepositoryUrl returns https://gitlab.example/group for a valid nested GitLab URL such as .../group/pull/123/repo/-/merge_requests/42, so the repository link opens the namespace page instead of the repository. The provider-agnostic regex matches /pull/123 before reaching GitLab’s /-/merge_requests/42 marker; extract the GitLab repository path first, then fall back to the other provider patterns.

✅ Resolved in 0437edb65b7dc3483cfa26bb49e9366d6d2e60b0

🟡 Medium components/HeaderFilterMenu.tsx:47

Disabled options expose disabledReason only through title, but MenuRadioItem applies pointer-events-none and disabled items cannot receive focus. As a result, users see unavailable options with no way to discover why they are disabled; render the reason in visible/accessibly focusable UI or attach the tooltip to a non-disabled wrapper.

✅ Resolved in 2dd5525de4577b23ff331860af354f54ead0a154

pullRequest/PullRequestDetailPanel.tsx:1159

order-last paints the overflow menu at the end of this action row while it stays first in the DOM, so keyboard focus reaches "More pull request actions" before the Check out, primary action and Collapse controls that are drawn ahead of it — visual order and focus order no longer match. Moving the <Menu> block to the end of this flex row (and dropping order-last) keeps the two in sync without changing the layout.

No longer relevant as of 2dd5525de4577b23ff331860af354f54ead0a154

pullRequest/PullRequestDetailPanel.tsx:1159

order-last paints this trigger at the end of the actions row while it stays first in the DOM, so keyboard focus reaches "More pull request actions" before Check out and the primary action, and assistive tech reads the row in an order that no longer matches what is shown (it would also land after the panel-collapse button when onClose is passed). Moving the whole <Menu> block to after the primary-action button and dropping order-last keeps visual, DOM and focus order the same.

No longer relevant as of 2dd5525de4577b23ff331860af354f54ead0a154

🟠 High routes/_chat.pull-requests.tsx:1648

An unavailable host with unavailable: "" is re-enabled, allowing selection of a source already known to be unreadable. This truthiness check omits disabledReason for empty strings; preserve definedness by checking option.unavailable !== undefined.

✅ Resolved in 2dd5525de4577b23ff331860af354f54ead0a154

pullRequest/PullRequestDetailPanel.tsx:1644

The segmented track treatment (rounded-lg bg-muted/45 p-0.5) is hand-written here, making this the fourth call site carrying the same three classes — UsagePage repeats them verbatim on each of its three segmented ToggleGroups. That is durable geometry for the segmented variant, not contextual layout, so it belongs in the primitive: ToggleGroup already branches on variant === "segmented" for its gap, and the radius, surface, and inset padding can join that branch. Call sites would then pass only variant="segmented" size="segmented" plus any local layout classes, and the track stays consistent if it is ever retuned.

Cross-file fix, so no inline suggestion: add the track classes to the segmented branch in apps/web/src/components/ui/toggle-group.tsx and drop the className here.

No longer relevant as of c4b280b249755ab463d28edee1532e856ae838a5

pullRequest/PullRequestDetailPanel.tsx:1336

ps-[8.5px] overrides the xs size's own inline-start padding at one call site, so the two size="xs" icon-leading buttons in this row no longer share a geometry: the neighbouring Resolve conflicts / Merge buttons keep the primitive's px-[calc(--spacing(2)-1px)]. The primitive already accounts for leading glyphs via [&_svg]:-mx-0.5.

Dropping the override keeps the row uniform; if the optical compensation is genuinely wanted for icon-leading buttons, it belongs in the Button size contract rather than here.

No longer relevant as of 2dd5525de4577b23ff331860af354f54ead0a154

pullRequest/PullRequestDetailPanel.tsx:1642

This repeats the segmented track treatment (rounded-lg bg-muted/45 p-0.5) that UsagePage already sets on each of its three ToggleGroups, so with this call site the same durable geometry is spelled out in four places. ToggleGroup already special-cases variant === "segmented" (for the gap) but owns only the items, not the surface they sit in — a call site that forgets the string gets a visually different segmented control.

Consider moving the track classes into the segmented branch of ToggleGroup and leaving only contextual layout here.

No longer relevant as of 2dd5525de4577b23ff331860af354f54ead0a154

Actions menu wraps unrelated controls

Medium Severity

The overflow actions Menu now wraps the checkout Menu, badges, and primary action buttons, with MenuTrigger after MenuPopup. Those controls used to be siblings in a fragment. Nesting an independent menu and non-menu actions under the overflow root ties them into the same menu tree and can break open/dismiss and focus behavior for checkout and the ⋯ menu.

^{Reviewed by Cursor Bugbot for commit 2dd5525de4577b23ff331860af354f54ead0a154. Configure here.}

Unreachable conflict menu action

Low Severity

The overflow menu still gates Resolve conflicts with conflicting && primaryAction !== "resolve", but open conflicting PRs now always set primaryAction to resolve. That branch can no longer render, so the menu path is dead leftover from the old priority order.

^{Reviewed by Cursor Bugbot for commit ac244453615ce03ede0d2b210249f038c9b980e6. Configure here.}

pullRequest/PullRequestDetailPanel.tsx:1181

Moving the trigger last does fix visual/DOM order, but wrapping the whole actions row in this <Menu> also puts the Check out <Menu> (line 1346) — plus the auto-merge Badge and the primary-action Button — inside the actions menu's React context. Base UI derives submenu behaviour from that context (MenuSub in ui/menu.tsx is just MenuPrimitive.SubmenuRoot, the same root component), so the Check out menu is now treated as a nested menu rather than an independent one: nested menus default to openOnHover and share the parent's floating tree, focus restoration and Escape handling. Nothing else in apps/web/src nests a Menu root outside a popup.

Keeping the overflow menu self-contained — Menu → MenuTrigger → MenuPopup — and rendering that whole block after the primary-action button keeps visual, DOM and focus order aligned without changing menu semantics.

No longer relevant as of 46c5c1ba3bca374dbff131076a47e5d063865899

disabledReason is fed the host's full failure sentence (e.g. "GitHub CLI (gh) is required to browse change requests on this host. Install it from https://cli.github.com/ and reload."), and this span has no width cap, so one unreadable host stretches the popup far past its min-w-40. The pill/menu row for the same option data keeps that string inside a max-w-80 tooltip (PullRequestListFilters.tsx:179-184), so the two treatments of one string also diverge. Capping the width lets the reason wrap inside the item instead of widening the menu.

^{Posted via Macroscope — UI Consistency}

Removed from this PR when the shared filter control moved down to #7147 with its first consumer.

🟡 Medium routes/_chat.pull-requests.tsx:1197

Closing the panel clears the linked selection, so rightPanelAvailable becomes false even when selectedPullRequestSurface still contains a retained tab. The layout toggle is then disabled and the user cannot reopen the panel; base availability on the retained selected surface instead.

✅ Resolved in 46c5c1ba3bca374dbff131076a47e5d063865899

chat/PanelLayoutControls.tsx:99

This label can't be reached, so the route's "Select a pull request first" never renders: the tooltip trigger is the Toggle, and while it is disabled toggleVariants' base disabled:pointer-events-none (plus the native disabled attribute Base UI puts on it) keeps hover and focus from ever opening the popup. The branch only runs when rightPanelAvailable is false, i.e. exactly when the control is disabled.

The established pattern here for explaining a disabled control is to hang the trigger on a wrapper element rather than on the disabled control — see GitActionsControl.tsx:1768-1785, where a disabled MenuItem sits inside a <span> trigger, and the menu rows that use data-disabled:pointer-events-auto. Rendering TooltipTrigger as a wrapping <span className="flex shrink-0" /> around the Toggle (leaving the Toggle disabled) keeps the disabled semantics and makes the reason visible.

No longer relevant as of 7d39c806200f28ca98850d29da23777149fed7cf

pullRequest/PullRequestDetailPanel.tsx:1284

This size="xs" button pins its glyph to size-3 while the Check out button now beside it in the same action row uses size-3.5, so two peers of the same size variant show differently sized icons. Matching the neighbour (or dropping the override entirely and letting the xs size contract pick size-4/sm:size-3.5) keeps the row uniform.

No longer relevant as of 696a2e10e7ace22568c0c6b8a62d571ab5854f69

pullRequest/PullRequestDetailPanel.tsx:1673

The rewritten copy-branch button keeps outline-none but drops focus-visible:ring-offset-1 focus-visible:ring-offset-background, so its keyboard focus ring now paints flush against the branch text, while every control around it — Button, Toggle, and this button's previous class string — draws the same ring with a background-coloured offset. Restoring the offset pair keeps one focus treatment across the row.

No longer relevant as of 696a2e10e7ace22568c0c6b8a62d571ab5854f69

chat/PanelLayoutControls.tsx:65

This wrapper is the right fix for a tooltip on a disabled control, but it is now the only one: the terminal control just above (lines 41–62) still renders its trigger as the Toggle, so while terminalAvailable is false, toggleVariants' base disabled:pointer-events-none plus the native disabled attribute keep hover and focus from ever opening "Terminal drawer is unavailable" — exactly the case that branch exists for. Two sibling controls in one component with the same need and two different compositions.

Consider hanging the terminal trigger on the same wrapper so both unavailable states explain themselves identically:

The new test could then cover both triggers with the same assertion.

No longer relevant as of a7586658ecd31f5b053350cdbcfd8d2d2f2ffcf5

pullRequest/PullRequestDetailPanel.tsx:1560

Removing the eased track makes reopening the chrome snap. grid-rows-[0fr]/[1fr] now flips with no transition-[grid-template-rows], so when the reader scrolls back to top < 4 the fold's full height reappears in one frame and the content area below jumps down by it. Collapsing does have to stay instant — the scrollTop compensation refunds foldHeight - condensedRow and would fight an animated track — but the reopen path deliberately gets no refund, which is why that direction used to ease.

Restoring the transition on the expanding branch (and the mirrored one on the condensed row's track at line 1479, so the two rows still trade height in one motion) keeps the geometry it had before.

No longer relevant as of e942ef87286e84e130ba58d6ad5685ade4e2f5b6

pullRequest/PullRequestDetailPanel.tsx:436

The collapse machinery survived this restructure unchanged, but its explanations did not: chromeStateByTab and the compensationRef layout effect (lines 432-447), the foldHeight - condensedRow refund (line 1856), and the deliberately one-directional track animation (grid-rows-[0fr] with no transition on collapse, eased only on reopen — lines 1570-1572) are all still here with nothing left saying why.

The asymmetric track is the risky one: it now reads as an oversight, so the next reader is likely to "fix" it by letting collapse animate too, which puts the animated track back in a fight with the scrollTop write and reintroduces the mid-scroll content jump the refund exists to prevent. Since the mechanism itself did not change, consider restoring the two notes that cover it — why collapse is instant while reopening eases, and why the refund is the fold's height minus the condensed row's.

No longer relevant as of 6fe8428ca433ad6d4151f6757be25b190638c33e

pullRequest/PullRequestDetailPanel.tsx:436

Two of the four rationales dropped from the collapse machinery are back (the one-way track at 1567-1574 and the refund at 1858), but the two here are still gone while the code they explained is unchanged:

• chromeStateByTab (431-435): nothing left says why chrome state is remembered per tab — only the active tab emits scroll, and a tab too short to scroll has to remember "expanded" or it is stranded under a chrome it has no scrollbar to reopen.
• the compensationRef layout effect (440-447): nothing says the scrollTop write exists so the content under the reader does not leap when the fold's height returns to the scrollport.

Both are non-obvious scroll-compensation constraints, so a line of why above each would keep the next reader from taking them for removable state.

No longer relevant as of 5ea3cfaeeae3981a36e3a757ac6e71c8789c7e77

pullRequest/PullRequestDetailPanel.tsx:1517

The condensed chrome shows the author as a bare avatar: PullRequestActorAvatar renders aria-hidden markup, so the login only exists in the sr-only span and a sighted reader has no way to find out who it is. This surface's convention is the opposite — CommentAuthor in PullRequestSummaryTab ("The avatar carries the attribution alone; who it is arrives on hover") and PullRequestActorLabel both hang a tooltip off the avatar, and the expanded fold below uses the labelled form.

Wrapping the avatar in the same tooltip trigger keeps one attribution treatment and drops the need for the separate sr-only node.

No longer relevant as of 1088b8114cca06cba3fc0b18e926a5a7582c3195

Fixed in 1088b8114. The condensed author avatar now uses the shared tooltip treatment with an accessible login label.

pullRequest/PullRequestDetailPanel.tsx:1296

This revision deletes the overflow-menu fallback that used handoffLabels.resolveConflicts, so with the conflict row already gone pullRequestHandoffLabels has two entries (resolve, resolveConflicts) that nothing renders, while this button hardcodes its own copy. The contract exists to name where a hand-off lands "without letting each surface guess independently", and the sibling hand-offs in this same panel (fixFindings in the menu, fixFinding/fixCheck passed to the tabs) still read from it — so in thread context this is now the one hand-off that does not say the work stays in the open thread.

Smallest fix either way: render handoffLabels.resolveConflicts here, or drop the two entries (and their assertions in pullRequestDetail.logic.test.ts) if the short copy is deliberate for this compact row, so the contract stops advertising labels no surface uses.

No longer relevant as of f1c8120882a1c7ad951abf7f8e9d12bb06ac1f2e

Fixed on the latest head by removing the dead resolve handoff labels and keeping the compact visible copy.

pullRequest/PullRequestDetailPanel.tsx:1534

The new LayersIcon is the only place "stacked pull request" is said, and it is said on a bare <svg> inside a tooltip trigger whose popup repeats just the branch name — so a sighted reader gets an unexplained glyph, and the label sits on an element with no role rather than on the trigger. Its neighbour in this same row (PullRequestBaseFreshnessWarning) does the opposite: the label is on the interactive element and the meaning is spelled out in the layer it opens.

Folding the meaning into the tooltip that is already here is the smallest fix (the expanded copy of this row at ~1683 needs the same change).

No longer relevant as of f1c8120882a1c7ad951abf7f8e9d12bb06ac1f2e

Fixed on the latest head. The tooltip now explains the glyph as “Stacked on ”.

pullRequest/PullRequestDetailPanel.tsx:1120

Both crossfade layers of the top row dropped sm:text-xs while keeping text-sm, so the repository/number line (and the condensed title layer, line 1176, which carries the identical string) now renders at 14px on desktop inside a row that shrank from h-11 to h-7 — while every other line of this chrome (the condensed meta row, the branch pair, the tab bar) stays 12px. text-sm … sm:text-xs is the coarse-pointer/desktop pairing the control primitives themselves use (Button xs, Select xs, Badge), so the desktop step is worth keeping here too.

The same addition applies to the condensed layer at line 1176.

No longer relevant as of 1088b8114cca06cba3fc0b18e926a5a7582c3195

Fixed in 1088b8114. Both crossfade layers keep sm:text-xs so the compact desktop header stays at the intended scale.