💭 I'm looking at the ChatComposer.tsx changes, where the fallback value for the drawer inset CSS variable was removed since it's now defined directly on the Shell. I want to verify that the computed style lookup still resolves correctly even when the anchor element isn't nested inside that Shell component. Then in ComposerBannerStack.tsx, there are accessibility changes worth checking—removing the onFocusCapture handler that used to expand the stack on focus, and adding tabIndex/aria-hidden attributes to the peek element, which raises a question about whether aria-hidden could end up on an element containing a focused child. I also notice the attachment width calculation now depends on that CSS variable being defined on an ancestor, and that a max-width constraint was removed from the ContextStrip, which previously capped it to match the composer's width—removing that could change the layout at wider viewport sizes if the parent Shell doesn't already constrain the width itself. I should look at the actual files to confirm.

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

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

💭 I'm noticing that the max-width removed from ContextStrip and Attachment matches exactly what the Shell's max-w-3xl already provides once padding is subtracted, so it seems redundant unless a call site overrides the Shell's width with a wider class, in which case the behavior would change. Now I'm checking the aria-hidden and tabIndex logic on the Peek button—when it expands, focus moves via a layout effect to the first control in the expanded items, but there's a brief window where aria-hidden is set on the element that still has focus before the effect runs, which could trigger a transient accessibility warning. I want to trace the pointerEnter expansion path too: focus only shifts to "notice" if the peek was the active element, otherwise focus stays put, which seems fine. I should check ComposerBanner.Peek's implementation to confirm it properly forwards tabIndex and aria-hidden, and also look at the drawerInset parseFloat call in ChatComposer since it lacks a fallback and could produce NaN.

3. browse_code — apps/web/src/components/chat/ComposerBanner.tsx

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

💭 I'm worried the drawer inset CSS variable could be undefined if the form isn't wrapped in a Shell, which would produce NaN values for left and width — the previous fallback of 1.375 guarded against that. I need to verify the var is always defined on an ancestor and check how the anchor fallback for the form is handled.

5. git_tools — ComposerSurface.Shell|data-chat-composer-form|chat-composer-drawer-inset

💭 I'm trying to track down where the form attribute gets set in ChatComposer, since my grep only found it near ComposerBanner and the closest selector. I'm also checking whether ChatComposer is always nested inside ComposerSurface.Shell, or if there are other rendering paths outside ChatView. There's a real concern here: ComposerBanner.Attachment now depends on the --chat-composer-drawer-inset CSS variable for its width calculation, but if that variable is undefined outside the Shell context, the calc() becomes invalid and the width would fall back to auto instead of the previous fixed-width behavior. This could be a regression if attachments render outside the Shell, so I want to find all the actual usages to confirm.

6. git_tools — ComposerBanner.(Attachment|Dock)|<ChatComposer|ComposerBannerStack

💭 ChatComposer only renders inside Shell within ChatView, so the variable is always available there — the parseFloat fallback removal isn't actually a robustness loss since the var is now defined at Shell level. I'm now double-checking whether the form referenced in the computed style call is also scoped inside Shell. Turning to ComposerBannerStack's accessibility changes: removing focus-capture expansion means the stack now only expands via pointer hover or clicking the peek button, and since the peek is a real button, Enter/Space still triggers expansion and moves focus into the notices — that seems sound. The blur-capture collapse logic when focus leaves and hover isn't active also looks correct, though I want to trace through what happens when expansion is triggered by hover alone versus keyboard interaction, since there could be a subtle mismatch between hover-driven and focus-driven expansion states. Checking the collapsed state's focusability: since the inner wrapper gets invisible when collapsed, its descendant buttons become genuinely non-focusable via visibility, so tabbing correctly skips over hidden notice items — that part checks out. Now I'm looking at the newly added preventDefault() on Escape keydown within the notices container, and I'm concerned this could interfere with nested interactive elements — if a notice contains its own popup or menu that relies on Escape to close, this container-level handler sitting on a bubble-phase listener might intercept and collapse the whole stack instead of just closing the nested element, especially since stopPropagation was already present before this change. Focus-visible styling on the peek button seems fine since it's small. I'm also considering the edge case where Escape triggers focus back to the peek but the peek isn't rendered, losing focus entirely. There's also a subtler issue: the peek carries aria-expanded/aria-controls while being aria-hidden when the stack is expanded, which could confuse screen reader semantics. Then I shift to checking the ChatComposer ordering change, placing the activity stack item first in the array before banner items—since the sort by priority is stable, this only affects ordering among equal-priority items. I'm also verifying the CircleAlertIcon's text-error class actually maps to a valid color token in this Tailwind setup, checking whether --color-error is defined in the theme.

7. git_tools — --color-error|text-error\b

8. git_tools — LoaderCircleIcon|motion-safe:animate-spin

💭 That's consistent with ComposerActivityStatus. Now I want to check the full ComposerServerUpdateStatus and its usage in ChatView to confirm the statuses line up and that the variant matches, like whether the banner error variant corresponds to the right icon color.

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

