fix(mobile): Catch up the thread feed after a long background - #6642

💭 I need to compare against the prior run's commit to see if the ChatMarkdown findings were addressed and check for new in-scope changes, so I'm pulling the diff stat for apps/web/src.

1. git_tools — apps/web/src/** d35c71d..088b2ab

2. git_tools — 5755e9e..088b2ab

💭 This diff is huge, and I see the prior run already left four comments on ChatMarkdown.tsx marked as resolved. Since the referenced commit may not be reachable due to a force push on the v2.1 branch, I need to check the current state of ChatMarkdown.tsx directly to see if the flagged regressions around blockquote/ordered-list renderers still exist. There's also a substantial new thread details panel in this PR, so I'll need to review the web UI changes for consistency once I've narrowed down the core file review.

3. git_tools — hastHasText|openChangeRequestLink|GITHUB_ALERT_PRESENTATIONS|orderedListGutterStyle|canOpenInPreview|blockquote(|ol({

4. browse_code — apps/web/src/components/chat/threadDetailsPanel*

💭 Good, all four prior findings look addressed and threads are resolved. Now I'm shifting focus to the new web UI changes in this run—the thread details panel, its styles, ThreadDetailsPrRow, the BranchToolbar panel mode, plus environment routing rules and MenuPopup. Since the prior review commit was force-pushed away, I don't have a clean delta to compare against, so I'll be careful not to duplicate already-fixed ChatMarkdown findings while reviewing the current in-scope changes. Let me pull a stat-only list of changed files under apps/web/src to see what's actually new.

5. git_tools — apps/web/src/** d35c71d..088b2ab

💭 The stat_only option didn't actually limit the output, so I'll try listing PR files directly through the API instead, paginating through them to spot the in-scope files rather than parsing the entire 37k-line diff. I want to zero in on the files behind the new thread details panel UI since that's the key new feature.

6. browse_code — apps/web/src/components/chat/ThreadDetails*

7. browse_code — apps/web/src/components/chat/threadDetailsPanelStyles.ts

💭 These large call-site class strings are overriding the primitive's core height, radius, padding, and base colors, which the rule flags directly. They've built a shared style module instead of a proper primitive variant—a reasonable compromise, but the check really wants a named primitive size/variant instead. Now I'm noticing THREAD_DETAILS_PANEL_ROW_CLASS also uses !bg important overrides on hover for a Button primitive, which strengthens the case for a finding recommending a proper variant rather than important overrides. I want to check ThreadDetailsPanel.tsx and ThreadDetailsPrRow.tsx next for concrete issues around routing, accessibility, and ScrollArea usage.

8. browse_code — apps/web/src/components/chat/ThreadDetailsPanel.tsx

💭 I notice the onClose prop in ThreadDetailsPanel appears unused, though that seems like a minor dead-prop issue rather than a core UI consistency finding, so I'll set it aside. Now I want to check ThreadDetailsPrRow.tsx for its environment-scoped actions like merge and open, to verify it follows the environment routing rules.

9. browse_code — apps/web/src/components/chat/ThreadDetailsPrRow.tsx

💭 This is well-constructed, with environmentId explicitly threaded through for per-environment routing. I'm now checking the other panel-mode changes—OpenInPicker, ProjectScriptsControl, GitActionsControl—to verify environment scoping and the activeThreadRef usage, plus reviewing how environment routing rules apply to shared renderers like ChatMarkdown. There was a change in ChatMarkdown involving how it checks whether markdown file shell actions can be used based on an environment reference. I want to trace ChatMarkdown's fallback logic for resolving the active environment, since that rule shows up prominently in the config, hinting at past bugs. Let me grep for activeEnvironmentId usage there.

10. git_tools — activeEnvironment|useActiveEnvironment|environmentId

💭 Good, the environmentId resolution correctly falls back through threadRef then explicit prop with no active-environment fallback. Now I need to trace the other usages—the asset URL logic and the MarkdownAssetImage component—to check whether new ChatMarkdown call sites in this PR trigger environment actions without an explicit scope.

11. browse_code — apps/web/src/components/ChatMarkdown.tsx

12. browse_code — apps/web/src/components/ChatMarkdown.tsx

💭 Given the diff is huge, I want to narrow focus to the newly changed files that matter: the OpenInPicker, ProjectScriptsControl, and GitActionsControl panel modes, plus ThreadAutomationsPanel, ThreadRelationshipsControl, and any CSS updates in the web app. I'll check those specifically for issues.

13. git_tools — apps/web/src/**/*.css d35c71d..088b2ab

