Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 55 additions & 0 deletions apps/desktop/electron/connection-apply.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,61 @@
})
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 () => {

Check warning on line 76 in apps/desktop/electron/connection-apply.test.ts

View workflow job for this annotation

GitHub Actions / JS & TS checks / apps/desktop / check:lint

Expected blank line before this statement
await gate.promise
})
const sendApplied = vi.fn()

Check warning on line 79 in apps/desktop/electron/connection-apply.test.ts

View workflow job for this annotation

GitHub Actions / JS & TS checks / apps/desktop / check:lint

Expected blank line before this statement

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()

Check warning on line 96 in apps/desktop/electron/connection-apply.test.ts

View workflow job for this annotation

GitHub Actions / JS & TS checks / apps/desktop / check:lint

Expected blank line before this statement

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()

Check warning on line 113 in apps/desktop/electron/connection-apply.test.ts

View workflow job for this annotation

GitHub Actions / JS & TS checks / apps/desktop / check:lint

Expected blank line before this statement
expect(sendApplied).not.toHaveBeenCalled()

gate.resolve()
await Promise.all([first, second])

expect(teardownPrimary).toHaveBeenCalledOnce()
expect(sendApplied).toHaveBeenCalledOnce()
})
})

describe('resolveTerminalConnection', () => {
Expand Down
28 changes: 26 additions & 2 deletions apps/desktop/electron/connection-apply.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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) {
Expand Down
26 changes: 26 additions & 0 deletions apps/desktop/src/app/gateway/hooks/use-gateway-boot.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(<Harness beforeConnectionSwitch={beforeConnectionSwitch} />)
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(<Harness />)
await flushAsync()
Expand Down
7 changes: 6 additions & 1 deletion apps/desktop/src/app/gateway/hooks/use-gateway-boot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
Loading