Skip to content
Closed
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
159 changes: 159 additions & 0 deletions apps/desktop/src/store/gateway-secondary-open-check.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'

// Regression for issue #92265: a transient first-dial WebSocket failure
// (e.g. ECONNRESET before the socket reaches `open`) must not let Desktop
// publish the closed gateway as the active route. entry.connection is set
// BEFORE the dial completes in openSecondary(), so checking only its
// truthiness previously let a failed activation still "succeed" and
// publish -- the next chat RPC then failed with "Hermes gateway is not
// connected" even though the UI had already switched to that route.

const gatewayMocks = vi.hoisted(() => ({
connect: vi.fn(async (_wsUrl: string): Promise<void> => undefined),
setConnection: vi.fn(),
setGatewayState: vi.fn()
}))

vi.mock('@/hermes', () => ({
setApiRequestConnection: vi.fn(),
HermesGateway: class {
connectionState = 'closed'
connect = async (wsUrl: string): Promise<void> => {
// Unlike gateway-agent-scope.test.ts's always-succeeds mock, this
// one propagates gatewayMocks.connect's outcome -- letting tests
// below simulate a rejected first dial without flipping
// connectionState to 'open'.
await gatewayMocks.connect(wsUrl)
this.connectionState = 'open'
}
close = (): void => {
this.connectionState = 'closed'
}
onEvent = vi.fn(() => () => {})
onState = vi.fn(() => () => {})
}
}))
vi.mock('@/store/session', () => ({
setConnection: gatewayMocks.setConnection,
setGatewayState: gatewayMocks.setGatewayState
}))
vi.mock('@/store/notify-baseline', () => ({ markNativeNotifyBaseline: vi.fn() }))

const {
$gateway,
activeGateway,
closeSecondaryGateways,
configureGatewayRegistry,
ensureGatewayForAgent,
ensureGatewayForProfile,
isActivePrimary,
setPrimaryGateway
} = await import('./gateway')

interface DesktopStub {
getConnection: ReturnType<typeof vi.fn>
getConnectionFor: ReturnType<typeof vi.fn>
}

function installDesktop(stub: DesktopStub): void {
;(window as unknown as { hermesDesktop: unknown }).hermesDesktop = stub
}

function makePrimary(): { connectionState: string } {
return { connectionState: 'open' }
}

const agentConn = {
authMode: 'token',
baseUrl: 'https://homelab.invalid',
mode: 'remote',
profile: 'research',
token: 'fake-test-token',
wsUrl: 'wss://homelab.invalid/api/ws?token=fake-test-token'
}

function installAgentDesktop(): DesktopStub {
const stub: DesktopStub = {
getConnection: vi.fn(async () => agentConn),
getConnectionFor: vi.fn(async () => agentConn)
}

installDesktop(stub)

return stub
}

beforeEach(() => {
configureGatewayRegistry({ onEvent: vi.fn() })
})

afterEach(() => {
closeSecondaryGateways()
vi.clearAllMocks()
delete (window as unknown as { hermesDesktop?: unknown }).hermesDesktop
})

describe('secondary activation requires an open socket, not just a connection descriptor (issue #92265)', () => {
it('ensureGatewayForAgent: a transient first-dial failure does not activate or publish the closed gateway', async () => {
const primary = makePrimary()
setPrimaryGateway(primary as never, 'default')
await ensureGatewayForProfile('default')
const publishedPrimary = $gateway.get()
installAgentDesktop()

gatewayMocks.connect.mockRejectedValueOnce(new Error('ECONNRESET'))

const activated = await ensureGatewayForAgent('homelab', 'research')

expect(activated).toBe(false)
// The exact reported symptom: the UI must NOT have switched away from
// the primary onto the closed secondary.
expect(isActivePrimary()).toBe(true)
expect(activeGateway()).toBe(primary)
expect($gateway.get()).toBe(publishedPrimary)
})

it('ensureGatewayForAgent: a successful dial still activates and publishes normally', async () => {
const primary = makePrimary()
setPrimaryGateway(primary as never, 'default')
installAgentDesktop()

const activated = await ensureGatewayForAgent('homelab', 'research')

expect(activated).toBe(true)
expect(isActivePrimary()).toBe(false)
expect(activeGateway()).not.toBe(primary)
expect($gateway.get()).not.toBe(primary)
})

it('ensureGatewayForProfile: a transient first-dial failure does not publish the closed gateway', async () => {
const primary = makePrimary()
setPrimaryGateway(primary as never, 'default')
await ensureGatewayForProfile('default')
const publishedPrimary = $gateway.get()
installDesktop({
getConnection: vi.fn(async () => agentConn),
getConnectionFor: vi.fn(async () => agentConn)
})

gatewayMocks.connect.mockRejectedValueOnce(new Error('ECONNRESET'))

await ensureGatewayForProfile('research')

// Must still be on the primary -- the closed secondary was never
// published as the active route.
expect(isActivePrimary()).toBe(true)
expect($gateway.get()).toBe(publishedPrimary)
})

it('ensureGatewayForProfile: a successful dial still activates and publishes normally', async () => {
const primary = makePrimary()
setPrimaryGateway(primary as never, 'default')
installAgentDesktop()

await ensureGatewayForProfile('research')

expect(isActivePrimary()).toBe(false)
expect($gateway.get()).not.toBe(primary)
})
})
16 changes: 10 additions & 6 deletions apps/desktop/src/store/gateway-shared-remote.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ const {
$gateway,
closeSecondaryGateways,
configureGatewayRegistry,
ensureActiveGatewayOpen,
ensureGatewayForProfile,
setPrimaryGateway
} = await import('./gateway')
Expand Down Expand Up @@ -113,7 +112,7 @@ describe('ensureGatewayForProfile under a shared global remote', () => {
expect($gateway.get()).not.toBe(primary)
})