14. git_tools — apps/web/src/components/ui/** d35c71d..088b2ab

💭 I'm noticing the change to popover.tsx expands the transition property from just transform to also include top, left, right, and bottom — meaning position changes now animate globally across all popovers, not just transforms. This could introduce a lag when popovers reposition due to collision detection or a moving anchor, since it's a shared primitive change affecting every consumer, not just the inline panel it was likely intended for.

15. browse_code — apps/web/src/components/ui/popover.tsx

16. git_tools — positionerClassName|collisionAvoidance

💭 I notice PanelLayoutControls disables the new positioner transition at its one call site, but the broader primitive change from transform-only to including top/left/right/bottom transitions now affects every other popover in the app. Since base-ui's Positioner writes inline top/left styles that update on scroll/resize/anchor changes, this could cause visible lag in popovers elsewhere. I'm checking whether Tailwind's transition-[top,left,right,bottom,transform] actually applies a 150ms duration by default — it does, since Tailwind pairs custom transition-property utilities with default duration and timing unless overridden. So any top/left changes trigger a 150ms animation, meaning during scroll with autoUpdate, the popup would visibly trail behind its anchor since the transition restarts every frame as position updates continuously. I'm wondering why this transition was added in the first place — maybe to animate the details popover on layout width changes — but it seems oddly disabled at one call site with !transition-none, which hints the primitive was changed and then patched over locally. I want to look at the git history for popover.tsx to understand the original intent behind that change.

17. git_tools — apps/web/src/components/ui/popover.tsx

18. browse_code — apps/web/src/components/chat/PanelLayoutControls.tsx

💭 I want to figure out which commit changed that transition, so I'm checking blame on line 51 of popover.tsx. There are two candidate commits — one splitting open-in-editor controls, one mapping the thread panel into the title bar and sidebar — so I'll look at the diff for the latter before running blame directly.

19. git_tools — apps/web/src/components/ui/popover.tsx

💭 The transition change originated in that PR, and the only new consumer immediately overrides it with transition-none, which suggests the primitive-wide change affects every other popover with animated positioning while nothing actually benefits from it. This looks like a solid finding — reverting the positioner transition back to transform-only while keeping the other additions seems right, so I want to double-check whether prior runs already caught this. Now I'm scanning other changed components with panel display modes for consistency issues — OpenInPicker, ProjectScriptsControl, GitActionsControl, ThreadAutomationsPanel, ThreadRelationshipsControl — and confirming MenuPopup itself wasn't touched by this PR. I also want to verify whether scrollFade is a legitimate prop on ScrollArea before trusting its use in ThreadDetailsPanel.

20. git_tools — scrollFade

21. git_tools — apps/web/src/components/chat/OpenInPicker.tsx d35c71d..088b2ab

💭 I'm looking at OpenInPicker's panel mode, where they swap the Group primitive for a raw div with role="group" and an aria-label to preserve semantics—though passing role="group" to Group itself in toolbar mode seems redundant but harmless. Now I'm checking whether the MenuPopup's width-detection contract recognizes w-(--anchor-width) as a valid width utility that should suppress the default min-w-32, since it's an arbitrary CSS-variable-based width value rather than a standard utility class.

22. git_tools — min-w-32|WIDTH|width

23. browse_code — apps/web/src/components/ui/menu.tsx

💭 That width pattern checks out. Now I want to look at the other panel-mode diffs: ProjectScriptsControl, GitActionsControl, ThreadAutomationsPanel, ThreadRelationshipsControl, and the main chat layout wiring for ChatView/ThreadPanel presentation.

24. git_tools — apps/web/src/components/GitActionsControl.tsx d35c71d..088b2ab

💭 I'm noticing that in the toolbar (non-panel) mode of GitActionsControl, the disabled quick action's padding class was dropped when the panel conditional replaced it, so the icon alignment tweak is lost in that path, and the enabled quick action button seems to have lost a similar padding class too. Confirming this from the diff: the toolbar variant switched to outline/ghost conditionally on isPanel, and the ps-[8.5px] optical alignment override was dropped for the non-panel case, along with the same drop for the disabled popover trigger. It's a tiny half-pixel visual regression rather than a functional bug, so I'm weighing whether it's worth flagging given how minor the alignment shift actually is. I'm also noting that the sibling OpenInPicker component still retains ps-[8.5px] for its non-panel path, which makes the GitActionsControl change look like an accidental inconsistency rather than an intentional one — that's a small but concrete finding worth calling out. Separately, I want to check whether the open_pr quick action handling was fully removed across the icon resolver, disabled-reason logic, and dialog/action runners, or whether it's just been relocated elsewhere in the flow.

25. git_tools — open_pr

26. git_tools — ps-[8.5px]

💭 I found something significant: gitActions.ts still generates open_pr quick actions and menu items, but GitActionsControl on the web side stripped out all the handling for it—the icon mapping now defaults to InfoIcon, runQuickAction silently no-ops on click, and openDialogForMenuItem falls through to the wrong dialog instead of recognizing that item kind. I need to check whether this PR actually modified gitActions.ts to gate open_pr from being returned to web callers—maybe there's a new parameter like hidePullRequestActions that prevents this dead-end scenario. Let me pull up the diff for gitActions.ts to see if resolveQuickAction was updated accordingly.

27. git_tools — packages/client-runtime/src/state/gitActions.ts d35c71d..088b2ab

💭 gitActions.ts appears unchanged, so resolveQuickAction can still return open_pr. I want to check the exact conditions in that file for when a PR exists and is open.

28. browse_code — packages/client-runtime/src/state/gitActions.ts

💭 I see that resolveQuickAction still returns "open_pr" with a "View PR" label whenever the branch has an open PR and is clean, which is a common case. But runQuickAction in web's GitActionsControl no longer handles open_pr, so I need to check the current fall-through path and look at how GitQuickActionIcon and openExistingPr are wired up.

29. git_tools — openExistingPr|const runQuickAction|quickAction.kind

💭 Checking if openExistingPr is referenced elsewhere in the file — the grep only returns its definition at line 1329, confirming it's now dead code since both call sites were removed. That means the "View PR" quick action and its menu item are now inert too. The "View PR" quick action's icon falls through to the wrong glyph (InfoIcon), and clicking it likely falls through all the runQuickAction branches with no matching action since open_pr has no action field defined. I need to look at lines 1615-1640 to see what actually happens.

30. browse_code — apps/web/src/components/GitActionsControl.tsx

💭 I've confirmed a concrete bug: the open_pr handling was removed from GitActionsControl, but resolveQuickAction and buildMenuItems still emit it. So when a branch has an open PR and is clean, the quick action button becomes dead (clicking does nothing), and the "View PR" menu item incorrectly opens the commit dialog instead. This isn't fixed by the panel mode either, since resolveQuickAction is shared across both toolbar and panel. Now I'm figuring out the best line to anchor this comment on in the diff — likely near the GitQuickActionIcon change or openDialogForMenuItem. I need to verify inline comments have to fall within a diff hunk, so I'm checking which lines around 346-348 and 1612-1626 are actually part of the changed hunks, since further-down lines like quickAction.action may fall outside the three-line context window. I'll settle on anchoring near the GitQuickActionIcon hunk since that's clearly within the diff, and note both consequences there. I want to pin down the exact icon fallback logic—checking lines 336-360 to confirm the final return statement after the CloudUpload/CloudDownload branches.

31. browse_code — apps/web/src/components/GitActionsControl.tsx

💭 Good, the SourceControlIcon param is still used, so that's fine. Now I want to check other in-scope areas like the ProjectScriptsControl panel, ThreadAutomationsPanel, and ThreadRelationshipsControl, plus figure out where the --thread-details-panel-width CSS variable is actually defined since I don't see CSS changes for it in apps/web/src. It might be set via a Tailwind arbitrary value or inline style attribute instead, so I should grep for it.

32. git_tools — thread-details-panel-width

💭 Wait, index.css was changed but my earlier glob search for CSS files under apps/web/src returned nothing—maybe the **/*.css pattern didn't match since **/ requires a subdirectory. Let me retry targeting the file directly.