10. browse_code — apps/web/src/components/ChatView.tsx

💭 The icon span is aria-hidden already, so the redundant aria-hidden on icon components is harmless. I'm checking the ToggleIcon's pointer-events-none addition, since that span uses ghost button styles with hover effects nested inside an actual button row, and I want to see whether disabling pointer events there would suppress the hover state unexpectedly. Now I want to trace all ToggleIcon usages to confirm none of them rely on the span itself receiving hover or click events, like a tooltip trigger, before assuming pointer-events-none is safe everywhere.

11. git_tools — ToggleIcon

💭 ToggleIcon usages all sit inside triggers or rows, so pointer-events-none stays consistent. I confirm the role="list" addition is the standard Safari VoiceOver fix for list-style: none, and the ComposerBanner styling looks fine. Now I'm digging into ComposerBannerStack's accessibility details, starting with how the Peek element's aria-hidden state interacts with expansion. I'm tracing the focus timing: when activeElement is somewhere else, nothing changes on hover, but when the peek itself is focused, hover triggers a notice and moves focus before paint. For keyboard activation via Enter, I'm checking whether the querySelector used to find the first focusable control inside the newly expanded item could accidentally match a control that's still visually hidden during the transition, since visibility and grid-rows are being set in the same commit. I'm also considering whether calling focus() in the layout effect works correctly given React commits the DOM before running layout effects, so the visibility class should already be applied by the time focus is attempted. Then I'm thinking through which control ends up first in document order within a notice row (icon, content, then actions/dismiss), and confirming that removing focus capture on the collapsed stack is fine since those elements aren't focusable when invisible anyway -- but I'm flagging that hover-driven focus stealing could still interrupt someone navigating by keyboard. The rationale for the pointer-triggered focus shift makes sense though: since visibility:hidden blurs the focused element and sends focus to the body, the code needs to explicitly redirect focus somewhere sensible when the peek collapses out of view, so that's a deliberate design choice rather than a bug. I'm also noting a smaller concern that aria-hidden on the peek combined with aria-expanded could confuse screen reader users about the disclosure state while the stack is expanded. Checking the blur-driven collapse logic now: when focus moves from the peek into the notices group, the relatedTarget check confirms it's contained within the notices div so it doesn't trigger collapse, and the same holds since the peek itself lives inside that div. Tracing through the Escape key handler and layout effect for refocusing the peek after collapse, then examining a trickier edge case where an item inside the expanded list has focus when the stack collapses — the item becomes invisible, blur fires with a null relatedTarget, and that correctly triggers the collapse handler. Now I'm confirming that focus properly lands back on the peek via the layout effect and that the min-height utility is just preserving the collapsed gap. I'm also checking whether the raw button used for the peek/strip control violates the primitives-over-native-controls guideline, but since it's a pre-existing shared component acting as a handle, it should be fine — then moving to check whether the focus-visible outline styling on the group container creates any issue. I'm checking whether Attachment and ContextStrip components stay within the Shell's max-width containers, and confirming the composer banner stack and command menu portal don't need direct access to the drawer inset variable since it's read from the form element instead. Both attachment and banner components are only used inside apps/web, so the scope stays contained. Now I'm scrutinizing the calc expression syntax more closely -- CSS requires whitespace around the minus operator in calc(), and I need to verify whether the arbitrary Tailwind value with 100%-2*var(...) actually parses correctly given that spacing rule. I'm now checking whether Tailwind's math normalization also reaches into the more complex calc() expressions nested inside shape() for the clip-path values, since those contain var(--...)*0.4477 and similar operators that also need spacing. I recall the parser walks the entire value tree, so nested calc expressions inside non-math functions like shape() would still get found and processed—this matches how the previous shipped code with a similar nested calc pattern worked. The remaining question is whether multiplication with * gets the same whitespace normalization treatment as + and -, since these calcs previously didn't include any multiplication. I'm cross-checking the docs against the implementation—Escape closing the stack, touch peek behavior, and Tab/Enter focus handling all line up with what's coded. One thing catching my eye is the redundant explicit tabIndex on the peek button, though that's harmless, and I want to double check whether hiding the button with aria-hidden while it's still focusable creates an accessibility conflict. I'm also thinking through edge cases in focus management: if expansion happens via hover while focus sits on the peek, pendingFocusRef only gets set when the active element matches the peek ref, which seems fine. But there's a gap when the stack auto-collapses because items drop below two—if focus was inside the now-hidden expanded items, nothing redirects it and focus could just fall back to the body instead of somewhere sensible. I'm considering whether this focus-loss issue is actually new or just a pre-existing quirk from before, since removing onFocusCapture was intentional to stop unrelated focus events from triggering an auto-expand. Then I check ComposerBannerStack for a related accessibility gap: when expanded, the peek is aria-hidden but still carries aria-controls and aria-expanded, meaning screen reader users navigating by virtual cursor may lose track of the toggle to collapse, though Escape still works as a fallback—minor, not a blocking issue. Now I want to look harder for a genuine bug worth flagging, specifically around the added preventDefault() in the Escape keydown handler on the notices container. I'm wondering if stacked notice content—like the stash menu, tasks badge, or pending user input panel—could contain nested popups such as Select, Menu, Tooltip, or Collapsible components, and whether pressing Escape while one of those is open would cause unintended interference. I need to trace back to ChatComposer to see how these stack items are actually assembled. I'm now looking at where bannerItems and activityStackItem get combined. A second bug candidate is the reordering to put activityStackItem first — since sort is stable and activity priority is 0, it lands first, but I need to verify what priority activityStackItem actually carries. Checking ChatComposer around line 2820-2930 to confirm.

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