it('refreshes the active connection after a pooled profile reconnect succeeds', async () => {
it('does not publish on a failed first dial, but a retried activation succeeds once the reconnect works (issue #92265)', async () => {
const connection = {
authMode: 'token',
baseUrl: 'https://worker.invalid',
Expand All @@ -130,14 +129,19 @@ describe('ensureGatewayForProfile under a shared global remote', () => {

gatewayMocks.connect.mockRejectedValueOnce(new Error('temporarily offline')).mockResolvedValueOnce(undefined)

// First attempt: the dial fails transiently. Per the fix, this must
// NOT publish the closed gateway as the active connection -- the
// prior behavior here (publish immediately, closed socket and all)
// is exactly the bug #92265 reports.
await ensureGatewayForProfile('worker')

expect(gatewayMocks.setConnection).toHaveBeenCalledOnce()
expect(gatewayMocks.setConnection).toHaveBeenLastCalledWith(connection)
expect(gatewayMocks.setConnection).not.toHaveBeenCalled()

await ensureActiveGatewayOpen()
// Retrying the activation (e.g. the user selects the profile again,
// or an app-level retry) succeeds on the second dial and publishes.
await ensureGatewayForProfile('worker')

expect(gatewayMocks.setConnection).toHaveBeenCalledTimes(2)
expect(gatewayMocks.setConnection).toHaveBeenCalledOnce()
expect(gatewayMocks.setConnection).toHaveBeenLastCalledWith(connection)
})
})
19 changes: 17 additions & 2 deletions apps/desktop/src/store/gateway.ts
Original file line number Diff line number Diff line change
Expand Up @@ -749,11 +749,17 @@ export async function ensureGatewayForAgent(connectionId: null | string, profile
entry.activationLeaseUntil = 0

// A source edit/remove may dispose this entry while its dial is still in
// flight. Only the still-registered, still-owned activation may publish.
// flight. Only the still-registered, still-owned activation may publish --
// and only when the WebSocket actually reached open: entry.connection is
// set BEFORE the dial completes in openSecondary, so a transient first-dial
// failure (caught above, left for scheduleReconnect) must not count as a
// successful activation just because a connection descriptor exists
// (issue #92265).
const activated =
entry.wantOpen &&
g.secondaries.get(scope) === entry &&
Boolean(entry.connection) &&
isOpen(entry.gateway) &&
applyActive(scope, activationEpoch)

if (activated && entry.connection) {
Expand Down Expand Up @@ -812,7 +818,16 @@ export async function ensureGatewayForProfile(profile: string): Promise<void> {
// The activation is settling either way — release the prune lease.
entry.activationLeaseUntil = 0

if (entry.wantOpen && g.secondaries.get(key) === entry && applyActive(key, activationEpoch) && entry.connection) {
// Only publish when the WebSocket actually reached open -- entry.connection
// is set before the dial completes, so a transient first-dial failure must
// not count as a successful activation (issue #92265).
if (
entry.wantOpen &&
g.secondaries.get(key) === entry &&
isOpen(entry.gateway) &&
applyActive(key, activationEpoch) &&
entry.connection
) {
publishActiveConnection(entry.connection)
}
}
Expand Down
Loading