💭 I'm reviewing the in-scope files—the composer approval components, ChatComposer, session logic, and tests. It looks like the appName shrink issue flagged in the prior run has been resolved with the truncation fix. Now I want to check the current state of these files' code more closely.

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

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

💭 Now I'm checking the ComposerPendingApprovalActions component for consistency—it uses the Button primitive with a ghost-muted micro variant, which looks right. I'm noting that keying by option.decision could cause React key collisions if the provider ever supplies duplicate decision values, since options aren't deduplicated before rendering. I'm also considering that mobile's PendingApprovalCard drops the default "cancel" option, though that's out of scope for web. More importantly, provider labels like "Always allow Safari" could be arbitrarily long and might overflow the composer row, so I want to check the ChatComposer usage around lines 2875 and 2905 to see how the layout handles that.

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

💭 The provider-supplied option labels are likely fine since buttons wrap and use nowrap styling. I'm noticing that in ComposerPendingApprovalActions, the "acceptAlways" decision falls into a default styling branch without the text-foreground treatment, which could be a minor UI consistency issue when providers advertise an "Always allow" option. The mobile default options omit "cancel" while web includes it, but mobile is out of scope for this review. For the appName truncation styling, max-w-32 with shrink and truncate should behave correctly without needing min-w-0 since the max-width constraint handles the shrinking. I'm also worried about duplicate React keys if a provider sends options with the same decision value, since the filter logic doesn't dedupe — this could be a real rendering risk since options are provider-supplied, though built-in generators like describeMcpElicitation produce unique decisions, so I want to check derivePendingApprovals in session-logic to see if uniqueness is validated. The acceptAlways decision type is already handled fine in ProviderApprovalDecision, so no gap there. I'm now considering whether requestKind is exhaustively handled somewhere in the codebase.

4. git_tools — requestKind

💭 This grep output seems mismatched — it looks like it's pulling from the main branch rather than the PR head I intended, since the requestKind values and panel logic don't align with what the diff claims for that line. I should re-run the grep with the explicit head commit sha to make sure I'm inspecting the right revision rather than relying on the URL tree ref, which may be getting ignored.

5. git_tools — requestKind

💭 I'm noticing that extractWorkLogRequestKind in session-logic.ts falls back to requestKindFromRequestType, which now returns "mcp-elicitation" for mcp_elicitation_approval requests, meaning work-log entries can carry that new requestKind. I should check how MessagesTimeline.logic handles this. requestKind for icon/label decisions—if mcp-elicitation entries aren't handled there and default to something like "file change," that would be a UI inconsistency between the timeline and composer panel introduced by this change.

6. browse_code — apps/web/src/components/chat/MessagesTimeline.logic.ts

💭 Mcp-elicitation entries would default to "other," which seems acceptable rather than mislabeled. Now I want to check whether MessagesTimeline renders approval buttons somewhere in the timeline that also need the same option handling, so I'll search for onRespondToApproval usage in the web codebase.

7. git_tools — onRespondToApproval|respondToApproval|acceptForSession

💭 I should also check whether ChatComposer/ChatView has keyboard shortcuts that assume fixed approval decisions, like a hotkey sending "acceptForSession" even when the provider's option set doesn't include it. I'll grep for "accept" in the web source to check for this inconsistency.

8. git_tools — "accept"

💭 No keyboard shortcuts, good. I'm now checking the styling logic for approval action buttons—specifically whether "acceptAlways" gets the same visual emphasis as "accept." It seems inconsistent that accept gets emphasized styling while acceptAlways doesn't, even though both are affirmative actions. Then I move to the mobile PendingApprovalCard component, though that's out of scope, and it has the same treatment. I check the collapsed panel layout and truncation styling, noting that the app name uses truncate without a title attribute, which could hide the full name from users if it overflows—though the accompanying detail message likely already includes the full name. I also look at how options flow through the ChatComposer props and how session-logic filters them by validity, without capping the count. I confirm the micro button size variant exists from prior usage. Then I spot a potential React key collision issue: since options are keyed by decision, a malformed provider payload with duplicate decisions could cause key collisions and rendering warnings, which seems like a legitimate correctness concern since the data is externally supplied and unvalidated. Now I'm weighing whether that's a substantive enough finding versus checking a more concrete layout issue: whether long provider-supplied labels could overflow the flex-wrap composer container if buttons use whitespace-nowrap without shrinking, so I want to check the button component's base classes to confirm.

9. git_tools — micro|whitespace|shrink|const buttonVariants|ghost-muted

💭 Since the buttons use shrink-0 and whitespace-nowrap without truncation or max-width, a long provider-supplied label could overflow the composer's width in that flex-wrap button row. I'm checking whether ProviderApprovalOption.label enforces a max length, and it looks like it's just a TrimmedNonEmptyString with no upper bound, so unbounded label lengths could cause horizontal overflow. This mirrors the appName issue we flagged earlier, which was fixed with max-w-32 and truncate — so I want to recommend the same fix here for consistency: capping label width with a max-w class plus truncate (and a title attribute for full text on hover), or clamping the string length at the source. I'm also thinking through whether truncate works properly on the Button component given its whitespace-nowrap and shrink-0 base styles, since truncate needs a bounded width to actually clip text with ellipsis. I also considered whether the duplicate key={option.decision} warrants a separate finding, but traced that options are constructed via a Map in describeMcpElicitation, making duplicates unlikely from legitimate payloads—so I'll skip it to keep the report focused on the higher-confidence issue. I'm also weighing whether to look at the collapsed mobile view of the mcp-elicitation panel next. One more real edge case worth flagging: session-logic passes activePendingApproval.options through, and if a provider's options filter down to just a single valid decision like accept, the user would lose any way to explicitly decline or cancel the request, since the composer only renders whatever's in that array. Actually the label comes straight from the option title unvalidated, so a trailing-space title could fail schema encoding on the server side rather than in web. Still, in the web scope, the visible risk is that a filtered option silently disappears from the approval UI with no fallback — potentially even dropping the Decline choice — so I should flag that the code needs a safe default rather than silent omission. I'm settling on two review comments: one about the approval action buttons overflowing with long provider labels, and one about the options filter in session-logic.ts possibly dropping Decline/Cancel if any entry is invalid, arguing for an all-or-nothing fallback to defaults. I want to double-check the strength of the overflow finding—confirming that labels do come from arbitrary MCP server data (option.label from field.oneOf titles or enumNames), so unbounded length is a real, server-controlled input worth flagging. I'm pinpointing the exact file and line to anchor the comment—ComposerPendingApprovalActions.tsx around line 50 where option.label renders inside the Button. For the fix, I'm wrapping the label text in a span with a max-width and truncate class so long labels get cut off with an ellipsis instead of breaking the layout, while keeping the existing indentation and button structure intact. I'm now leaning toward marking the review as a failure since there's a legitimate layout regression risk with long labels, even though it's a single moderate finding—the check seems to default to failure whenever something concrete needs fixing.

10. modify_pr — One finding on the web approval controls; see the inline comment.