Repository navigation
UI Consistency: 1 issue found
apps/web/src/components/settings/SettingsSidebarNav.tsx — the new searchableItems memo shows the wsl-backend search result whenever desktopWsl.error !== null, even when a stale snapshot is still present. Because desktopWslStateAtom is Atom.swr + Atom.keepAlive and an AsyncResult.Failure retains previousSuccess, useEnvironmentQuery can report data and error together. On Windows desktop with WSL unavailable and unused, a failed revalidation then surfaces a "WSL backend" result while ConnectionsSettings.renderWslRow() takes the loaded-state path and renders nothing — the same dead-anchor navigation (no scroll/focus/pulse, #wsl-backend left in the URL) this PR sets out to prevent. Suggested fix posted inline: derive the error branch only when there is no snapshot, mirroring renderWslRow, and ideally share the predicate so the two copies cannot drift.
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.
Verification performed
- Compared the sidebar's visibility rule against every branch of
ConnectionsSettings.renderWslRow()(apps/web/src/components/settings/ConnectionsSettings.tsxL2762–2900): no-state + error → retry row; loaded +available→ picker; loaded +!available+ (enabled||wslOnly) → recovery row; otherwisenull. - Confirmed
canManageLocalBackendis always true under Electron (currentSessionScopes = desktopBridge ? AuthAdministrativeScopes : …) and thatisElectronis exactlywindow.desktopBridge !== undefined, so the sidebar'sisElectrongate matches the panel'sdesktopBridgegate — the earlier permission-gap concern does not apply. - Confirmed
AsyncResult.FailurecarriespreviousSuccessandAsyncResult.valuereads it (vendored effect source), and thatapps/web/src/state/query.tsderivesdataanderrorindependently, so both can be non-null;Atom.keepAliveondesktopWslStateAtomkeeps the node alive so retention holds. - Re-checked the previously flagged items: the
useMemodependency list now includesdesktopWsl.error(fixed in4cb1b3f), anddesktopOnly+windowsOnly+ the atom-based filter cover the browser, non-Windows desktop, and Windows-without-WSL cases.
Not flagged
searchSettingsreadingnavigator.platformmatches existing repo convention (ConnectionsSettings,useTrayRunningCountSync,windowControlsOverlay).- The extra
getWslState()probe on non-Windows desktop settings mounts is shared through the same SWR atom and is not a visual or interaction regression. - Catalog/test additions (
settingsSearch.ts,settingsSearch.test.ts) and thesearchableSetting("wsl-backend")spreads inConnectionsSettings.tsxpreserve the existing anchor/title contract.
🔍 Agent activity — 22 tool calls
💭 I want to check the current state of the code at head 4cb1b3f, since prior runs flagged issues at commits 8ae1c9b, 9bb587f, and e8d6709 -- let me see if those were addressed.
1. browse_code — apps/web/src/components/settings/settingsSearch.ts
2. browse_code — apps/web/src/components/settings/SettingsSidebarNav.tsx
💭 I want to verify whether the head commit at 4cb1b3f actually differs from the earlier flagged commit e8d6709, since the diff still shows the memo depending on
desktopWsl.error, contradicting the earlier note that it was missing from the dependency array.
3. git_tools — 9b2d043..4cb1b3f
4. git_tools — isWindowsPlatform
💭 The latest commit 4cb1b3f appears to be a direct response to my earlier comment at e8d6709, so I want to look at its diff to see what changed.
5. git_tools — 4cb1b3f
6. git_tools — e8d6709
💭 The author addressed the previous finding, so now I need to verify correctness. I want to check what useEnvironmentQuery returns for error—if it defaults to undefined rather than null when there's no error, the WSL item's filter check would always be true, defeating the filter entirely. I also need to check whether desktopWslStateAtom is per-environment and how the row rendering logic handles it.
7. git_tools — useEnvironmentQuery
8. browse_code — apps/web/src/state/desktopWslState.ts
💭 I'm confirming the error check is correctly typed and dependencies are updated. Tracing through the non-Electron and Windows desktop cases to verify the memo filters items correctly based on data/error availability, then moving to check how ConnectionsSettings renders the WSL row.
9. browse_code — apps/web/src/components/settings/ConnectionsSettings.tsx
💭 I'm noticing a divergence: ConnectionsSettings only renders the WSL row in the error case when both
desktopWslErrorandcanManageLocalBackendare true, but SettingsSidebarNav's memo shows the item whenever there's an error, regardless of the permission check. I need to trace whererenderWslRowis called and whether there's a Windows-platform gate, and confirmdesktopWslStatecomes from the same atom via useEnvironmentQuery. Since the search's windowsOnly filter already excludes non-Windows via navigator.platform, the remaining mismatch scenario is a Windows desktop with an error butcanManageLocalBackend === false— I need to find how that flag is actually computed.
10. git_tools — canManageLocalBackend|desktopWslError|renderWslRow|desktopWslState =
💭 I'm noticing an inconsistency: ConnectionsSettings gates the atom subscription behind
canManageLocalBackend && desktopBridge, but SettingsSidebarNav only checksisElectronwithout that same permission gate, so on Windows desktop without proper authorization the nav could still be subscribing when it shouldn't. This means search could return "WSL backend" as a live result even when the row is hidden due to insufficient scope, creating a dead anchor — the same failure mode regardless of whether it stems from a resolved state or a thrown error, since the root cause is the missing scope gate in the sidebar's subscription logic. There's also a more significant side effect worth flagging: subscribing to the WSL state atom unconditionally in the sidebar means it now fires a WSL IPC call on mount for every Electron user, including those without write scope and non-Windows desktops where WSL is irrelevant, whereas before it was only queried when the user had write scope and opened Connections. The clean fix would be gating that atom subscription behind both platform (Windows) and permission checks. Weighing whether this overlaps too much with the earlier windowsOnly comment, I conclude it's a distinct condition — the remaining gap after that fix is the permission gate — and worth reporting since it's a concrete behavioral regression. I want to verify how the sidebar derives currentSessionScopes and whether useEnvironmentQuery has access to scope info there, so let me check that before finalizing the fix suggestion.
11. git_tools — currentSessionScopes
💭 I'm noting that since desktopBridge existing forces AuthAdministrativeScopes, canManageLocalBackend is always true on Electron desktop, making the earlier permission mismatch irrelevant there. Now I want to verify whether isElectron is actually equivalent to window.desktopBridge existing by checking the env setup.
12. browse_code — apps/web/src/env.ts
💭 Good, isElectron and desktopBridge align, so canManageLocalBackend is always true in Electron and there's no permission mismatch. I'm now noting that on non-Windows desktop, the sidebar still subscribes to desktopWslStateAtom and fires a getWslState() IPC call on every mount even though windowsOnly filtering hides the search item anyway—wasteful, though I need to check if it's a real concern. Since the atoms are shared via SWR caching, this shouldn't duplicate the query ConnectionsSettings already makes, but I want to confirm getWslState is cheap on macOS/Linux rather than spawning processes like it does on Windows. I'm also checking the memo condition around desktopWsl.error and cross-referencing renderWslRow's visibility logic against the search item's own conditions to see if they truly line up. Both the row and search item hide during the initial loading state and only differ in a minor transient race where typing "wsl" right as settings mount might miss the item briefly—an acceptable edge case since previously the item didn't exist at all. Dependency arrays and Windows-specific gating look fine, so I'm moving on to check the rules-of-hooks compliance next. I want to double-check the added test file to make sure the assertion that
searchSettings("wsl")returns empty is coherent, particularly whether it depends on mockingnavigator.platformcorrectly.
13. browse_code — apps/web/src/components/settings/settingsSearch.test.ts
💭 I'm confirming the word-wrap entry correctly routes to the appearance settings page in the catalog. I also notice there's no existing test coverage for the windowsOnly filtering logic or the sidebar memoization, and that filter reads platform info directly rather than through an injectable source, which could make it harder to test reliably. I'm checking whether this deprecated platform-detection approach matches existing conventions elsewhere in the codebase, and it does—other files use the same pattern, so it's consistent. I'm now tracing through a scenario where WSL isn't available and isn't enabled to confirm the sidebar correctly filters that row out, and thinking about whether toggling WSL on properly triggers a re-render through the atom. I'm noticing a possible redundancy: the visibility logic for WSL-only settings might now be duplicated between the search filter and the sidebar's atom-based filter, while the actual source of truth lives in the connections settings render logic. That's worth flagging as an ownership concern, so I want to grep for other consumers of
searchSettingsto see how widely this logic is relied upon.
14. git_tools — searchSettings|SETTINGS_SEARCH_ITEMS
15. browse_code — apps/web/src/lib/utils.ts
💭 I'm checking that the Windows platform detection regex correctly matches "Win32" from navigator.platform, which it does. Then I'm looking at how the sidebar's atom-based gating handles environment routing for shared renderers, specifically examining the WSL state query hook. The real concern I'm noticing is that the sidebar duplicates ConnectionsSettings' visibility logic — the same conditions as renderWslRow — creating a drift risk if one changes without the other. This is a legitimate ownership finding, though it's borderline between a concrete regression and a design preference, so I need to check whether there's an actual current mismatch between the two condition sets before deciding whether to flag it. Comparing the render conditions carefully, I trace through each state case (null with error, non-null without availability, available) and confirm the sidebar and row logic match in all these branches. The remaining question is whether the row lacks the platform gate that the sidebar has — I need to verify whether
availablecan ever be true on non-Windows platforms, or whether persistedenabled/wslOnlysettings could cause the row to render somewhere the sidebar wouldn't. There's a subtler risk though: withAtom.swr, a failed refresh might retain stale data instead of nulling it out, which could cause the row to hide (data present but unavailable) while the sidebar still shows the item because its error flag remains set — a potential mismatch between the two. Right,Result.valuein @effect-atom/atom does return the previous success value even when the current state is Failure, since Failure can carrypreviousSuccess. That confirms the code path where data coexists with an error - I can see the component's rendering logic actually accounts for this, displayingdesktopWslErrorinside branches where data is also present, which validates that this dual-state scenario is expected and handled. The bug I found is that the OR-branch condition treats an error as making the row visible regardless of whether data exists, but this reintroduces the dead-anchor problem the PR is meant to fix. The fix should only fall back to checking the error when data is null, mirroring how renderWslRow already does it - I'm restructuring the searchableItems computation to check wslState first, and only consult the error field when wslState is null. I want to confirm this "keeps previous data alongside error" behavior is actually how AsyncResult works here, so I'm searching the repo for related patterns like isPending checks paired with null data, and looking for any comments mentioning preserved previous values.
16. git_tools — previousSuccess|previous value|stale value|keeps the previous
17. git_tools — error persists across an in-flight|The error persists
💭 I'm not certain whether AsyncResult.value persists on Failure, so I want to check the effect library's AsyncResult source or any repo tests that construct a failure state carrying a previous value.
18. git_tools — AsyncResult.(failure|Failure|value|success)
19. git_tools — isPending &&|.isPending
💭 I want to check desktopWslState.test.ts to confirm it verifies that a failed refresh preserves the previous stale-while-revalidate value.
20. browse_code — apps/web/src/state/desktopWslState.test.ts
💭 I'm looking at how ConnectionsSettings combines
desktopWslMutationErrorwithdesktopWsl.error, since mutation errors can coexist with data but load errors might behave differently. The comment about keeping a recovery row when there's no desktopWslState suggests load failures follow a distinct pattern from mutation failures. I'm now thinking through theResult.valuesemantics in effect-atom: for aFailurevariant, it likely falls back to the previous success's value if one exists, otherwise returns none, meaningAtom.swrand similar constructs preservepreviousSuccesson transition to failure so stale data can still render alongside an error. That gives a plausible bug path: on a Windows desktop where WSL is unavailable and unused, a stale-while-revalidate failure after 30s (due to a bridge error) would keepdatapopulated whileerrorbecomes non-null, causing the sidebar's memoized list to include the "WSL backend" item since the filter didn't account for that error state — leading to a dead anchor when the user searches "wsl" and clicks it, sincerenderWslRowstill returns null for the unavailable/unused case. I'm also connecting this to the earlier fix where the author added|| desktopWsl.error !== nullto the condition, likely in direct response to this exact scenario I flagged before. Rather than treating this as a new edge case worth blocking on, I'm realizing the more compelling fix is addressing the root duplication: the sidebar re-derives WSL row visibility from raw state fields instead of sharing logic withrenderWslRow, so the two will keep diverging. The cleanest solution is extracting a shared predicate function into a common module that both the sidebar filter and the row renderer can import. I'll skip flagging the minor non-Windows IPC call to avoid piling on comments, and instead post a single focused inline comment on the memo lines about this divergence with a suggested fix, once I verify the exact diff line numbers for placement. Finishing the searchableItems logic... Before posting, I want to verify that AsyncResult.value retains the previous success state — if that assumption is wrong, the whole finding falls apart, so I should check the effect package version in node_modules. I searched for "previousSuccess" but found nothing, so I'm checking package.json for the effect version and any patches. I'm also looking at similar patterns elsewhere in the codebase — like the pull-requests loading-more check and the branch selector's initial-load pending check — but those handle pending states, not failure states with stale data. I want to find a place that explicitly handles data and error coexisting from useEnvironmentQuery, so I'm searching around ProjectContentSearchDialog and grepping for comments about stale data alongside errors. I'm considering whether desktopWsl's error check is a solid unconditional case, but that one resolves to null when not in Electron, so it doesn't prove the divergence claim. I need to find a genuinely unconditional example where the sidebar's query isn't gated the way ConnectionsSettings is.
21. browse_code — In this repo (t3code), the web app uses effect/unstable/reactivity AsyncResult with Atom.swr. Question: when an atom that previously succeeded is refreshed and the new load FAILS, does the resulting AsyncResult Failure retain the previous success value such that AsyncResult.value(result) still returns Option.some(previousValue)? Please cite the AsyncResult/Result implementation (e.g. a previousSuccess field or similar) from the installed effect package or any vendored source, or any repo code/tests/comments that demonstrate data being retained alongside an error from useEnvironmentQuery (apps/web/src/state/query.ts). Answer yes/no with evidence.
💭 This confirms the atom keeps state alive, so previousSuccess persists even when error is set, meaning data and error can coexist — and the sidebar's OR-with-error logic diverges from renderWslRow's loaded-state path, which silently ignores error when unavailable and unused. I also note renderWslRow's error-recovery row requires the state to be absent, consistent with my proposed fix, and I should check whether desktopWslMutationError behaves similarly. Now I'm drafting the inline review comment with a suggested code replacement for the searchableItems memo block in SettingsSidebarNav.tsx, matching the existing indentation and dependency array so the diff applies cleanly.
22. modify_pr — One finding: the sidebar's WSL visibility rule now diverges from renderWslRow when a failed revalidation keeps stale state, which re-opens the dead-anchor case this PR is closing.
Agent Credits: 249 credits
Token usage: 60 uncached input · 1,432,131 cache-read input · 164,591 cache-write input · 24,771 output
Agent Credits may also include non-token charges from external tools such as web research.