Repository navigation
fix(desktop): show saved gateways without opening Settings - #119727
Merged
Merged
Conversation
૮ >ﻌ< ა ci reviewran on db7558a — test(desktop): wait for the endpoint form before editing it debug infoCI timingsCI timings · View report · View jobWall time 6m10s vs 5m11s (+19.0%). 7 job(s) slower, 6 faster, 1 unchanged.
|
OutThisLife
enabled auto-merge (rebase)
September 23, 2026 01:56
arkheioncorp
left a comment
There was a problem hiding this comment.
Review: fix(desktop): show saved gateways without opening Settings
Verdict: APPROVE
Findings
- Sound refactoring. Moving connection registry lifecycle from
ConnectionSwitcherinto a dedicateduseConnectionsRegistryhook is the right decomposition — the component should paint a cache, not own IPC retry/backoff logic. - Boot lifecycle preserved. The hook correctly defers
initializeConnectionsRegistryuntilboot.runningis false, and skips it for peer/auxiliary windows — same semantics as the old effect, just relocated. - Retry/backoff is correct. Two fixed delays (1s, 2s) then warn-on-focus is reasonable; the "requested while pending" queue prevents concurrent refresh storms when Electron pushes
onChangedduring an in-flight read. - Test coverage. The new hook tests exercise the boot-gated init, peer/auxiliary early return, and the stuck-IPC + focus-recovery path. The gutted component tests (boot-phase, peer, auxiliary) are redundant with the hook tests — the remaining component test still validates the repaint-on-registry-change behavior.
- Mode context extension. Adding
connectionCounttoModeContextand using it forprofileRailVisibleis consistent with the existingprofileCountpolicy. The controller subscription is the right wireup point. - Documentation updated. The multi-connection doc now accurately describes that the registry loads at startup without Settings and that loading ≠ connecting.
Minor notes (non-blocking)
- The rewritten "repaints the menu after a connections.save without reload" test no longer exercises the Electron
onChangedIPC path directly — that path is covered byuse-connections-registry.test.tsanduse-gateway-boot.test.ts, so coverage is not lost, just relocated.
No security concerns, no logic errors. Approve.
10 of 19 tasks
10 of 19 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Loads the saved gateway registry during renderer startup so the sidebar shows the registered gateways without opening Settings. Previously, the optional status-bar
ConnectionSwitcherowned the initial read and registry-change subscription. When that control did not mount, the registry stayed null and the fleet rail never started enumeration.The always-mounted gateway lifecycle now owns those reads. Simple mode also keeps the existing rail visible for multiple registered gateways, even when the active gateway has only its default profile.
Related Issue
Fixes #101257.
Related alternative: #101529 loads the registry from
ProfileRailon mount. This PR uses the boot owner instead, covering hidden rails and status bars, ongoing registry changes, and failed initial reads. The contributor's branch is unchanged.Type of Change
Changes Made
use-connections-registry.tsowns initial hydration and the registry-change subscription, with two delayed retries and a focus retry after failure. Reads are serialized; changes received during a read request another snapshot. Healthy registries are not polled.use-gateway-boot.tsmounts that lifecycle. Source restoration still waits for primary boot reads and excludes peer/auxiliary windows.ConnectionSwitcheronly consumes the published state.store/connections.tsbounds registry IPC reads at five seconds and preserves the last good cache if a read times out.store/interface-mode.tsandapp/contrib/controller.tsxinclude saved gateway count in Simple mode's existing rail visibility policy. No styling changes.How to Test
Verification performed on macOS:
Targeted command, from
apps/desktop:npm exec --offline -- vitest run --project ui \ src/app/gateway/hooks/use-connections-registry.test.tsx \ src/app/gateway/hooks/use-gateway-boot.test.tsx \ src/store/connections.test.ts \ src/store/interface-mode.test.ts \ src/app/chat/sidebar/connection-switcher.test.tsx \ src/app/chat/sidebar/profile-rail-fleet.test.tsx \ src/store/fleet-roster.test.tsChecklist
Screenshots / Logs
The isolated browser checks rendered all three fixture gateway groups with zero mounted status bars and no Settings opened. No runtime exceptions occurred. External network access was blocked; the existing Google Fonts request produced the expected blocked-resource console error.