💭 This PR reverts a refactor that introduced ComposerBanner/ComposerSurface primitives, going back to raw divs and global CSS classes. I'm weighing whether this reversion away from shared primitives back to global index.css should get flagged, given it's undoing a previous change rather than introducing a new inconsistency. Looking closer at the diff, it deletes ComposerBanner.tsx, ComposerSurface.tsx, ComposerActivityStatus.tsx, and ComposerServerUpdateStatus.tsx, reintroducing raw markup plus roughly 466 lines of global CSS — but since this looks like an intentional revert to a prior known-good state, I should focus on finding genuinely concrete regressions rather than objecting to the revert itself. I'm noticing the ComposerBannerStack lost its keyboard/Escape handling and the accessible "peek" button (with aria-expanded/aria-controls and an aria-label) was replaced with a plain aria-hidden div, which looks like a real behavioral regression worth flagging. That's worth flagging — hover-only disclosure without keyboard or touch affordance is a real regression. Looking at ComposerStashMenu, I notice removed focus management for arrow-key navigation, a semantic downgrade from a <time> element to a plain <span>, though the pointer-events-none delete button still works fine with keyboard focus. In ComposerPlanFollowUpBanner there's a commented-out dead code block being reintroduced — minor cleanliness issue but not a major UI concern. Now I'm moving on to check ChatComposer next. Reverting ChatComposer removes the auto-close logic for the stash menu, so now it stays open even after the last entry is removed. I'm also tracing through whether showShoulderTabs can render the stash badge with a count of zero when only task progress exists but no stashed prompts — that would surface an empty "Stashed prompts: 0" tab, which looks like a genuine bug worth digging into further in the diff. I should check whether ComposerStashBadge has an early return when count is zero — the diff doesn't show one, so I need to look at the actual file to confirm. The shoulderTabReserve test checks that surface top minus tab top equals 28, which matches the expected calculation, so that's fine. The ChatView revert also renders ComposerBannerStack in both branches, which seems okay. One thing worth flagging: there's a change to the update-in-progress condition logic in ChatView.tsx — the revert alters the boolean logic around updateInProgress, updateFailed, and versionMismatchDismissKey in a way that changes the original condition's meaning. That change reverts the dismissible failed-update banner behavior, which is a feature reversion but seems intentional given the scope of this revert. There's also a change from a span to a div in ComposerPendingApprovalPanel, which should be fine since it's now nested inside another div, and MessagesTimeline reverts to its prior row structure. Since this is a large revert, I need to verify a few things directly against the actual repo rather than relying on the diff alone. I want to check ComposerStashBadge.tsx and ComposerBannerStack.tsx at head, and confirm whether the shoulder-tab group selector class still exists anywhere in the codebase. I'm also checking whether the shoulder-tab selector change is consistent, then scanning for any leftover references to modules that were deleted in this revert—things like ComposerBanner, ComposerSurface, ComposerActivityStatus, or ComposerServerUpdateStatus—since stray imports of those would break compilation. I want to grep across the repo for any remaining imports or CSS selectors tied to these removed components, including checking index.css for leftover data-attribute selectors and confirming no other files still reference ComposerBanner outside what's already in the diff.

1. git_tools — ComposerBanner|ComposerSurface|ComposerActivityStatus|ComposerServerUpdateStatus|ComposerActivityRow|composer-shoulder-tab|data-composer-banner",

2. browse_code — apps/web/src/components/chat/ComposerStashBadge.tsx

💭 Good, ComposerStashBadge correctly returns null for zero count, avoiding an empty stash tab, and there are no dangling imports of deleted modules. Now I want to verify the CSS classes referenced in the diff—chat-composer-stash-tab, chat-composer-shoulder-tab, chat-composer-tasks-tab—are actually defined in index.css, and I should also check the alert-glass class used by ComposerBannerStack. I also need to confirm Alert, AlertAction, and AlertTitle exist in the ui/alert module, and that Button supports the ghost-muted variant along with micro and icon-micro sizes, so let me grep index.css for these class names.

3. git_tools — chat-composer-stash-tab|chat-composer-tasks-tab|alert-glass|chat-composer-drawer-slot|chat-composer-banner-stack-cap

4. browse_code — apps/web/src/components/chat/ComposerBannerStack.tsx

