From fe615a0099fa94ae6621fadbb85944fc613248d5 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 26 Aug 2026 04:10:18 -0700 Subject: [PATCH] fix(desktop): republish the connections registry to renderers after every successful save (#95393) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live-confirmed on the Phase B build: hermesDesktop.connections.save() succeeds and the registry on disk gains the row, but the switcher menu (fed by the renderer $connectionsRegistry snapshot) keeps painting the stale list until reload. remove() already broadcasts hermes:connections:changed; save() only did so on the dial-material-edit branch, so a brand-new connection or a label rename never reached the switcher's onChanged re-pull (or any other window). Fix at the publish seam only: saveRegistryConnection now broadcasts a new 'saved' reason for every successful save that isn't a dial-material edit. 'saved' is a pure registry-refresh signal — the use-gateway-boot listener explicitly ignores it (nothing moved, so no dispose/redial/forget), while the switcher's existing onChanged listener re-pulls the snapshot. Tests: - electron/hardening.test.ts pins both broadcast branches in saveRegistryConnection (source-assertion pattern; main.ts has no exports). - connection-switcher.test.tsx mirrors the live repro scenario (/tmp/mg-ab/w2_95393.py): menu before save lacks the row, Electron's 'saved' push arrives, menu after — without reload — shows it. --- apps/desktop/electron/hardening.test.ts | 28 +++++++ apps/desktop/electron/main.ts | 10 ++- .../chat/sidebar/connection-switcher.test.tsx | 75 +++++++++++++++++++ .../src/app/gateway/hooks/use-gateway-boot.ts | 8 ++ apps/desktop/src/global.d.ts | 2 +- 5 files changed, 121 insertions(+), 2 deletions(-) diff --git a/apps/desktop/electron/hardening.test.ts b/apps/desktop/electron/hardening.test.ts index e6ffdf5175..aa2cdfc030 100644 --- a/apps/desktop/electron/hardening.test.ts +++ b/apps/desktop/electron/hardening.test.ts @@ -1017,3 +1017,31 @@ test('sanitizeDesktopConnectionConfig exposes secureTokenStorage and remoteToken assert.match(returned, /\bsecureTokenStorage\b/, 'the renderer needs the secure-storage availability signal') assert.match(returned, /\bremoteTokenPlainText\b/, 'the renderer needs the plain-text token signal') }) + +// #95393: connections.save succeeded but the switcher menu (renderer +// $connectionsRegistry snapshot) never refreshed until reload. The registry +// push (broadcastConnectionsChanged) fired only on the dial-material-edit +// branch, so a brand-new connection or a label rename never reached other +// windows — or the switcher's onChanged re-pull. Mirrors the live repro at +// /tmp/mg-ab/w2_95393.py: save → menu (no reload) must include the new row. +test('saveRegistryConnection republishes the registry to renderers on EVERY successful save (#95393)', () => { + const source = readMain() + const fnStart = source.indexOf('async function saveRegistryConnection(') + assert.notEqual(fnStart, -1, 'saveRegistryConnection must exist in main.ts') + const fnEnd = source.indexOf('\nasync function ', fnStart + 1) + const body = source.slice(fnStart, fnEnd === -1 ? undefined : fnEnd) + + // The dial-material edit branch keeps its dispose+redial semantics… + assert.match( + body, + /broadcastConnectionsChanged\(\{ connectionId: entry\.id, reason: 'updated' \}\)/, + 'a dial-material edit must still push the dispose+redial signal' + ) + // …and every OTHER save (new connection, label rename) must still push a + // registry refresh, or the switcher menu paints stale until reload. + assert.match( + body, + /broadcastConnectionsChanged\(\{ connectionId: entry\.id, reason: 'saved' \}\)/, + 'a non-dial-material save must republish the registry snapshot (#95393)' + ) +}) diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index 23296814e9..b0db64b3f0 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -9088,6 +9088,14 @@ async function saveRegistryConnection(input: any = {}) { if (existing && connectionDialFieldsChanged(existing, entry)) { await stopRegistryConnectionBackends(entry.id) broadcastConnectionsChanged({ connectionId: entry.id, reason: 'updated' }) + } else { + // Every OTHER successful save (a brand-new connection, a label rename) + // must still republish the registry snapshot, or windows that didn't + // perform the save — and the switcher menu fed by $connectionsRegistry — + // keep painting the stale list until reload (#95393). 'saved' is a pure + // registry-refresh signal: no sockets moved, so listeners must not + // dispose or redial anything for it. + broadcastConnectionsChanged({ connectionId: entry.id, reason: 'saved' }) } return sanitizeRegistryConnection(entry) @@ -10330,7 +10338,7 @@ function sendConnectionApplied() { // scoped to that connection. Without this, a removed remote/cloud source keeps // its renderer WebSocket open and streaming as a ghost, and an edited one // keeps talking to the OLD endpoint until idle-reap. -function broadcastConnectionsChanged(payload: { connectionId: string; reason: 'removed' | 'updated' }) { +function broadcastConnectionsChanged(payload: { connectionId: string; reason: 'removed' | 'saved' | 'updated' }) { for (const win of BrowserWindow.getAllWindows()) { const { webContents } = win diff --git a/apps/desktop/src/app/chat/sidebar/connection-switcher.test.tsx b/apps/desktop/src/app/chat/sidebar/connection-switcher.test.tsx index fa0af2324f..627dcd3bc8 100644 --- a/apps/desktop/src/app/chat/sidebar/connection-switcher.test.tsx +++ b/apps/desktop/src/app/chat/sidebar/connection-switcher.test.tsx @@ -362,4 +362,79 @@ describe('ConnectionSwitcher', () => { expect(screen.getByRole('group', { name: 'Registered gateways' }).getAttribute('aria-busy')).toBe('true') }) + + // #95393: connections.save succeeded but the switcher kept painting the + // stale registry until reload. Mirrors the live repro (w2_95393.py): open + // the menu, save a new connection via the bridge, re-open the menu WITHOUT + // reload — the new row must be there. Electron now pushes a 'saved' + // onChanged for every successful save; the switcher's listener re-pulls the + // snapshot. + it('repaints the menu after a connections.save without reload (#95393)', async () => { + const before = registry([connection('local', 'This device', 'local'), connection('homelab', 'Homelab')]) + const after = registry([ + connection('local', 'This device', 'local'), + connection('homelab', 'Homelab'), + connection('w2-probe', 'W2Probe') + ]) + + $connectionsRegistry.set(before) + + let onChangedCallback: ((payload: { connectionId: string; reason: string }) => void) | null = null + + ;(window as { hermesDesktop?: unknown }).hermesDesktop = { + connections: { + list: vi.fn(async () => after), + onChanged: vi.fn((callback: (payload: { connectionId: string; reason: string }) => void) => { + onChangedCallback = callback + + return () => { + onChangedCallback = null + } + }) + } + } + + // The real refreshConnectionsRegistry re-pulls list() and republishes the + // atom; the mock mirrors exactly that seam against Electron's current + // registry state (before the save, then after it). + let electronRegistry = before + + refreshConnectionsRegistry.mockImplementation(async () => { + $connectionsRegistry.set(electronRegistry) + + return electronRegistry + }) + + try { + render() + + const trigger = screen.getByRole('button', { name: 'Registered gateways: This device' }) + + fireEvent.pointerDown(trigger, { button: 0, pointerType: 'mouse' }) + expect(screen.queryByRole('menuitemradio', { name: 'W2Probe' })).toBeNull() + fireEvent.keyDown(document, { key: 'Escape' }) + + // The save lands in Electron's registry… + electronRegistry = after + // …and Electron's post-save push (reason 'saved' — no dial change) is + // the ONLY signal this window gets. Pre-fix, save never emitted it. + expect(onChangedCallback).not.toBeNull() + ;(onChangedCallback as unknown as (payload: { connectionId: string; reason: string }) => void)({ + connectionId: 'w2-probe', + reason: 'saved' + }) + + await waitFor(() => expect(refreshConnectionsRegistry).toHaveBeenCalledTimes(2)) + + fireEvent.pointerDown(screen.getByRole('button', { name: 'Registered gateways: This device' }), { + button: 0, + pointerType: 'mouse' + }) + expect(screen.getByRole('menuitemradio', { name: 'W2Probe' })).toBeTruthy() + } finally { + refreshConnectionsRegistry.mockReset() + refreshConnectionsRegistry.mockResolvedValue(null) + delete (window as { hermesDesktop?: unknown }).hermesDesktop + } + }) }) diff --git a/apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts b/apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts index 142b8cd43d..8aafeae3ee 100644 --- a/apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts +++ b/apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts @@ -796,6 +796,14 @@ export function useGatewayBoot({ return } + // 'saved' is a pure registry-refresh push (new connection or label + // rename — #95393): no endpoint moved, so there is nothing to dispose, + // redial, or forget. The switcher's own onChanged listener re-pulls the + // registry snapshot for it. + if (payload.reason === 'saved') { + return + } + disposeSecondariesForConnection(payload.connectionId, { redial: payload.reason === 'updated' }) if (payload.reason !== 'updated') { diff --git a/apps/desktop/src/global.d.ts b/apps/desktop/src/global.d.ts index 398eec1b7a..e1fe714bf4 100644 --- a/apps/desktop/src/global.d.ts +++ b/apps/desktop/src/global.d.ts @@ -180,7 +180,7 @@ declare global { // materially edited so the renderer can dispose (and re-dial) the // secondary gateways scoped to it. Optional: older Electron mains // don't emit it. - onChanged?: (callback: (payload: { connectionId: string; reason: 'removed' | 'updated' }) => void) => () => void + onChanged?: (callback: (payload: { connectionId: string; reason: 'removed' | 'saved' | 'updated' }) => void) => () => void } sshConfigHosts: () => Promise sshResolveHost: (host: string) => Promise