33. git_tools — apps/web/src/index.css d35c71d..088b2ab

💭 Now I'm checking the CSS files in scope, noting that there's both a .surface-subheader utility class and a separate attribute-based selector targeting data-surface-subheader. I need to verify which consumers actually use each of these two different hooks to make sure they're not conflicting. I'm also noticing .glass-opacity-slider closely duplicates .settings-slider with nearly identical properties like slider height and track thickness, just with different variable names. This looks like new duplication introduced by the PR itself, so I want to compare them side by side to see if consolidating into one reusable class would be the better recommendation. I'm also checking whether --thread-details-panel-inset has real consumers by verifying the data attribute that toggles its value is actually set somewhere, and doing the same check for .alert-glass and its data-variant tints. Then I want to confirm whether the @media (min-width: 40rem) block for .messages-timeline-row-frame sits correctly within the same layer as the surrounding component rules.

34. git_tools — surface-subheader|surfaceSubheader

35. git_tools — thread-details-inline-reserved|messages-timeline-row-frame|chat-scroll-to-bottom|chat-content-lane|alert-glass|glass-opacity-slider

💭 I'm noticing .alert-glass gets defined twice — once as a pre-existing @utility alert-glass at line 284, and again by this PR's new class rules added around line 797. Since the diff only shows the addition at line 797, the original definition wasn't touched by this PR, meaning this change is creating a duplicate. I need to check whether these live in the same cascade layer to know if it's truly a conflict, so I want to look at the enclosing at-rule around lines 700-800 and 280-310 in index.css. Also concerning: a repo-wide grep for .glass-opacity-slider turned up zero consumers outside index.css, suggesting this new CSS class is entirely dead code with no component using it. Before ruling it dead code though, I should check for dynamic class name construction — like a settings component building a slider class name via string interpolation — so I want to grep for "slider" usage across the web app's source to be thorough.