💭 I'm noticing chat-composer-stash-tab is used as a class but has no matching CSS rule in index.css, unlike chat-composer-shoulder-tab. This suggests it's just a marker class for tests, and I'm trying to figure out whether it existed before the revert or was only ever defined in the ComposerSurface/ComposerBanner modules. This points to a real finding: the banner stack disclosure has become hover-only and keyboard-inaccessible. The reverted code drops the focusable peek button (with proper aria-label, aria-expanded, aria-controls) and Escape handling, replacing it with an aria-hidden, pointer-events-none cap div relying on CSS group-hover/group-focus-within. Since stacked items stay visibility:hidden until group-focus-within triggers, their children are pulled out of the tab order entirely — so keyboard focus can never enter the group and focus-within can never activate, meaning keyboard and touch users lose access to stacked banners and their action buttons. I'm confirming this is accurate: visibility:hidden on an ancestor does make descendants unfocusable in browsers, so this is a legitimate, concrete accessibility regression tied to the changed lines. I'm also noting a second candidate issue in the stash menu: arrow-key navigation now only updates highlighted state without scrolling the highlighted entry into view or moving focus, so with a scrollable list capped at a fixed height, keyboard users could highlight entries that remain off-screen — I want to verify this against the actual scroll container behavior before treating it as confirmed. I want to check whether the drawer correctly closes when the last stashed entry is removed, since the badge count logic and the open/closed state for the drawer may be tracked independently, potentially leaving an empty "Nothing stashed yet" drawer open after deletion. I should also verify whether ChatComposer conditions rendering the stash menu on both isStashMenuOpen and a non-empty queue, and I'm now noticing the plan follow-up banner brings back some previously commented-out code that needs checking. I'm also flagging that the stash menu swapped <time dateTime> for a plain span, losing the machine-readable timestamp, and that the restore item switched from a labeled button to a CommandItem with an onClick but no accessible label — worth checking if that component provides one internally. The list previously had an aria-label="Stashed prompts", and the test now explicitly asserts that string is gone, confirming the removal was intentional rather than accidental. Elsewhere, the tasks badge tab still uses a raw <button>, which seems acceptable given normal tab semantics. The reintroduced global CSS in index.css does conflict with the rule that presentation belongs in the owning module, though masks and glass composition are explicitly allowed to stay global, so I don't think that's a hard violation—more of a consistency note. The stronger concrete issue remains the banner stack's keyboard accessibility, so I want to go check ComposerStashMenu and ChatComposer around the stash logic to verify that finding.

5. browse_code — apps/web/src/components/chat/ComposerStashMenu.tsx

6. git_tools — isStashMenuOpen|ComposerStashMenu|showComposerTopDrawer =

💭 I want to check ChatComposer around lines 1370-1400, then look at the stash menu render and takeStashEntry handler near 2570-2600, along with the shoulder-tab and drawer render around 2800-2900.

7. browse_code — apps/web/src/components/chat/ChatComposer.tsx

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

💭 I notice deleteStashEntry no longer closes the menu when deleting the last entry, so I need to find where the stash menu is rendered, likely in ChatComposer.

9. git_tools — ComposerStashMenu

💭 I'm noticing the search stopped because it hit a null byte in ChatComposer.tsx, treating it as binary — that seems odd, so I want to check whether that null byte was already present at the merge base rather than something introduced later.

10. git_tools — ComposerStashMenu

11. browse_code — apps/web/src/components/chat/ChatComposer.tsx

💭 I need to locate where the stash menu is rendered in the head branch to check its guard conditions, so I'll search around lines 3600-3700.

12. browse_code — apps/web/src/components/chat/ChatComposer.tsx

