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
6 changes: 5 additions & 1 deletion apps/desktop/src/app/contrib/wiring.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import { requestVoiceConversationStart } from '@/store/composer'
import { $activeConnectionId } from '@/store/connections'
import { $cronReviewRequest, setCronFocusJobId } from '@/store/cron'
import { $pinnedSessionIds, pinSession, restoreWorktree, unpinSession } from '@/store/layout'
import { notifyError } from '@/store/notifications'
import { $previewTarget } from '@/store/preview'
import {
$activeGatewayProfile,
Expand Down Expand Up @@ -749,7 +750,10 @@ export function ContribWiring({ children }: { children: ReactNode }) {
if (payload?.start_new_session !== false) {
newSessionInProfile(targetProfile)
} else {
void ensureGatewayProfile(normalizeProfileKey(targetProfile))
void ensureGatewayProfile(normalizeProfileKey(targetProfile)).catch((error: unknown) => {
// #81094: the voice-path switch must surface its failure too.
notifyError(error, `Failed to switch to profile "${normalizeProfileKey(targetProfile)}"`)
})
}
} else if (payload?.start_new_session !== false) {
startFreshSessionDraft()
Expand Down
18 changes: 12 additions & 6 deletions apps/desktop/src/store/gateway-shared-remote.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,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('rejects the failed dial without publishing an activation, then activates once the backend returns', async () => {
const connection = {
authMode: 'token',
baseUrl: 'https://worker.invalid',
Expand All @@ -130,14 +130,20 @@ describe('ensureGatewayForProfile under a shared global remote', () => {

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

await ensureGatewayForProfile('worker')
// #81094: a failed dial must REJECT instead of silently activating a
// closed socket that would route messages to the primary backend.
await expect(ensureGatewayForProfile('worker')).rejects.toThrow('temporarily offline')

expect(gatewayMocks.setConnection).toHaveBeenCalledOnce()
expect(gatewayMocks.setConnection).toHaveBeenLastCalledWith(connection)
// No activation was published for the dead dial — $connection keeps the
// primary's descriptor (set by setPrimaryGateway), never the unreachable
// secondary's.
expect(gatewayMocks.setConnection).not.toHaveBeenCalled()

await ensureActiveGatewayOpen()
// Once the backend is reachable again, retrying the switch activates and
// publishes the live connection descriptor.
await ensureGatewayForProfile('worker')

expect(gatewayMocks.setConnection).toHaveBeenCalledTimes(2)
expect(gatewayMocks.setConnection).toHaveBeenCalledTimes(1)
expect(gatewayMocks.setConnection).toHaveBeenLastCalledWith(connection)
})
})
180 changes: 180 additions & 0 deletions apps/desktop/src/store/gateway.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'

// Connection lifecycle for registry-scoped secondary gateways:
//
// 1. Removing a connection must dispose its secondaries — remote/cloud
// sources have no local process whose death would drop the socket, so
// without an explicit dispose the WebSocket stays open and streams ghost
// events until page reload.
// 2. A materially edited connection re-dials so fresh sockets target the
// NEW endpoint.
// 3. When the Electron main reports the connection no longer exists
// (`No connection with id`), the reconnect loop fail-stops and evicts
// the entry instead of retrying forever.

const gatewayMocks = vi.hoisted(() => {
const instances: { close: ReturnType<typeof vi.fn>; connectionState: string }[] = []

return {
connect: vi.fn(async (_wsUrl: string): Promise<void> => undefined),
instances
}
})

vi.mock('@/hermes', () => ({
setApiRequestConnection: vi.fn(),
HermesGateway: class {
connectionState = 'closed'
close = vi.fn(() => {
this.connectionState = 'closed'
})
connect = async (wsUrl: string): Promise<void> => {
await gatewayMocks.connect(wsUrl)
this.connectionState = 'open'
}
onEvent = vi.fn(() => () => {})
onState = vi.fn(() => () => {})
constructor() {
gatewayMocks.instances.push(this as never)
}
}
}))
vi.mock('@/store/session', () => ({
setConnection: vi.fn(),
setGatewayState: vi.fn()
}))
vi.mock('@/store/notify-baseline', () => ({ markNativeNotifyBaseline: vi.fn() }))

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

function installDesktop(stub: Record<string, unknown>): void {
;(window as unknown as { hermesDesktop: unknown }).hermesDesktop = stub
}

beforeEach(() => {
configureGatewayRegistry({ onEvent: vi.fn() } as never)
setPrimaryGateway({ connectionState: 'open' } as never, 'default')
})

afterEach(() => {
closeSecondaryGateways()
gatewayMocks.instances.length = 0
vi.clearAllMocks()
vi.useRealTimers()
delete (window as unknown as { hermesDesktop?: unknown }).hermesDesktop
})

describe('ensureGatewayForProfile — secondary connect failure surfaces (#81094)', () => {
it('rethrows the dial failure instead of activating a closed socket', async () => {
const getConnection = vi.fn(async ({ profile }: { profile: string }) => ({
authMode: 'token',
baseUrl: `https://${profile}.invalid`,
mode: 'local',
profile,
token: 'fake-test-token',
wsUrl: `wss://${profile}.invalid/ws`
}))

installDesktop({ getConnection })

// First activation succeeds so the entry exists.
await ensureGatewayForProfile('work')

const live = activeGateway()

expect(live).toBeTruthy()

// The socket then dies (backend restart): state flips to closed, so the
// next activation must re-dial instead of reusing the dead socket.
;(live as unknown as { connectionState: string }).connectionState = 'closed'
gatewayMocks.connect.mockRejectedValue(new Error('backend unreachable'))

await expect(ensureGatewayForProfile('work')).rejects.toThrow('backend unreachable')

// The failed switch must NOT fall through to setActive() with a closed
// socket: the active gateway is still the previously-live one, never the
// dead entry that just failed to dial.
const stillActive = activeGateway()

expect(stillActive).toBe(live)
expect(gatewayMocks.instances).toHaveLength(1)
})

it('releases the activation lease when the first dial is rejected so pruning disposes it', async () => {
const getConnection = vi.fn(async ({ profile }: { profile: string }) => ({
authMode: 'token',
baseUrl: `https://${profile}.invalid`,
mode: 'local',
profile,
token: 'fake-test-token',
wsUrl: `wss://${profile}.invalid/ws`
}))

installDesktop({ getConnection })
gatewayMocks.connect.mockRejectedValue(new Error('backend unreachable'))

await expect(ensureGatewayForProfile('work')).rejects.toThrow('backend unreachable')

pruneSecondaryGateways(new Set())

expect(gatewayMocks.instances[0].close).toHaveBeenCalledTimes(1)
})

it('keeps the reconnect schedule armed so transient failures still self-heal', async () => {
vi.useFakeTimers()

let failFirst = true

const getConnection = vi.fn(async ({ profile }: { profile: string }) => ({
authMode: 'token',
baseUrl: `https://${profile}.invalid`,
mode: 'local',
profile,
token: 'fake-test-token',
wsUrl: `wss://${profile}.invalid/ws`
}))

installDesktop({ getConnection })

gatewayMocks.connect.mockImplementation(async () => {
if (failFirst) {
throw new Error('backend unreachable')
}
})

await expect(ensureGatewayForProfile('work')).rejects.toThrow('backend unreachable')

// The catch kept the reconnect schedule: exactly one backoff timer is armed
// for the failed entry (transient failures still self-heal).
expect(vi.getTimerCount()).toBe(1)

// Backoff fires → reconnect dials again → succeeds → socket opens.
failFirst = false
await vi.runAllTimersAsync()
expect(gatewayMocks.instances[0].connectionState).toBe('open')
})

it('activates the secondary when connect succeeds', async () => {
const getConnection = vi.fn(async ({ profile }: { profile: string }) => ({
authMode: 'token',
baseUrl: `https://${profile}.invalid`,
mode: 'local',
profile,
token: 'fake-test-token',
wsUrl: `wss://${profile}.invalid/ws`
}))

installDesktop({ getConnection })

await ensureGatewayForProfile('work')

expect(activeGateway()).toBe(gatewayMocks.instances[0])
})
})
41 changes: 30 additions & 11 deletions apps/desktop/src/store/gateway.ts
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,16 @@ async function openSecondary(entry: Secondary): Promise<void> {

const wsUrl = await resolveGatewayWsUrl(wsDeps, conn)

await entry.gateway.connect(wsUrl)
try {
await entry.gateway.connect(wsUrl)
} catch (error) {
// Log the dial target for support, but RETHROW THE ORIGINAL ERROR —
// reconnectSecondary classifies failures by message ("No connection
// with id", "no longer exists") to fail-stop permanent conditions, and
// wrapping here would break that. Callers decide surfacing (#81094).
console.error(`[gateway] dial for profile "${entry.profile}" failed:`, error)
throw error
}

if (!entry.wantOpen) {
entry.gateway.close()
Expand Down Expand Up @@ -798,20 +807,30 @@ export async function ensureGatewayForProfile(profile: string): Promise<void> {
// profile-door twin of the agent path's lease above (#89622).
entry.activationLeaseUntil = Date.now() + ACTIVATION_LEASE_MS

if (!isOpen(entry.gateway)) {
clearTimer(entry)
entry.reconnectAttempt = 0
try {
if (!isOpen(entry.gateway)) {
clearTimer(entry)
entry.reconnectAttempt = 0

try {
await openSecondary(entry)
} catch {
scheduleReconnect(entry)
try {
await openSecondary(entry)
} catch (error) {
// #81094: a failed secondary dial must NOT fall through to setActive()
// with a closed socket — that silently routes the user's messages to the
// primary backend (cross-profile session writes). Keep the reconnect
// schedule (transient failures still self-heal via the backoff below)
// but RE-THROW so the profile-door caller surfaces the failure and skips
// the activation. The agent-door twin (ensureGatewayForAgent) keeps its
// boolean contract and is guarded by the activeGateway() null invariant.
scheduleReconnect(entry)
throw error
}
}
} finally {
// The activation is settling either way — release the prune lease.
entry.activationLeaseUntil = 0
}

// 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) {
publishActiveConnection(entry.connection)
}
Expand Down
82 changes: 82 additions & 0 deletions apps/desktop/src/store/profile-switch-failure.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'

// Profile-door activation failure surfacing (#81094): when a secondary's
// socket cannot be opened, ensureGatewayForProfile must rethrow (after
// arming the reconnect schedule) so the caller can surface the failure,
// and profile.ts must NOT publish an activation for a backend that never
// came up — previously the catch swallowed the error and setActive() ran
// anyway, silently routing messages to the primary socket.

const gatewayMocks = vi.hoisted(() => {
const instances: { close: ReturnType<typeof vi.fn>; connectionState: string }[] = []

return {
connect: vi.fn(async (_wsUrl: string): Promise<void> => undefined),
instances
}
})

vi.mock('@/hermes', () => ({
setApiRequestProfile: vi.fn(),
getProfiles: vi.fn(async () => ({ profiles: [] })),
HermesGateway: class {
connectionState = 'closed'
close = vi.fn(() => {
this.connectionState = 'closed'
})
connect = async (wsUrl: string): Promise<void> => {
await gatewayMocks.connect(wsUrl)
this.connectionState = 'open'
}
onEvent = vi.fn(() => () => {})
onState = vi.fn(() => () => {})
constructor() {
gatewayMocks.instances.push(this as never)
}
}
}))
vi.mock('@/store/session', () => ({
setConnection: vi.fn(),
setGatewayState: vi.fn()
}))
vi.mock('@/store/notify-baseline', () => ({ markNativeNotifyBaseline: vi.fn() }))
vi.mock('@/lib/query-client', () => ({ invalidateProfileScopedQueries: vi.fn() }))

const { ensureGatewayProfile } = await import('./profile')

function installDesktop(stub: Record<string, unknown>): void {
;(window as unknown as { hermesDesktop: unknown }).hermesDesktop = stub
}

function descriptorFor(profile: string) {
return {
authMode: 'token',
baseUrl: `https://${profile}.invalid`,
mode: 'local',
profile,
token: 'fake-test-token',
wsUrl: `wss://${profile}.invalid/ws`
}
}

beforeEach(() => {
gatewayMocks.instances.length = 0
})

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

describe('ensureGatewayProfile — switch failure surfaces instead of silent fallback (#81094)', () => {
it('rejects when the target backend cannot be dialed, instead of resolving silently', async () => {
const getConnection = vi.fn(async ({ profile }: { profile: string }) => descriptorFor(profile))
installDesktop({ getConnection })

// The secondary dial fails at connect(): the whole switch must reject.
gatewayMocks.connect.mockRejectedValue(new Error('backend unreachable'))

await expect(ensureGatewayProfile('work')).rejects.toThrow('backend unreachable')
})
})
Loading