Repository navigation
UI Consistency: 1 issue found
apps/web/src/components/ProjectFavicon.tsx — The new repository-wide favicon source overrides every row's own icon. selectProjectFaviconSources returns an entry for every known project, so input.faviconPath is only ever used for projects missing from the atom. The selector groups solely by repositoryIdentity.canonicalKey (remote-URL only, identical for every sub-directory of one clone), while the sidebar and Project settings group rows by the logical project key (repository_path grouping and per-project overrides keep sub-directory projects separate, and Project settings writes faviconPath only onto that group's members). Result: distinct rows such as repo/apps/web and repo/apps/server render whichever member the selector picked, and the Project settings preview can show an icon that contradicts the path printed in its own description row. Suggested minimal fix (posted inline): keep a row as its own source when it declares its own faviconPath, since that path is relative to the row's workspace root and cannot be resolved against the source checkout's cwd.
No Tailwind, CSS-ownership, theme-variant, scroll/mask, or shared-primitive violations were introduced by this PR.
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.
Files reviewed in scope: apps/web/src/components/ProjectFavicon.tsx, apps/web/src/components/ProjectFavicon.test.tsx, apps/web/src/state/projects.ts.
Context consulted outside the include patterns: packages/client-runtime/src/state/projectFavicon.ts, packages/client-runtime/src/state/projectGrouping.ts, packages/contracts/src/settings.ts (grouping modes repository / repository_path / separate), apps/server/src/project/RepositoryIdentityResolver.ts (confirming canonicalKey is derived from the git remote URL only and is shared by every sub-directory of a clone), apps/web/src/assets/assetUrls.ts and packages/client-runtime/src/state/assets.ts (confirming the added second useProjectFaviconAsset call resolves to the same value-keyed atom, so it does not duplicate requests), and the ProjectFavicon call sites (Sidebar, ChatHeader, CommandPalette, PullRequestListFilters, ThreadCommandSubtitle, settings/ProjectSettingsPanel).
Checked and found clean:
- No class-string, geometry, radius, focus-ring, or fallback-glyph changes;
ProjectFaviconFallbackand the<img>classes are unchanged. - Hook order is stable (
useProjectFaviconAssettwice plususeSyncExternalStoreall execute before the conditional fallback return). - No global CSS, theme variant,
MenuPopupwidth defaulting, or scroll/mask behavior touched by this PR. - Focused tests were added for the new source-selection and remembered-icon behavior, which satisfies the test gate for a behavior-changing composition change.
🔍 Agent activity — 1 tool call
💭 I'm checking the changed web files for UI consistency concerns, focusing on ProjectFavicon.tsx since this PR is mostly logic-based. I want to verify the hook usage follows React's rules correctly. This check focuses on UI consistency across the shared component system, and this PR doesn't touch CSS or primitives much. I'm considering whether the favicon caching logic change could cause a visual regression—specifically whether
loadedProjectFaviconSrcs.get(cacheKey)might return a stale displayed source when a remembered plan reuses a remembered cacheKey. I'm also checking hook ordering and SSR safety:useSyncExternalStoreis called without agetServerSnapshot, which would throw during server-side rendering, so I want to confirm whether this app is actually SSR or just a client-side SPA. The module-level memory and eviction logic for remembered favicons look fine otherwise. One real concern I'm flagging: every row now issues two asset requests (source and own), and when the source equals the row's own project, that's a duplicate identical request per row—unless the underlying atom family already dedupes by resource key, this could double network/token usage across the UI. A more serious issue is that remembering a row's own live icon under a repository-wide key could leak that icon across all rows in the group, since the memory is keyed by the shared canonical repo key rather than per-project, so a source env's icon could incorrectly appear for other projects sharing that repo. I should also check whetheruseAssetUrlStatededupes repeated calls with the same arguments and whether the web app handles SSR properly, sinceuseSyncExternalStoremight be missing a server snapshot. I'm considering whether using ProjectFavicon in list views creates redundant subscriptions per row, though that's not a UI consistency concern under the rules. I want to check how ProjectFaviconPickerDialog and other consumers use useProjectFaviconAsset, since ProjectFavicon now overrides the input path with the source's faviconPath, which could change behavior for specific paths. This seems intentional per the docs stating projects sharing a repository share one icon, even though a project's own chosen favicon would then be ignored if it's not the source. I'm scanning other call sites for preview mismatches but finding no actual styling or Tailwind/CSS changes in this PR, so I'm leaning toward there being no findings to report, though I need to format the response according to the required schema. I'm digging for a genuine bug within web scope—checking whether callinguseProjectFaviconAssettwice creates duplicate atoms ifcreateUrlkeys by reference rather than value, which could cause leaks or loops, and whether the image key logic handles cacheKey fallback correctly on disconnect whensourceStatechanges. I'm noticing that ProjectSettingsPanel uses ProjectFavicon to preview the project's own configured favicon, so with the new source override, that preview would end up showing the group's icon instead of the project's own setting — creating a mismatch between what's being edited and what's displayed. I should look into this more closely. I'm trying to figure out where faviconPath actually comes from in the settings preview—whether it's a group-level setting applied to all checkouts, or tied to the source project specifically, since that affects whether the preview accurately reflects what gets set. There's a subtlety here: the preview now resolves through the projects list atom using the source's stored faviconPath, rather than the locally-edited prop. That means right after a user picks a new icon, the preview could show a stale image until the projects list refreshes with the updated value, since the optimistic local state is effectively being ignored whenever a source entry exists—which is always the case. The faviconPath is relative to each checkout's workspace root, so since updateAllMembers applies the same relative path to all members, this is consistent and fine — the preview reflects saved data from the representative project, and showing the source environment's rendering is the intended shared-icon behavior. One genuine edge case worth flagging: in ProjectFavicon, when a user resets an icon to Automatic, the remembered entry (keyed by repo canonical key) still holds the old URL/cacheKey. Since the new source briefly enters a Loading state, the fallback plan shows the stale remembered icon momentarily before the live source overwrites it — a minor flash rather than a real regression. I also want to check whether useSyncExternalStore here has a getServerSnapshot, since without one it could crash under SSR if apps/web uses TanStack Start — worth peeking at the package/vite config. Separately, I'm wondering if duplicated ProjectFavicon subscriptions (source vs. own asset URL) in long virtualized sidebar lists cause redundant atom subscriptions when they're identical, which depends on whether createUrl dedupes by value — I should check that implementation in the client-runtime assets state. Since the atom is deduped by value key, double subscription stays cheap, and calling usePreparedConnection twice with different environment ids is fine too. I'm checking the hook order inside useProjectFaviconAsset — the count stays fixed, but I want to make sure useAtomValue behaves correctly when the underlying atom identity changes as the source changes. Actually, I think I've found a more substantive issue:selectProjectFaviconSourcesgroups favicon sources by repository canonical key, but project settings groups might use a different grouping key. If two distinct project groups happen to share the same repository identity — like separate worktrees of the same repo — then one group's favicon setting could silently override the other's, since the icon source is keyed per-repo while the setting is per-group. I want to check projectGrouping.ts to see whether worktrees of the same repo actually end up as separate groups or get merged, since that determines whether this is a real bug. I'm realizing that under the default grouping mode, subdirectories of the same repo like monorepo packages get treated as separate project groups with their own icons and favicon paths, so I need to check how the favicon source selector keys into that. SinceselectProjectFaviconSourceskeys purely byrepositoryIdentity.canonicalKey, which is repo-level and shared across subdirectory projects, all those distinct groups collapse into one favicon source — meaning sidebar rows forapps/webandapps/serverwould all show one arbitrary project's icon instead of their own, a regression from current per-project favicon behavior. This holds even in "separate" grouping mode where rows stay distinct but icons still collapse, and since favicon settings are stored per-project, a user customizingapps/web's icon would see it bleed into other subdirectory projects sharing the same canonical key. This feels like a legitimate UI consistency finding worth flagging: the favicon resolution now ignores subdirectory-scoped project groups and per-project icon settings within the same repository, which is a real visual/behavioral regression in the changed web UI code. I'm realizing the new favicon source doesn't respect grouping settings at all, so under "repository_path" or "separate" modes where users intentionally keep projects distinct, rows sharing the same repo—like monorepo subdirectory projects—would all end up showing the same favicon anyway. This is a real regression in the changed web code, but I'm questioning whether it falls under the scope of this check, which is framed around component system consistency, Tailwind ownership, and visual preservation rather than product logic. The enumerated rules don't cleanly cover this kind of behavioral regression, so I'm weighing whether to flag it under "visual preservation" broadly or treat it as out of scope. Let me verify the mechanics: since ProjectSettingsPanel updates all member projects sharing a repository_path group, and selectProjectFaviconSources picks only one source project per canonical key, every physical project mapped to that key inherits the same icon regardless of its own faviconPath — so distinct sidebar rows for different projects can end up showing the wrong, shared icon. This is a concrete, user-visible bug introduced by the changed logic, not a pre-existing issue, which pushes me toward reporting it despite the ambiguous fit with the listed rules. I'll place the inline comment on the atom construction in projects.ts, since that's the changed, in-scope line. I'm also weighing whether ProjectFavicon.tsx's handling ofinput.faviconPathdeserves its own comment, sincesourceis non-null for every known project and only unknown ones fall back — meaning that prop path may effectively be dead code worth flagging separately. I need to confirm the diff line numbers land on the added lines 38-45 on the right side, and double-check mobile is out of scope and the test file changes are fine. I'm leaning toward marking this a failure since it's a consistency/regression risk, though I pause to consider whether this might actually be an intentional product decision rather than a bug. The docs describe the intent as sharing an icon when the same repository is checked out on multiple machines, not merging different subdirectories of the same clone on one machine. Since canonicalKey is derived purely from the repository-level identity, it would also merge subdirectory projects within a single checkout, which goes beyond that stated intent — so I still think this is a real scope bug. I want to verify how canonicalKey is actually derived to confirm it's repo-level only. Right, so monorepo subdirectory projects sharing a canonicalKey get collapsed into one icon even when the user has set "repository_path" or "separate" grouping, which should show distinct rows with their own favicons per subdirectory project. That override gets ignored, and even in the default "repository" grouping mode, distinct subdirectory projects would already be merged into one group regardless. I'll flag this as a single finding, suggesting the icon source selection key off the same logical project key used for grouping, or fall back to the row's own configured favicon when available. Actually, the faviconPath is likely relative to the project's own cwd, so an explicit per-project path needs to pair with that project's own cwd rather than the group's shared source. The real fix is to treat a row as its own source whenever it defines an explicit faviconPath, rather than inheriting the group's source project. I'll draft the review comment targeting lines 41-45, noting that every project in the atom gets a source entry, so the explicit faviconPath should only be used when paired with its own cwd. I'm adjusting the fix so a row with its own declared favicon path skips the group source entirely, since that path is relative to the row's own workspace root rather than the group's checkout. This makes iconKey fall back to physicalKey for such rows, giving each project its own memory entry, which is the desired behavior. That looks solid to post.
1. modify_pr — One finding: the new repository-wide favicon source ignores the app's project grouping settings and the per-row faviconPath, so sub-directory projects of one clone can render each other's icons. Details inline.
Agent Credits: 306 credits