💭 The activity item sorts first as expected, so the stacked hidden items are just notice banners, and their focus queries should work fine. The group's aria-label fallback for focus also looks correct, so this change seems safe overall. Looking closer though, the aria-hidden attribute is redundant since the invisible styling already removes the element from the accessibility tree, but that's harmless. The bigger issue I'm spotting is around pointer-events-none on the peek element when expanded—since the container collapses to zero height with the peek absolutely positioned at its bottom edge, I need to trace through how the hover/hit area actually behaves in the collapsed state. That confirms why min-h-3 was added — it maintains a 12px hover target during the collapse-to-expand transition so the pointer doesn't accidentally leave and cause a flicker loop, though once items are actually rendered their own height makes min-h-3 visually irrelevant. Now I'm checking the pointerLeave handler behavior when expanded and the pointer exits. Removing onFocusCapture doesn't affect the front banner, and the toggle icon's use of buttonVariants on a decorative span with aria-hidden and pointer-events-none looks like an accepted pre-existing pattern, not a new violation. That's not really our job to fix here. There's a minor edge case in the querySelector logic for finding the first focusable control—it doesn't exclude hidden or negative-tabindex elements, but that's minor. I'm also tracing a potential stale-ref bug: if pendingFocusRef gets set to "notice" but setStackExpanded(true) fires while already true, the layout effect's dependency array won't rerun, leaving pendingFocusRef stuck. Walking through several scenarios though—hover expand while focus is on peek, click on peek while already expanded (blocked by pointer-events-none), pointerEnter while peek has focus but stack already expanded (impossible since peek becomes invisible/unfocusable)—none of them actually trigger the stale state, so it seems like a non-issue in practice. That stale "peek" state persists until the stack collapses again, at which point the effect fires and focus jumps to the peek button — potentially stealing focus away from wherever the user was actually typing, like the composer textarea. This feels like a genuine bug: a user tabs to the peek, hits Escape without expanding (a plausible reflex action), then later just hovers and moves away, and suddenly focus unexpectedly shifts to the peek button. So when focus lies outside the container and collapse fires via pointerLeave, the effect still yanks focus onto the peek button — a genuine regression worth flagging, if a bit edge-case. The cleanest fix is to guard the Escape handler so it only sets pendingFocusRef when the stack is actually expanded, bailing out otherwise. That guard also fixes a second, more concrete issue: on a collapsed stack, pressing Escape now calls preventDefault and stopPropagation, swallowing the key event before global handlers (like ChatView's stop-generation/close-panel/blur-composer shortcuts) can see it. The stopPropagation was already there pre-PR, but preventDefault is new, and both problems disappear with the same expanded-state check. I want to confirm there's actually a global Escape listener that would be affected — React's stopPropagation blocks the native event too, so bubble-phase window listeners wouldn't fire, though capture-phase ones still would, and that part predates this change anyway. I'll flag the stale pendingFocusRef / unconditional Escape handling as one finding on lines 167-173, since it's a genuine focus-stealing regression introduced by these changed lines and warrants an inline comment and a failing result rather than "All clear." Tracing the reachability: the collapsed peek has tabIndex 0 so it's focusable, sits inside the container with the onKeyDown handler, and keydown events bubble up to it — so Escape while focused on the collapsed peek does trigger the buggy handler. Confirming the stale ref persists — setStackExpanded(false) as a no-op means React bails out of re-rendering, and even if it does re-render once, the layout effect's dependency array doesn't change, so the effect never reruns and the stale ref sticks around. Then on the next expand/collapse cycle (hover in, pointer leave with focus outside), the effect fires and incorrectly steals focus back to the peek because pendingFocus still says "peek." There's also a similar path where clicking the peek sets pendingFocus to "notice." I've confirmed this is a real bug and I'm drafting a fix: adding an Escape key handler that, when the stack is expanded, prevents default, stops propagation, sets pendingFocusRef to "peek", and collapses the stack. I'm checking the exact indentation in the file to match the existing style before writing up the suggestion. I'm reconsidering the exact line range to target for the inline comment, trying to make sure both endpoints fall within the diff hunk boundaries so GitHub accepts it, and weighing whether to include a suggestion block replacing the Escape-key handler lines with the preventDefault/stopPropagation/focus logic intact.

13. modify_pr — One finding in ComposerBannerStack: the Escape handler runs even when the stack is already collapsed, which leaves a stale pending-focus request that later steals focus. Details inline.