Repository navigation
UI Consistency: 2 issues found
Two shared-component consistency issues were found in the new settings UI; both have inline comments on the PR.
apps/web/src/components/settings/ComputerHistorySettings.tsx(L131-133): the exclusion-list entry overridesBadge's core geometry (h-auto,px-2 py-1,font-normal,text-left) to host asize-5SettingIconActionthat does not fit Badge'sh-5.5 sm:h-4.5box;justify-betweenis inert because Badge remainsinline-flex/content-width. Suggested rendering it as a plain removable row instead of bending the primitive.apps/web/src/components/settings/ComputerUseMoreBrowsers.tsx(L8-15):RowTitleis duplicated three times incomponents/settings(also inComputerUseSettings.tsxandComputerHistorySettings.tsx) and the copies have already diverged (extra<span>wrapper in one). Suggested exporting a singleSettingRowTitlefromsettingsLayout.tsx, matching theSettingIconActionextraction this PR already makes.
No regressions were found in accessibility, theming, scroll/mask behavior, or CSS ownership: the PR adds no CSS and does not touch index.css, global selectors, ScrollArea, MenuPopup width defaulting, or theme variants.
Details
Note
Your check run agent prompt is: .macroscope/check-run-agents/ui-consistency.md
More information about how Check Run Agents work can be found in our Help Center.
Scope reviewed (include patterns apps/web/src/**/*.{ts,tsx,css}):
- New:
components/settings/ComputerUseSettings.tsx,ComputerHistorySettings.tsx,ComputerUseMoreBrowsers.tsx,browserBrandIcons.tsx,routes/settings.computer-use.tsx,routes/settings.computer-history.tsx - Modified:
components/settings/settingsLayout.tsx,settingsSearch.ts,SettingsSidebarNav.tsx,components/chat/ComposerPendingApprovalPanel.tsx,MessagesTimeline.tsx,session-logic.ts,routeTree.gen.ts
Checks performed:
- No raw
<button>/<input>/<select>/<textarea>in the new settings files; all controls usecomponents/uiprimitives (Button,Switch,Input,Badge,Dialog*,Collapsible*).<img>brand icons are legitimate image targets, not reconstructed primitives. - Prop contracts verified against the primitives:
Input.onValueChange(Base UI),DialogPopup.showCloseButton,SettingsRow.status/children,Buttonsize="icon-micro"+variant="ghost-muted",Badgevariants. Explicittype="submit"correctly overridesButton's defaulttype="button"throughmergeProps. - Accessibility/interaction: every
Switchand icon action carries anaria-label; the collapsible trigger renders throughButton(keeping focus ring, cursor, and coarse-pointer hit target) and wiresaria-controlsto the panel id. - Desktop-gating consistency:
mirrorToCodexis intentionally not gated onwindow.desktopBridgebecauseapps/server/src/computerHistory/service.tsconsumes it server-side, so this is not a finding. - Settings-search anchors: the new
SETTINGS_SEARCH_ITEMSids/targetIds resolve to renderedSettingsSection/SettingsRowids; sidebar nav entries derive automatically fromSETTINGS_SECTION_LABELSand both new icons are registered. - CSS/Tailwind ownership, theme variants, emitted-CSS selector transforms,
MenuPopupwidth defaulting, and virtualized scroll-fade contracts were not touched by this PR, so those gates were not applicable. Tailwind class ordering differences inComputerHistorySettings.tsxwere deliberately not reported as harmless.
🔍 Agent activity — 1 tool call
💭 I need to narrow down the diff to just the web app files instead of looking at everything. Looking at the key UI files for the computer use feature—ComputerHistorySettings, ComputerUseSettings, ComputerUseMoreBrowsers, browserBrandIcons, and settingsLayout—to understand the new settings components. I'm also checking whether CSS changes were included in the diff, but it looks like the web app changes are focused on TypeScript components rather than stylesheet modifications. Now I need to verify that the shared SettingsRow primitive properly supports the
statusandchildrenprops that ComputerUseMoreBrowsers is passing to it, so I'm examining settingsLayout.tsx to confirm the component's interface. Checking the new settings pages for consistency — SettingsRow accepts children and status, which is good. I'm verifying that searchableSetting returns the right shape (id and title) across ComputerHistorySettings and other pages, and making sure the spread operator works correctly when passing those values to both SettingsRow and SettingsSection components. I'm also scanning for potential issues like nested interactive elements (SettingIconAction inside Badge), hardcoded colors that might not exist in the theme, and whether the Collapsible component is properly set up with aria-controls and matching IDs between trigger and panel. Now checking Button variants likeghost-mutedandicon-microto confirm they're defined, and examining Badge overrides — the className with h-auto, padding, and font-weight changes is a moderate override but probably acceptable since it's a single call site rather than a repeated pattern. Looking at the new settings files to check for any details the diff might have missed. I'm noticing that ComputerHistorySettings.tsx wraps the RowTitle in a span with inline-flex styling, similar to how it's used in ComputerUse with icons, and I'm checking whether some rows are missing searchable IDs. The busy state is only set by clear buttons and properly disables the refresh button, which is fine. However, I'm spotting a potential inconsistency: the "Mirror to Codex" switch doesn't have thedisabled={!onDesktop}check that other Computer History switches have. On non-desktop hosts, toggling this switch is enabled but doesn't actually do anything functional since the bridge is missing. It's arguably an oversight—all other switches on this page disable themselves when not on desktop, so this one breaks that pattern. It's a borderline finding, but the disabled-state inconsistency within the same page is concrete enough to mention. Now I'm checking the Badge component to see if it has any button-nesting or cursor issues, and reviewing how the Input and Button components handle their props—particularly whether Input supports onValueChange and whether the submit button is properly typed. Input supports onValueChange, which is good. Now I'm checking other settings pages to see how they handle desktop-only rows and list items, particularly looking at patterns like shared "Clear" button groups. Let me scan the new files for any raw elements that should be using primitives instead — ComputerHistorySettings looks solid with Button, Input, Badge, Switch, and Dialog, but browserBrandIcons is using raw img elements. ComputerUseSettings also sticks to Button, Switch, and Dialog. I'm noticing some class overrides though: Badge is getting custom height, padding, and typography withh-auto px-2 py-1 font-normal, and there's a DialogPopup with width constraints. The bigger issue I'm catching is inconsistent Tailwind class ordering — ComputerHistorySettings puts color classes first (bg-muted/40 flex min-h-48...), but the rest of the codebase puts them last (mb-1 flex items-start gap-2... text-muted-foreground). This ordering mismatch needs to be fixed to match the repo's convention. Now I'm checking whether the theme tokens likebg-successandbg-warningactually exist in the design system. I'm noticing there's already a ConnectionStatusDot component in the codebase along with provider status styling, and I'm seeing how SettingsRow and other settings components handle status indicators. I should check whether ExtensionStatus is duplicating an existing status-dot primitive or if it needs to be a new implementation. Looking at the git history and patterns in these files, I'm noticing a potential inconsistency: the "Mirror to Codex" toggle in ComputerHistorySettings is missing thedisabled={!onDesktop}check that other desktop-only toggles have, which means it could be activated even when there's no bridge available. The permissions row in ComputerUseSettings spreads the searchable setting properties correctly, and the conditional rendering of the button based oncanOpenworks as expected. In ComputerHistorySettings, the form submission button properly overrides the default button type through props merging. SettingIconAction's hardcoded size and variant get overridden by caller props since they're spread after, which is fine. Now I need to check the Badge primitive to verify the override behavior. Looking at how the Badge component is being used as a container for list-row chips, where the call site overrides most of the base styling—replacing the height, padding, justification, and font weight—to create a different layout pattern than the Badge's original intent. This feels like a real consistency issue: Badge is designed as a compact badge element, but here it's being repurposed as a full-width removable row, which breaks its geometric contract. I should check if there's an existing pattern elsewhere in the codebase for removable chips or list items in settings that might be more appropriate to follow. The Badge override seems like a moderate concern, and I'm noticing the PR includes a good refactor in settingsLayout.tsx that extracts SettingIconAction as a shared primitive. I should check the git history to see if there were earlier review rounds and whether prior commits already touched index.css or the UI primitives. I'm also trying to get a complete picture of all the web files affected by this diff since the output got cut off. Looking at the full diff for apps/web/src, it appears complete and ends with the session-logic.ts changes. I don't see any modifications to index.css, and the desktopBridge typing isn't in this diff—it's likely declared in packages/contracts or the desktop preload layer, which isn't a UI consistency concern. Now I'm diving into ComputerHistorySettings to check for actual UI and behavior issues, starting with how the busy state is being used for the Refresh and clear button disabled states. The timeline items have Reveal/Delete buttons styled as ghost variants in small size, which looks fine. I'm noticing the SettingsSection is using searchableSetting to provide metadata like id and title, though the title gets overridden—I should verify what searchableSetting actually returns to understand the full picture. I'm noticing how the search item usestargetIdto navigate — when someone searches for "Enable Computer History," thetargetIdof "computer-history" points them to the corresponding SettingsSection rather than the search result itself, so navigation usestargetId ?? idto determine the hash destination. For the other settings like "computer-history-privacy" and timeline items, thetargetIdmatches the section id directly, which works cleanly. The computer-use items don't have atargetId, so they navigate to their own row ids instead. I'm checking whether the search navigation properly handles thistargetIdlogic to avoid broken anchors when users click search results. Looking at the sidebar navigation, I need to verify that the new sections appear in the nav items list and that the icon mappings are complete — both look good. Now I'm noticing a potential issue: theRowTitlehelper component is being duplicated across three files (ComputerHistorySettings, ComputerUseMoreBrowsers, and ComputerUseSettings), which violates the pattern of extracting repeated geometry into a shared named primitive. Two of these implementations are identical — both include an icon prop — while the third doesn't. Since they already extractedSettingIconActionas a shared component, I should check whether a similarSettingRowTitleexport already exists in settingsLayout.tsx or if this duplication is genuinely new to the PR. I'm spotting a few duplicates introduced by this PR—isDesktopHost()appears in both ComputerUseSettings and ComputerHistorySettings, andExtensionStatusis defined in ComputerUseSettings but only used there. There's also aSettingsRowwithRowTitlein ComputerHistorySettings that's just wrapping content in inline-flex without adding much value. Let me dig deeper into whether there are any actual behavioral issues here. The "Mirror to Codex" switch isn't disabled off-desktop while all its siblings are, and the IPC call silently no-ops when it's off-desktop—so the toggle appears functional but only writes web settings, which is a concrete problem. There's also an inconsistency in how settings are updated: ComputerUseSettings spreads the full object when updating, whereas ComputerHistory uses a partial patch with a comment warning about clobbering. On the accessibility side, I'm checking whether thearia-controlsattribute being passed manually to the CollapsibleTrigger conflicts with what Base UI's Collapsible already sets automatically—it looks like passing it manually would override the auto-generated value and point to the panel's id instead. I'm noticing that the CollapsiblePanel uses overflow-hidden with height animation, and the child content inside has margin-top styling. The issue is that when measuring the panel's content height via scrollHeight, child margins get included in that calculation, which could affect how Base UI determines the panel's dimensions. Let me look at how other parts of the codebase use Collapsible to see if there's a consistent pattern for handling inner spacing. I'm noticing that other CollapsiblePanel implementations use padding on the inner div rather than margins, but this one applies a top margin to a child element inside an overflow-hidden animated panel. Since overflow-hidden creates a block formatting context, margins won't collapse outward, and when Base UI measures the panel's scrollHeight for the animation, it's capturing that margin as part of the height calculation. The scrollHeight includes margin boxes of descendants in the scrollable overflow region, so the top margin of the first child inside a BFC container contributes to the layout height. This should be fine, though it's a bit of an edge case. I'm going to skip flagging this as a risk. Let me step back and check if there's something more significant I'm overlooking by reviewing the full ComputerUseSettings and ComputerHistorySettings files as they appear in the current version. I want to verify a couple of things, particularly whether theSettingsRowstatusprop is being used correctly and whether it was pre-existing or added by this PR. Looking at the diff, thestatusprop was already present in settingsLayout before this change, so that's good. Now I'm checking the Dialog component to make sure it supports theshowCloseButton={false}prop that's being used in the privacy dialog. The privacy dialog disables the close button to prevent overlap issues. The ComputerHistory privacy dialog wraps its exclusion lists in a ScrollArea with a DialogPanel, where each list has a minimum height constraint. I'm also considering how autoFocus behaves on inputs within dialogs. Looking at my findings, I've identified three issues: duplicate RowTitle components that should consolidate into a shared pattern in settingsLayout, a Badge being repurposed as a full-width list row with custom spacing and font overrides, and the Mirror-to-Codex switch missing a disabled state check for non-desktop hosts. The third one is the most concrete — on non-desktop systems the control remains interactive but only persists to server settings, creating an inconsistency where neighboring Computer History controls are properly disabled. I need to verify whether the setting actually takes effect through the settings patch flow or if it's truly ineffective on non-desktop. For the Badge issue, I'm seeing five style overrides that violate the rule about not overriding a primitive's core properties like height, padding, and text styling — I should flag this as needing either a plain row element styled appropriately or a cleaner Badge variant. The RowTitle duplication clearly matches the pattern rule about consolidating repeated geometry and treatment into a named primitive. Now I'm noticing there are two identical local components in this PR plus a third variant, so I'll flag just the duplicate in ComputerUseMoreBrowsers.tsx and suggest exporting it from settingsLayout instead. I'm also checking whether the section intro paragraphs in ComputerHistorySettings and ComputerUseSettings follow an existing pattern in the repo — looking for how SettingsPanels handles section descriptions to see if there's already a shared component I should reference. The class ordering inconsistency in ComputerHistorySettings doesn't warrant a fix since the linter explicitly allows harmless variations like this. I should verify whether the repo's formatter actually sorts Tailwind classes — if it does, unsorted classes would fail CI rather than being a UI issue. Let me check the theme directory next. Looking at the dark mode handling, the brand icons withalt=""are fine since they're colored logos that work in both themes. TheAgentCursorIconPNG with rounded corners is also fine, and the icon sizing insideRowTitledoesn't need to follow the Button rules. TheSettingsRowtitle renders as an<h3>with an inline-flex span for the icon, which is valid. I also confirmed the "Add app" Button in PrivacyExclusionList hastype="button"and sits outside the form, so that's correct. I'm narrowing down to three potential findings now and need to weigh which ones are worth reporting based on severity. The server also reads that same mirrorToCodex flag when writing memories, so the mirror toggle actually matters even without the desktop bridge—it's not just a UI-only setting. I should verify this by looking at the server's service implementation. The mirrorToCodex feature is server-side, so leaving it enabled off-desktop is fine — that finding doesn't hold up. With just the Badge override and RowTitle duplication remaining as modest issues, I'm wondering if there's something more substantial I'm overlooking. Let me go back through ComputerUseSettings and ComputerHistorySettings more carefully to see what else might be worth flagging. The Badge component defaults to inline-flex, which means it shrinks to fit its content rather than expanding to full width. Even withmax-w-fullandjustify-betweenapplied, there's no free space for the justify-between to work unless the content is long enough to hit that max-width constraint. For shorter items, the badge just stays content-sized. The real issue is that the Badge primitive isn't designed for this use case — it can't accommodate the micro icon button (20px) within its default height constraints, which is why the override withh-autoexists. This suggests the component might be better served by a dedicated chip pattern that's already in the codebase, rather than forcing Badge to do something it wasn't built for. I'm stepping back to assess what I've found so far: the duplicated RowTitle helper and the Badge override both match the rule criteria explicitly. Let me do a final sweep through the new files to see if there's something more substantial — like missing keyboard handling or raw button elements that would be a clearer violation. Now I'm checking the chat-side changes in ComposerPendingApprovalPanel and MessagesTimeline to verify that labels are properly rendered and that aria-labels are applied to the appropriate regions. Added the ShieldIcon mapping for the messages timeline. Now I'm verifying that the new settings pages are accessible through the sidebar navigation — I've added the icons to SettingsSidebarNav and need to confirm it's pulling items from SETTINGS_SECTION_LABELS so the nav links actually render. The navigation derives automatically, which is good. For the settings search, I'm noticing thatsearchableSetting("computer-history-enabled")provides the row title "Enable Computer History" that gets rendered as text, which matches the pattern other rows use with<RowTitle>. However, there's an issue with entries like "computer-use-accessibility" and "screen-recording" — they reference IDs that only appear once permissions load, so the scroll-to-settings behavior might not work as expected. Looking at the findings: I have two moderate issues to consider. The RowTitle duplication is a concrete violation — two identical local components were introduced in the same PR within the same directory, both used with SettingsRow, which violates the principle of preferring named primitives when multiple call sites repeat the same treatment. The Badge issue is about its geometry contract being overridden with custom height, padding, and typography because Badge can't accommodate a 20px icon-micro action. Both are fair, actionable comments worth flagging. For the Badge fix, I'd suggest replacing it with a plain div using the same border and background utilities, or using SettingIconAction outside the badge instead. The simplest approach is swapping the Badge element for a div with matching styling — it's a contained change that removes the geometry mismatch without requiring a cross-file import adjustment. Now I'm weighing the check state: these are consistency issues rather than regressions, but the guidance says the check defaults to failure when findings exist, and only passes when there are no findings at all. Given that I've identified real issues, I should mark this as failure. Let me pinpoint the exact line numbers for the inline comments. For new files, all lines appear in the diff, so I can reference them directly. I need to locate the Badge component in ComputerHistorySettings.tsx around lines 131-133 and the RowTitle component in ComputerUseMoreBrowsers.tsx starting around line 8. I'm checking the RowTitle component in ComputerUseSettings around lines 47-54, which creates a flex container that displays an optional icon alongside the children text wrapped in a span. The RowTitle function appears three times across the directory with subtle differences — this version in ComputerUseMoreBrowsers renders children directly, while ComputerUseSettings wraps it in an extra span. I should consolidate these into a single SettingRowTitle export from settingsLayout.tsx alongside SettingIconAction, then import it everywhere to prevent further drift. For the second comment, the Badge component is being forced to override its core geometry (height, padding, text alignment) because a size-5 SettingIconAction doesn't fit within Badge's standard box. I'll suggest either rendering the removable entry as a plain flex row instead of bending Badge's contract, or moving the action outside the badge entirely to keep Badge's dimensions intact. Before posting these via GitHub, I should check if there are existing review comments on the PR to avoid repeating feedback that's already been addressed. I've confirmed the existing comments are on unrelated files, so my two findings are genuinely new. I'm going to post the review with inline comments targeting the specific lines in ComputerHistorySettings.tsx and ComputerUseMoreBrowsers.tsx, keeping the feedback concise.
1. modify_pr — UI consistency review of the new Computer Use / Computer History settings surfaces. Two consistency findings, both in apps/web/src/components/settings. The rest of the new UI composes the shared primitives (Button, Switch, Input, Dialog, Badge, SettingsRow/SettingsSection) correctly, and extracting SettingIconAction into settingsLayout.tsx is the right direction.
Agent Credits: 393 credits