36. git_tools — -slider

37. browse_code — apps/web/src/index.css

💭 I found there's a duplicated alert-glass style: the original utility includes a fallback for browsers without backdrop-filter support, but the newly added block omits it and sits in a different layer, so I need to check which layer takes precedence to know which version actually applies. I also noticed .glass-opacity-slider is unused dead code -- the actual component uses settings-slider instead, so this new block duplicates existing functionality without any consumer. I want to verify the enclosing block context for both these additions before recommending they be removed.

38. browse_code — apps/web/src/index.css

💭 I need to figure out which layer this block belongs to, so I'm searching backwards for the enclosing @layer declaration.

39. git_tools — ^@layer|^@Utility|^}

💭 I'm noticing .alert-glass is now defined twice—once as a utility with a supports fallback, and again as a plain rule inside the components layer—both using !important on background, which means the layer ordering will determine which one actually wins in the cascade. Since Tailwind orders layers as theme, base, components, utilities, and !important reverses precedence, the earlier components-layer rule actually beats the utilities one, meaning the fallback for browsers without backdrop-filter support never applies. That's a real regression worth flagging: the duplicate block in components should be removed. I'm also finding .glass-opacity-slider appears to be dead code with no consumers, essentially duplicating .settings-slider which is what the settings panel actually uses. Similarly, .surface-subheader looks unused as a class — components only reference it via data-surface-subheader attributes, not as a class name — so I want to double-check by searching more broadly for "subheader" to confirm there's no hidden consumer through a helper function.

40. git_tools — subheader

💭 I've confirmed the .surface-subheader class rule is dead CSS since actual consumers rely on the data-surface-subheader attribute with their own Tailwind classes instead, making the attribute-based override the live one. Now I want to double-check whether the related attribute selectors like [data-preview-panel-mode="inline"] and [data-right-panel-surface-content] actually exist elsewhere to confirm the override behaves as expected.

41. git_tools — data-right-panel-surface-content|data-preview-panel-mode

💭 I'm checking the DOM hierarchy — whether the surface-content element is actually a descendant of the preview panel shell, or if it's the reverse, since that determines whether the CSS selector chain would even match correctly.

