fix(desktop): publish gateway, profile, and connection descriptor atomically on a profile switch
ensureGatewayProfile used to activate the target gateway and set $activeGatewayProfile while the connection descriptor fetch was still in flight, so during that window $gateway already targeted the new backend while $connection still described the previous one, and any request or plugin mode-listener firing then announced the wrong mode to the new backend. A failed descriptor fetch made the mismatch permanent. prepareGatewayForProfile (new gateway-store seam) opens the socket and returns a synchronous activation thunk without publishing anything; ensureGatewayForProfile now delegates to it. The switch resolves the descriptor and opens the socket first, then flips the active gateway, the profile atom, and $connection in one synchronous frame. A descriptor failure aborts the switch as a unit: nothing is published and every atom still consistently describes the previous profile. The deferred-descriptor test holds the fetch open and asserts the public atoms never disagree, then releases it and asserts all three flipped together; the failure test asserts no partial publication.
This commit is contained in:
@@ -717,27 +717,34 @@ export async function ensureGatewayForAgent(connectionId: null | string, profile
|
||||
return activated
|
||||
}
|
||||
|
||||
// Make `profile` the active gateway, lazily opening its socket if needed. The
|
||||
// primary is a no-op fast path. Background sockets are never closed here.
|
||||
export async function ensureGatewayForProfile(profile: string): Promise<void> {
|
||||
// Open `profile`'s socket if needed and hand back a synchronous activation
|
||||
// thunk — the publication seam for atomic profile switches. The caller invokes
|
||||
// the thunk in the same synchronous frame as its own atom writes (profile
|
||||
// pointer, connection descriptor), so no subscriber can observe the active
|
||||
// gateway pointing at one backend while companion state still describes
|
||||
// another. Nothing is published until the thunk runs.
|
||||
export async function prepareGatewayForProfile(profile: string): Promise<() => void> {
|
||||
const key = normKey(profile)
|
||||
const activationEpoch = beginGatewayActivation()
|
||||
|
||||
if (key === g.primaryProfile) {
|
||||
applyActive(key, activationEpoch)
|
||||
|
||||
return
|
||||
return () => {
|
||||
applyActive(key, activationEpoch)
|
||||
}
|
||||
}
|
||||
|
||||
// Global-remote share (routing case 3): one remote host serves every
|
||||
// profile through the PRIMARY socket, scoped per request. Activate the
|
||||
// primary instead of dialing a doomed duplicate socket at the same
|
||||
// descriptor — $activeGatewayProfile still moves to `key`, so request
|
||||
// scoping and profile-aware surfaces behave identically.
|
||||
// descriptor - $activeGatewayProfile still moves to `key`, so request
|
||||
// scoping and profile-aware surfaces behave identically. Checked BEFORE
|
||||
// createSecondary so a shared-remote profile never mints a secondary
|
||||
// entry, and returned as a thunk like every other path here so this
|
||||
// switch publishes as atomically as a dedicated-socket one.
|
||||
if (await sharedPrimaryRoute(key)) {
|
||||
applyActive(g.primaryProfile, activationEpoch)
|
||||
|
||||
return
|
||||
return () => {
|
||||
applyActive(g.primaryProfile, activationEpoch)
|
||||
}
|
||||
}
|
||||
|
||||
let entry = g.secondaries.get(key)
|
||||
@@ -760,11 +767,31 @@ export async function ensureGatewayForProfile(profile: string): Promise<void> {
|
||||
}
|
||||
}
|
||||
|
||||
if (entry.wantOpen && g.secondaries.get(key) === entry && applyActive(key, activationEpoch) && entry.connection) {
|
||||
publishActiveConnection(entry.connection)
|
||||
// Bind the entry the await settled on. `g.secondaries.get(key)` can be a
|
||||
// DIFFERENT object by the time the thunk runs (a teardown + redial between
|
||||
// prepare and publish), and publishing that one's descriptor would be the
|
||||
// very mismatch this seam exists to prevent, so the identity re-check below
|
||||
// compares against this exact entry.
|
||||
const prepared = entry
|
||||
|
||||
return () => {
|
||||
if (
|
||||
prepared.wantOpen &&
|
||||
g.secondaries.get(key) === prepared &&
|
||||
applyActive(key, activationEpoch) &&
|
||||
prepared.connection
|
||||
) {
|
||||
publishActiveConnection(prepared.connection)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Make `profile` the active gateway, lazily opening its socket if needed. The
|
||||
// primary is a no-op fast path. Background sockets are never closed here.
|
||||
export async function ensureGatewayForProfile(profile: string): Promise<void> {
|
||||
;(await prepareGatewayForProfile(profile))()
|
||||
}
|
||||
|
||||
// Reconnect the active gateway after a transient request failure. Primary
|
||||
// reconnects are owned by use-gateway-boot, so we only drive secondaries here.
|
||||
export async function ensureActiveGatewayOpen(): Promise<HermesGateway | null> {
|
||||
|
||||
@@ -6,13 +6,21 @@ import type { ProfileInfo } from '@/types/hermes'
|
||||
|
||||
// Keep profile.ts's side-effecting imports inert: the gateway socket layer and
|
||||
// the REST query client must not run for real in a unit test.
|
||||
const activateGateway = vi.fn()
|
||||
const ensureGatewayForProfile = vi.fn(async () => undefined)
|
||||
const ensureGatewayForAgent = vi.fn(async () => undefined)
|
||||
const prepareGatewayForProfile = vi.fn(async (_profile: string) => activateGateway)
|
||||
const openGatewayForProfile = vi.fn(async (_profile: string) => undefined)
|
||||
const $gateway = atom<unknown>({ id: 'live-socket' })
|
||||
const resetStarmapGraph = vi.fn()
|
||||
|
||||
vi.mock('@/store/gateway', () => ({ $gateway, ensureGatewayForAgent, ensureGatewayForProfile, openGatewayForProfile }))
|
||||
vi.mock('@/store/gateway', () => ({
|
||||
$gateway,
|
||||
ensureGatewayForAgent,
|
||||
ensureGatewayForProfile,
|
||||
openGatewayForProfile,
|
||||
prepareGatewayForProfile
|
||||
}))
|
||||
vi.mock('@/hermes', () => ({
|
||||
getProfiles: vi.fn(async () => ({ profiles: [] })),
|
||||
setApiRequestProfile: vi.fn()
|
||||
@@ -53,7 +61,9 @@ const getConnection = vi.fn<(profile?: string | null) => Promise<HermesConnectio
|
||||
|
||||
beforeEach(() => {
|
||||
getConnection.mockReset()
|
||||
activateGateway.mockClear()
|
||||
ensureGatewayForProfile.mockClear()
|
||||
prepareGatewayForProfile.mockClear()
|
||||
openGatewayForProfile.mockClear()
|
||||
$gateway.set({ id: 'live-socket' })
|
||||
$activeGatewayProfile.set('default')
|
||||
@@ -79,7 +89,8 @@ describe('ensureGatewayProfile → $connection sync (#46651)', () => {
|
||||
|
||||
await ensureGatewayProfile('vps-remote')
|
||||
|
||||
expect(ensureGatewayForProfile).toHaveBeenCalledWith('vps-remote')
|
||||
expect(prepareGatewayForProfile).toHaveBeenCalledWith('vps-remote')
|
||||
expect(activateGateway).toHaveBeenCalledTimes(1)
|
||||
expect(getConnection).toHaveBeenCalledWith('vps-remote')
|
||||
expect($connection.get()?.mode).toBe('remote')
|
||||
expect($connection.get()?.profile).toBe('vps-remote')
|
||||
@@ -96,15 +107,51 @@ describe('ensureGatewayProfile → $connection sync (#46651)', () => {
|
||||
expect($connection.get()?.mode).toBe('local')
|
||||
})
|
||||
|
||||
it('leaves the prior connection intact when the descriptor fetch fails', async () => {
|
||||
it('fails as a unit when the descriptor fetch fails — no mixed state', async () => {
|
||||
// Previously the gateway was activated and $activeGatewayProfile set even
|
||||
// when the descriptor lookup failed, leaving $gateway on the new backend
|
||||
// while $connection kept describing the old one for the rest of the
|
||||
// session. Now nothing is published: every atom still consistently
|
||||
// describes the previous profile and the user can retry.
|
||||
getConnection.mockRejectedValue(new Error('backend unreachable'))
|
||||
|
||||
await ensureGatewayProfile('vps-remote')
|
||||
|
||||
// Best-effort: boot/reconnect resyncs later; we must not null it out here.
|
||||
expect(activateGateway).not.toHaveBeenCalled()
|
||||
expect($activeGatewayProfile.get()).toBe('default')
|
||||
expect($connection.get()?.mode).toBe('local')
|
||||
})
|
||||
|
||||
it('never publishes the new gateway before its connection descriptor', async () => {
|
||||
// The exact mixed-state window from the follow-up review: a slow
|
||||
// descriptor fetch must not leave $gateway/$activeGatewayProfile on the
|
||||
// remote backend while $connection still says local. All three flip
|
||||
// together only once the descriptor is in hand.
|
||||
let resolveDescriptor: (conn: HermesConnection) => void = () => undefined
|
||||
getConnection.mockReturnValue(
|
||||
new Promise<HermesConnection>(resolve => {
|
||||
resolveDescriptor = resolve
|
||||
})
|
||||
)
|
||||
|
||||
const switching = ensureGatewayProfile('vps-remote')
|
||||
// Let the socket-open half of the switch settle; the descriptor is still
|
||||
// deliberately pending.
|
||||
await Promise.resolve()
|
||||
await Promise.resolve()
|
||||
|
||||
expect(activateGateway).not.toHaveBeenCalled()
|
||||
expect($activeGatewayProfile.get()).toBe('default')
|
||||
expect($connection.get()?.mode).toBe('local')
|
||||
|
||||
resolveDescriptor(remoteConn())
|
||||
await switching
|
||||
|
||||
expect(activateGateway).toHaveBeenCalledTimes(1)
|
||||
expect($activeGatewayProfile.get()).toBe('vps-remote')
|
||||
expect($connection.get()?.mode).toBe('remote')
|
||||
})
|
||||
|
||||
it('does not churn $connection when the target is already the active profile', async () => {
|
||||
$activeGatewayProfile.set('vps-remote')
|
||||
$connection.set(remoteConn())
|
||||
@@ -112,7 +159,7 @@ describe('ensureGatewayProfile → $connection sync (#46651)', () => {
|
||||
await ensureGatewayProfile('vps-remote')
|
||||
|
||||
expect(getConnection).not.toHaveBeenCalled()
|
||||
expect(ensureGatewayForProfile).not.toHaveBeenCalled()
|
||||
expect(prepareGatewayForProfile).not.toHaveBeenCalled()
|
||||
expect($connection.get()?.mode).toBe('remote')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -12,7 +12,12 @@ import {
|
||||
storedStringRecord
|
||||
} from '@/lib/storage'
|
||||
import { invalidateCronModelImpactScopeState } from '@/store/cron-model-impact-scope'
|
||||
import { $gateway, ensureGatewayForAgent, ensureGatewayForProfile, openGatewayForProfile } from '@/store/gateway'
|
||||
import {
|
||||
$gateway,
|
||||
ensureGatewayForAgent,
|
||||
openGatewayForProfile,
|
||||
prepareGatewayForProfile
|
||||
} from '@/store/gateway'
|
||||
import { setConnection } from '@/store/session'
|
||||
import { resetStarmapGraph } from '@/store/starmap'
|
||||
import type { ProfileInfo } from '@/types/hermes'
|
||||
@@ -269,30 +274,24 @@ export function prewarmProfileBackend(name: string): void {
|
||||
|
||||
let gatewaySwitch: Promise<void> | null = null
|
||||
|
||||
// Keep the renderer's $connection (mode / baseUrl / profile) in lockstep with
|
||||
// the profile the live gateway is now on. $connection seeds from the PRIMARY
|
||||
// The target profile's connection descriptor (mode / baseUrl / …), fetched
|
||||
// BEFORE activation so the switch can publish it in the same synchronous frame
|
||||
// as the gateway and profile pointer. $connection seeds from the PRIMARY
|
||||
// (window) backend at boot and otherwise only refreshes on a sleep/wake
|
||||
// reconnect — so activating a *background* profile left $connection describing
|
||||
// the primary, with the wrong `mode` for everything that branches on
|
||||
// local-vs-remote. Headline symptom: with a local primary and a remote pool
|
||||
// profile active, image attachments went out via the path-based `image.attach`
|
||||
// instead of `image.attach_bytes`, handing the remote gateway a client-only
|
||||
// path it can't resolve ("image not found: C:\…"), while the /api/fs/* file
|
||||
// browser and /api/media fetches targeted the wrong machine (#46651).
|
||||
// Best-effort: a failed descriptor fetch leaves the prior connection intact for
|
||||
// boot/reconnect to resync.
|
||||
async function syncConnectionToActiveProfile(profile: string): Promise<void> {
|
||||
// reconnect — so activating a *background* profile without this left
|
||||
// $connection describing the primary, with the wrong `mode` for everything
|
||||
// that branches on local-vs-remote (#46651: path-based `image.attach` against
|
||||
// a remote gateway, /api/fs/* and /api/media on the wrong machine).
|
||||
//
|
||||
// Null means "no desktop bridge" (plain browser) — there is no descriptor to
|
||||
// sync then. A bridge REJECTION propagates: the caller aborts the whole switch
|
||||
// rather than activating a backend whose descriptor (and thus mode) is
|
||||
// unknown, which previously left $gateway on the new backend while
|
||||
// $connection kept describing the old one for the rest of the session.
|
||||
async function resolveConnectionForProfile(profile: string) {
|
||||
const getConnection = window.hermesDesktop?.getConnection
|
||||
|
||||
if (!getConnection) {
|
||||
return
|
||||
}
|
||||
|
||||
try {
|
||||
setConnection(await getConnection(profile))
|
||||
} catch {
|
||||
// Leave the prior connection in place; boot/reconnect resyncs it later.
|
||||
}
|
||||
return getConnection ? getConnection(profile) : null
|
||||
}
|
||||
|
||||
// Make `profile`'s backend the active gateway, lazily opening its socket if it
|
||||
@@ -331,14 +330,31 @@ export async function ensureGatewayProfile(profile: string | null | undefined):
|
||||
|
||||
$gatewaySwapTarget.set(target)
|
||||
gatewaySwitch = (async () => {
|
||||
// ensureGatewayForProfile opens (or reuses) the target's socket and points
|
||||
// the active gateway at it — without closing the profile you came from.
|
||||
await ensureGatewayForProfile(target)
|
||||
// Resolve the target's connection descriptor and open (or reuse) its
|
||||
// socket BEFORE anything is published — without closing the profile you
|
||||
// came from. The gateway used to be activated (and the profile atom set)
|
||||
// while the descriptor fetch was still in flight, so during that window
|
||||
// $gateway already targeted the new backend while $connection still
|
||||
// described the previous one — and any request or plugin mode-listener
|
||||
// firing then announced the WRONG mode to the new backend.
|
||||
const [connection, activate] = await Promise.all([
|
||||
resolveConnectionForProfile(target),
|
||||
prepareGatewayForProfile(target)
|
||||
])
|
||||
|
||||
// One synchronous publication frame — no awaits from here down, so the
|
||||
// active gateway, $activeGatewayProfile, and $connection flip together.
|
||||
activate()
|
||||
$activeGatewayProfile.set(target)
|
||||
// The active backend just changed; resync $connection so remote-aware
|
||||
// paths (image.attach_bytes vs image.attach, /api/fs/*, /api/media) follow.
|
||||
await syncConnectionToActiveProfile(target)
|
||||
})()
|
||||
|
||||
if (connection) {
|
||||
setConnection(connection)
|
||||
}
|
||||
})().catch(() => {
|
||||
// Descriptor lookup failed: the switch fails as a unit. Nothing was
|
||||
// published, so every atom still consistently describes the previous
|
||||
// profile; the user can retry the switch.
|
||||
})
|
||||
|
||||
try {
|
||||
await gatewaySwitch
|
||||
|
||||
Reference in New Issue
Block a user