💭 Deleting the last stash entry leaves the menu open with an empty state, and reverting removes the auto-close—likely a minor, deliberate regression since the drawer's still closable manually. I'll note it briefly but keep focus on the more concrete findings worth flagging, starting with the stacked banners issue in ComposerBannerStack. The invisible wrapper blocks focus from entering, so group-focus-within can never fire, and the peek trigger is aria-hidden with pointer-events-none, leaving hover as the only way in. That makes actions and dismiss buttons in stacked banners completely unreachable for keyboard and touch users—the fix is restoring a focusable peek control with proper aria attributes driving explicit state, like the pre-revert version had. Now I'm checking the ComposerStashMenu for whether arrow-key navigation still scrolls the highlighted entry into view. Also noting the time element losing its dateTime attribute in favor of a plain span, which I could fold into the earlier comment about lost semantics. There's also dead commented-out JSX reintroduced in ComposerPlanFollowUpBanner — marginal, but concrete and easy to fix, so I'll flag it briefly rather than skip it entirely, keeping the main inline comments focused on the two significant issues plus the stash menu auto-close behavior. The bigger structural issue is the removal of shared ComposerBanner/ComposerSurface primitives, replaced by ~470 lines of hand-rolled CSS and inline markup duplicated across ChatComposer, BranchToolbar, and ComposerPendingUserInputPanel — BranchToolbar in particular now repeats composer-surface geometry classes inline rather than reusing a shared component, which is the clearest consistency violation worth flagging. Since it's precisely the changed lines, I'll settle on a balanced review with 2-3 concrete findings: the banner stack's hover-only disclosure is an a11y issue, the stash menu's arrow navigation loses scroll-into-view and focus tracking, and the shared banner/surface primitives got replaced by duplicated raw markup and repeated class strings (including the pb-[calc(...)] pattern) — worth flagging at summary level. I also want to check ComposerTasksBadge and look at whether ChatView.tsx has any similar concrete issues. I checked the ComposerBannerStack rendering in both branches and it looks fine, same with the shoulder-tab padding logic in ChatComposer and the shoulderTabReserve query. Now I need to figure out the correct line numbers in the diff for placing inline comments — I'll pull the PR's file patches to determine positions on the right side of ComposerBannerStack.tsx, targeting around lines 149-157 for the invisible wrapper region. Both the invisible wrapper region and the cap div (lines 111-123) are within the added diff hunk, so they're valid targets for inline comments. Rather than a range, I'll just place a single-line comment at line 152 for simplicity. For ComposerStashMenu.tsx, since the removed scrollIntoView/focus lines are on the left side, I'll point the comment to line 86 on the right, which is definitely part of the added hunk. Now I need to figure out where the duplicated drawer markup lives in ChatComposer.tsx for the third comment.

13. browse_code — apps/web/src/components/chat/ChatComposer.tsx

14. browse_code — apps/web/src/components/chat/ComposerStashMenu.tsx

💭 I'm checking the delete button's visibility logic — it's hidden by default with opacity-0 and pointer-events-none, but keyboard tabbing can still focus it since pointer-events doesn't block tab focus, and focus-visible then reveals it properly. But the bigger issue is the restore action itself: CommandItem renders as role="option" and isn't independently focusable without an owning input using aria-activedescendant, unlike the old implementation which used real <button> elements per row with proper aria-labels that were fully tab-navigable. This reversion means keyboard users lose the ability to Tab directly to a stash entry to restore it — they're stuck with a global keydown handler that restores whatever's highlighted, without any screen-reader announcement or scroll-into-view behavior. That's a real accessibility regression in the stash drawer worth flagging. I'm treating this as a legitimate finding, so the check should fail here, though I'll frame it constructively since this is a revert that may have reintroduced the issue knowingly. I also want to verify how ComposerStashBadge and ComposerTasksBadge behave together in the shoulder tabs area, particularly around the trailing shoulder logic when the stash queue is non-empty. I'm also noticing repeated padding and drawer-surface class combinations duplicated across several files—ComposerBannerStack, ThreadSyncStatusPill, and others—which reinforces the case for extracting a shared primitive. I'll capture that as one consistency-focused finding, and now I'm drafting the inline comments, starting with the hover-only disclosure issue in ComposerBannerStack around lines 142-156. The suggested fix for that keyboard-focus regression isn't a clean one-liner since it needs reintroduced state plus a button element, so I'll skip a diff there and just describe the approach in prose instead. For the second issue in ComposerStashMenu (arrow navigation no longer scrolling or focusing rows since the restore buttons were removed), I'm weighing whether a scrollIntoView call tied to a ref could work, but since the exact data attributes and rendering behavior of the row elements are uncertain, I'll again avoid a diff and just describe the minimal fix needed. Now I'm looking at the third issue — a new file duplicating the same drawer surface class contract across four separate call sites. Since this pattern of repeating identical durable geometry across multiple components is worth flagging, I want to leave a comment noting that the same attached-drawer surface styling is now copy-pasted across several composer-related files rather than shared from one place. I'm also thinking through a smaller possible regression: deleting the last stash entry might leave an empty drawer still attached to the tab, which could be worth a low-key note as well. I'm sketching a suggested fix that closes the stash menu when the queue empties out, but I want to check whether this is actually a regression versus an intentional empty state, since the pre-revert code explicitly added that auto-close behavior. I decide it's fine to note briefly rather than flag as an inline comment. I also want to mention the leftover commented-out JSX in ComposerPlanFollowUpBanner as a small, cheap suggestion to just delete it, so I'm pulling up that file to get exact line numbers.