42. browse_code — apps/web/src/components/preview/PreviewPanelShell.tsx

43. browse_code — apps/web/src/components/RightPanelTabs.tsx

💭 I've confirmed that data-preview-panel-mode is indeed an ancestor of data-right-panel-surface-content, so the selector at 769 matches correctly and isn't a finding. Now I'm noticing that the .surface-subheader class at line 765 appears to have no actual consumer, making it dead code worth flagging. I searched the repo for "subheader" references and found only the expected call sites, confirming it's genuinely unused — possibly intended for DiffPanelShell's header row utility but never wired up. I'm consolidating this with another dead CSS finding (.glass-opacity-slider) and deciding to keep the review to 4-5 inline comments max, starting with the GitActionsControl issue where open_pr handling was removed but gitActions still emits it, breaking the "View PR" quick action. I'm confirming the openDialogForMenuItem removal is anchored on a context line within the diff hunk, so I can comment there about both the missing icon and the click-through bug. I'm also planning comments on the popover transition change affecting all consumers, a CSS duplicate in .alert-glass dropping the opaque fallback, the dead .glass-opacity-slider class duplicating .settings-slider, the dead .surface-subheader class, and a minor dropped padding value in the GitActionsControl toolbar that creates an inconsistency with its sibling component. Now I need to pin down the exact line numbers in the new file for these anchors, particularly the quick action button's className around line 1802-1812.

44. browse_code — apps/web/src/components/GitActionsControl.tsx

💭 I'll add one comment at line 1807 covering both the ps-[8.5px] loss and its disabled variant. Then I want to quickly scan ProjectScriptsControl, ThreadAutomationsPanel, and ThreadRelationshipsPanel for raw button element misuse in the changed chat files.

45. git_tools — <button

46. git_tools — apps/web/src/components/ProjectScriptsControl.tsx d35c71d..088b2ab

💭 I'm checking that the "Actions" button correctly opens the imports menu via controlled state, and confirming the aria-labels on the panel's buttons look appropriate. I also notice new state for addScriptFormId and editingScriptId that's likely referenced later in the component. The non-panel branch of the toolbar swapped its icon and label from "Add action"/PlusIcon to "Actions"/WrenchIcon, which seems like an intentional style change. Now I want to peek at ThreadRelationshipsControl.tsx around line 87 to check whether it's using a raw button element instead of the shared primitive.

47. browse_code — apps/web/src/components/chat/ThreadRelationshipsControl.tsx

💭 I'm noticing the "Show N more" button manually rebuilds the panel row styling instead of using the shared Button component with the panel row class, which also means it's missing the focus-visible ring that the primitive provides. This is a genuine issue since these rows are described as focusable buttons for keyboard users, but no focus indicator exists.I want to check the other row buttons further down in the file, around lines 175-300, to see if they follow the same pattern or properly use Button with focus rings.

48. browse_code — apps/web/src/components/chat/ThreadRelationshipsControl.tsx

💭 I've confirmed only the "Show N more" row uses a raw button instead of the shared Button component with panel row classes, missing the focus-visible ring—so the fix is swapping it to use Button with variant="ghost" size="sm" and the appropriate classes. I'm also noting that ThreadLineageRowList is exported and unit-tested elsewhere, so I need to check how that affects the change. I realize the file doesn't currently import THREAD_DETAILS_PANEL_ROW_CLASS, only other related constants, so the fix would also need a new import line—meaning a simple inline suggestion diff won't be complete on its own. I'll present the fix as a small diff along with a note about adding the missing import, and now I'm moving on to finalize the full list of six issues, starting with the popover positioner transition regression and the GitActionsControl open_pr handling gap. Now I'm weighing whether to include a minor toolbar padding regression alongside two dead-CSS findings in index.css—since two of the dead CSS classes sit on different lines and can't share a single anchored comment, I'll keep them as separate but terse entries, and lean toward keeping the toolbar padding note since it's a real, cheap-to-flag visual regression rather than dropping it for noise reduction. I'm double-checking that each finding actually lands on an added or in-hunk line of the diff so GitHub will accept the comment—confirming the index.css additions, the popover line, and the GitActionsControl removal context are all valid anchors, including verifying that comments can attach to unchanged context lines within a hunk, not just added ones. I'm checking the hunk boundaries for each file to confirm my target lines actually fall within the diff ranges - line 1631, 1807, and 51 all check out against their respective hunk headers. Now I need to verify whether ThreadRelationshipsControl.tsx is a newly added file in this PR, which would mean lines 87-94 are covered entirely.

