From 927b4cf22f434b35153d0b15e4906089730afd2b Mon Sep 17 00:00:00 2001 From: pierrenode <298902573+pierrenode@users.noreply.github.com> Date: Tue, 11 Aug 2026 21:32:51 +0300 Subject: [PATCH] fix(desktop): guard soft gateway-switch against concurrent invocation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit connection-config:apply's global/primary branch (apps/desktop/electron/ connection-apply.ts::applyConnectionChange) tears down the window backend and notifies the renderer via hermes:connection:applied, which drives softSwitch() in useGatewayBoot. Two independent UI triggers (Settings "Apply" and the cloud-agent "Connect" button) each have their own pending-state guard but don't know about each other, so both can call applyConnectionChange back-to-back for the primary scope. Without protection: a second teardownPrimary() races the first's still-pending process exit, fires a second hermes:connection:applied, and the resulting second softSwitch() races the first through getConnection()/adoptPrimaryProfile() with no guard against it. Fix: applyConnectionChange dedupes concurrent primary-scope calls through a single in-flight promise, so a second caller awaits the first's re-home instead of racing its own. softSwitch() gets an early $gatewaySwitching reentrancy check as defense in depth — the flag is already set for the whole switch, just never checked at entry. No behavior change for the common single-call path. --- .../desktop/electron/connection-apply.test.ts | 55 +++++++++++++++++++ apps/desktop/electron/connection-apply.ts | 28 +++++++++- .../gateway/hooks/use-gateway-boot.test.tsx | 26 +++++++++ .../src/app/gateway/hooks/use-gateway-boot.ts | 7 ++- 4 files changed, 113 insertions(+), 3 deletions(-) diff --git a/apps/desktop/electron/connection-apply.test.ts b/apps/desktop/electron/connection-apply.test.ts index ccf697a9284b..5420dd43d45a 100644 --- a/apps/desktop/electron/connection-apply.test.ts +++ b/apps/desktop/electron/connection-apply.test.ts @@ -64,6 +64,61 @@ describe('applyConnectionChange', () => { }) expect(events).toEqual(['cancel:worker', 'ssh:worker', 'pool:worker']) }) + + it('dedupes two concurrent primary-scope calls into one teardown + apply', async () => { + // Reproduces the Settings "Apply" button and the cloud-agent "Connect" + // button firing back-to-back: each is an independent UI trigger with its + // own pending-state guard, so nothing stops both calling + // applyConnectionChange for the primary scope close together. Without + // dedup, a second call starts its own teardownPrimary() while the first + // is still waiting on the real process exit. + const gate = deferred() + const teardownPrimary = vi.fn(async () => { + await gate.promise + }) + const sendApplied = vi.fn() + + const first = applyConnectionChange({ + cancelAndWait: vi.fn(async () => undefined), + isPrimary: true, + scope: '', + sendApplied, + stopPool: vi.fn(), + teardownPrimary, + teardownSsh: vi.fn(async () => undefined) + }) + + // Flush enough microtasks for `first` to reach the in-flight teardown + // and suspend on the still-open gate, without resolving it. + for (let i = 0; i < 10; i++) { + await Promise.resolve() + } + expect(teardownPrimary).toHaveBeenCalledOnce() + + const second = applyConnectionChange({ + cancelAndWait: vi.fn(async () => undefined), + isPrimary: true, + scope: '', + sendApplied, + stopPool: vi.fn(), + teardownPrimary, + teardownSsh: vi.fn(async () => undefined) + }) + + for (let i = 0; i < 10; i++) { + await Promise.resolve() + } + // The second call joined the first's in-flight re-home instead of + // starting its own teardown. + expect(teardownPrimary).toHaveBeenCalledOnce() + expect(sendApplied).not.toHaveBeenCalled() + + gate.resolve() + await Promise.all([first, second]) + + expect(teardownPrimary).toHaveBeenCalledOnce() + expect(sendApplied).toHaveBeenCalledOnce() + }) }) describe('resolveTerminalConnection', () => { diff --git a/apps/desktop/electron/connection-apply.ts b/apps/desktop/electron/connection-apply.ts index 579af1a79e97..5ccd76d93a92 100644 --- a/apps/desktop/electron/connection-apply.ts +++ b/apps/desktop/electron/connection-apply.ts @@ -1,3 +1,15 @@ +// The in-flight soft re-home promise, if any. Two connection-config:apply +// calls for the global/primary scope can arrive back-to-back (e.g. the +// Settings "Apply" button and the cloud-agent "Connect" button are two +// independent UI triggers with independent pending-state guards, so nothing +// stops both firing close together). Without this, a second call would start +// its own teardownPrimary() while the first is still waiting on the real +// process exit, race the "backend stopped" toast suppression back off early +// (surfacing a spurious crash toast for the first's still-pending teardown), +// and fire a second hermes:connection:applied the renderer has no guard +// against. Concurrent callers instead await the one in-flight re-home. +let primaryRehomeInFlight = null + async function applyConnectionChange({ cancelAndWait, isPrimary, @@ -23,8 +35,20 @@ async function applyConnectionChange({ return } - await teardownPrimary() - sendApplied() + // A second call arriving while one is already in flight awaits the same + // re-home instead of racing its own teardown + notify. + if (!primaryRehomeInFlight) { + primaryRehomeInFlight = (async () => { + try { + await teardownPrimary() + sendApplied() + } finally { + primaryRehomeInFlight = null + } + })() + } + + await primaryRehomeInFlight } function commitConnectionFailure(current, starting, commit) { diff --git a/apps/desktop/src/app/gateway/hooks/use-gateway-boot.test.tsx b/apps/desktop/src/app/gateway/hooks/use-gateway-boot.test.tsx index a86446d3a168..b968991a6d84 100644 --- a/apps/desktop/src/app/gateway/hooks/use-gateway-boot.test.tsx +++ b/apps/desktop/src/app/gateway/hooks/use-gateway-boot.test.tsx @@ -250,6 +250,32 @@ describe('useGatewayBoot remote reconnect loop (real hook, fake socket)', () => expect($gatewayState.get()).toBe('open') }) + it('two hermes:connection:applied events firing back-to-back only run softSwitch once', async () => { + // Reproduces main's connection-config:apply race: the Settings "Apply" + // button and the cloud-agent "Connect" button are two independent UI + // triggers with independent pending-state guards, so nothing stops both + // firing close together — main can (pre-fix) emit + // hermes:connection:applied twice for one user action. Without a + // reentrancy guard in softSwitch(), each event independently wipes the + // session lists and re-dials — beforeConnectionSwitch() is called once + // per real softSwitch body execution, right after the guard, so its call + // count is the signal a guard vs. no-guard implementation disagrees on. + const beforeConnectionSwitch = vi.fn() + render() + await flushAsync() + expect(connectionApplied).not.toBeNull() + expect(beforeConnectionSwitch).not.toHaveBeenCalled() + + act(() => { + connectionApplied?.() + connectionApplied?.() + }) + await flushAsync() + + expect(beforeConnectionSwitch).toHaveBeenCalledTimes(1) + expect($gatewayState.get()).toBe('open') + }) + it('a remote that drops post-boot keeps looping with NO boot.error (the dead-end CONNECTING combo)', async () => { render() await flushAsync() 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 334ea4e604dd..7d4cc49725bc 100644 --- a/apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts +++ b/apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts @@ -291,7 +291,12 @@ export function useGatewayBoot({ // Soft gateway-mode apply: main tore down the primary without reloading. // Wipe session lists so skeletons retrigger, then re-dial in place. const softSwitch = async () => { - if (cancelled) { + // Reentrancy guard: main dedupes concurrent connection-config:apply + // calls (see primaryRehomeInFlight in connection-apply.ts) so this + // should only ever fire once per soft re-home, but guard here too — a + // second overlapping in-flight switch must not wipe the session lists + // / re-dial a second time out from under the first. + if (cancelled || $gatewaySwitching.get()) { return }