15. browse_code — apps/web/src/components/chat/ComposerPlanFollowUpBanner.tsx

💭 I've confirmed those lines are dead commented-out JSX. I'm double-checking the ComposerBannerStack keyboard behavior against the pre-revert version—the Peek button with its aria-label, aria-expanded/controls, click handler, Escape key handling, and pointer enter/leave all matched, so the fix is accurate. Now I'm starting to write inline comments. For the first, I'm anchoring on the invisible wrapper region to explain that stacked banners can only be revealed by hover: the collapsed cap is aria-hidden and pointer-events-none, while the expanded region stays visibility-hidden until group-focus-within, which can't fire since hidden subtrees are pulled from the tab order. This means keyboard and touch users lose access to every stacked banner and its actions, since the removed Peek button was the real accessible trigger with aria-expanded/controls and an Escape path—I want to suggest keeping a focusable toggle that renders hidden items only when expanded. For the second comment, I'm looking at the ComposerStashMenu arrow-key navigation, noting that it now only updates highlightedId without the corresponding re-sync elsewhere that it used to have. Now I'm tracing that removed scrollIntoView/focus logic was what kept the highlighted entry visible and moved real focus to it—since rows are non-focusable CommandItems with no accessible name and there's no aria-activedescendant, keyboard users can highlight off-screen entries with no indication to assistive tech. I want to suggest restoring scroll-into-view behavior and exposing the restore action as a focusable, labelled control. Then for the third comment, I'm flagging that the drawer surface class contract is now duplicated across five components (ComposerBannerStack, ComposerStashMenu, ComposerCommandMenu, ThreadSyncStatusPill, and ChatComposer's top drawer), which risks geometry drift over time. I checked whether the pb-overlap padding is actually inconsistent across call sites — tracing through the CSS, it turns out the padding-bottom is applied fairly consistently via the shared class or CSS rule, so I'll soften my note to just point out the overlap calc() is spelled out twice rather than claiming outright inconsistency. I'm suggesting a shared wrapper component for the surface classes going forward. For ComposerPlanFollowUpBanner, there's dead commented-out JSX around lines 16-18 that should just be deleted — I'll provide a diff removing those lines cleanly. Since the file matches the include patterns, I'm ready to submit the review with these fixes. Now I'm working through each remaining comment to make sure I supply all required GitHub review fields — path, body, line, start_line, side, start_side — using proper line ranges rather than equal start/end lines to avoid API errors. I'm mapping out the specific line ranges for the ComposerBannerStack.tsx and ComposerStashMenu.tsx comments so each one spans the correct added lines in the diff.

16. modify_pr — ## UI Consistency — 4 findings This revert restores the pre-primitive composer markup. Most of it is a faithful rollback, but a few keyboard/AT affordances that existed on main do not come back, and the shared attached-drawer contract is now duplicated across call sites. - ComposerBannerStack.tsx — stacked banners become hover-only; the group-focus-within path is unreachable because the region is visibility: hidden. - ComposerStashMenu.tsx — arrow navigation no longer scrolls the highlighted row into view or moves focus, and rows are no longer focusable/labelled controls. - ThreadSyncStatusPill.tsx (+ 4 other call sites) — the attached-drawer surface geometry is now copy-pasted rather than owned by one component. - ComposerPlanFollowUpBanner.tsx — commented-out JSX reintroduced. None of these block the rollback itself; they are the parts of the reverted behavior worth keeping or re-adding.