49. git_tools — apps/web/src/components/chat/ThreadRelationshipsControl.tsx

💭 Since this isn't a new file, I need to check whether the last commit touching it falls within this PR's range, so I'm diffing the file to see if that button block is actually part of this PR's changes.

50. git_tools — apps/web/src/components/chat/ThreadRelationshipsControl.tsx d35c71d..088b2ab

💭 I'm confirming the new file's line 87 is part of the diff, then drafting my review comment about the popover's transition change—since it now animates position for every popover globally but only the new consumer opts out, unrelated popovers could visibly lag behind their trigger during scroll or resize updates. For the second comment, I'm noting that resolveQuickAction/buildMenuItems still return open_pr for clean branches with an open PR, but all three handlers for that action were removed—meaning the menu item now falls through incorrectly, runQuickAction does nothing, the icon defaults to InfoIcon, and openExistingPr is left unreferenced. For the third comment, I'm flagging that the toolbar mode lost the ps-[8.5px] optical padding on this leading-icon button (and its disabled variant), which breaks alignment with OpenInPicker's matching toolbar button since that one still retains the padding—suggesting a conditional class based on whether it's a panel or toolbar variant. Now I'm looking at a duplicate alert-glass definition in index.css: it already exists as a @utility with the same declarations plus a backdrop-filter fallback, but this new copy sits in @layer components, and since both background declarations use !important, layer ordering causes the components version to win unexpectedly. I flag this along with two other likely-dead CSS blocks—glass-opacity-slider, which has no consumer since the actual glass opacity input uses settings-slider, and .surface-subheader, whose four call sites all rely on their own Tailwind classes instead. I recommend dropping the duplicate and dead blocks so each pattern has a single owner. For ThreadRelationshipsControl, I note that the "show more" row hand-rolls its own button styling and, unlike sibling rows, lacks a focus-visible ring—leaving keyboard users without a visible target. I'm proposing a diff that swaps the raw button for Button with size="sm" and variant="ghost" to restore consistent geometry and the focus ring. Since the full change also needs a matching closing tag and a new import for THREAD_DETAILS_PANEL_ROW_CLASS, which spans multiple hunks, I'm deciding whether to show a partial diff or just describe the change in words to keep things clean. I'm also weighing whether it's worth flagging a related pattern elsewhere—raw role="group" divs standing in for a proper Group primitive across a few other files. I've decided the div-based grouping is actually fine here, since the primitive's default rounded-join geometry conflicts with what the panel needs, so overriding it manually is justified and not worth flagging. Instead, I'm now focusing on a more significant pattern: several call sites are overriding Button's core height, radius, padding, and color styles with !important class overrides rather than extending its variant system, and I'm counting how many places this repeats to see if it crosses the threshold for a real shared-pattern issue worth raising. I'm ranking the eight candidate comments by real impact — behavioral regressions and app-wide interaction issues at the top, smaller stylistic nits lower. To keep the review focused, I'll post the top seven as individual comments and fold the styles-module-versus-Button-variant point into the review body as a cross-cutting note instead. Since a real broken control turned up, the overall check should be marked as failure rather than the default pass. For the review body itself, I'll keep it limited to the shared-constants/primitive-variant observation, since everything else is already covered by inline comments and the body should only hold what isn't said elsewhere.

51. modify_pr — Cross-cutting note (not repeated inline): the panel's row geometry now lives in apps/web/src/components/chat/threadDetailsPanelStyles.ts as class-string constants that six call sites splice into Button/SelectTrigger/ComboboxTrigger, overriding the primitive's height, radius, padding, text size and base/hover background — the hover tint needs !bg-... to win against the variant it is overriding. Since this is durable geometry shared by every panel row, a named Button size/variant (e.g. size="panel-row") would carry it inside the primitive contract and drop the !important overrides, leaving only contextual layout at the call sites. Not blocking this PR, but worth doing before more rows adopt the constants.